[PATCH] D121141: [Clang] Add `-fexperimental-library` flag to enable unstable and experimental features: follow-up fixes

[Louis Dionne via Phabricator via cfe-commits]

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