ldionne marked 12 inline comments as done. ldionne added a comment. In D121141#3658946 <https://reviews.llvm.org/D121141#3658946>, @phosek wrote:
> That's correct, I implemented the support for `-stdlib=libc++` on MSVC > targets in D101479 <https://reviews.llvm.org/D101479>, but that change caused > a number of build failures including the compiler-rt due to > `_HAS_EXCEPTIONS=0` so relanding that change is blocked on D103947 > <https://reviews.llvm.org/D103947>. Awesome, thanks both for the additional context. I'll try to take a look at D103947 <https://reviews.llvm.org/D103947> soon. In the meantime, I'll mark the test as `XFAIL` on Windows just to unblock this patch, because it absolutely needs to land for LLVM 15. ================ Comment at: clang/lib/Driver/ToolChain.cpp:1016 CmdArgs.push_back("-lc++"); + if (Args.hasArg(options::OPT_fexperimental_library)) + CmdArgs.push_back("-lc++experimental"); ---------------- MaskRay wrote: > ldionne wrote: > > MaskRay wrote: > > > There may be an archive ordering problem. > > I'm not sure I follow -- what problem are you concerned about? > https://lld.llvm.org/ELF/warn_backrefs.html > > When these -l options are used to link archives (.a), they should be added in > a dependency order. > > -lc++experimental presumably uses symbols from -lc++abi and must precede > -lc++abi. > -lc++abi uses symbols from -lunwind and must precede -lunwind. > > For macOS and Windows, the order usually doesn't matter. Got it, thanks. In this case, however, we're only linking against libc++, not libc++abi. So presumably there isn't any potential issue here? However, I agree that the other places where I had `-lc++experimental` after `-lc++abi` are a problem, and I am fixing those. ================ Comment at: clang/test/Driver/experimental-library-flag.cpp:6 +// -fexperimental-library must be passed to CC1 +// CHECK: -fexperimental-library + ---------------- MaskRay wrote: > Suggest testing `-lc++` as well IMO this is not the right place to test this -- this should only be testing stuff that is specific to `-fexperimental-library`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D121141/new/ https://reviews.llvm.org/D121141 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
