[PATCH] D117888: [clang][driver][wasm] Support -stdlib=libstdc++ for WebAssembly

Tue Feb 1 07:41:28 PST 2022

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