tbaeder marked 4 inline comments as done.
tbaeder added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/WebAssembly.cpp:544
+
+ if (Version.empty()) {
+ // FIXME: Fallback correct?
----------------
sbc100 wrote:
> tbaeder wrote:
> > sbc100 wrote:
> > > Just early return here if no headers can be found? Looking at Gnu.cpp it
> > > seems that `addLibStdCxxIncludePaths` can simply to nothing if no GCC
> > > install is found.
> > I saw that, but I'm not sure if this is correct for wasm. The tests
> > certainly break because they check for the `/v1/` (and not `/11/`) include
> > paths but also use `-sysroot=/foo`, so the new code doesn't add any flags.
> > Is there a good way to update the tests so they stay functional and useful?
> I would take a look at how other platforms test this... perhaps they setup
> some kind of fake header tree? Or perhaps they don't test these paths at
> all. For sure they don't depend on the actual system where the tests run,
> right?
>
> Can you point me to the tests that fail if you simply return empty string
> like on other platforms?
The test using `/foo` as sysroot was just
`clang/test/Driver/wasm-toolchain.cpp`.
The driver tests are shipping a file hierarchy to be used for this, so I
switched the tests to use that as well.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D117888/new/
https://reviews.llvm.org/D117888
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
