[PATCH] D22663: Support setting default value for -rtlib at build time

[Lei Zhang via cfe-commits]

zlei added a comment.

In https://reviews.llvm.org/D22663#494460, @Hahnfeld wrote:

> With the changes applied and configured with `CLANG_DEFAULT_RTLIB=compiler-rt` some tests fail so you may have to adapt the idea of `-rtlib=platform` for the tests.


Got it. And I also need to fix those failing tests by adding `-rtlib=platform` to them, right?


================
Comment at: CMakeLists.txt:210
@@ +209,3 @@
+  message(WARNING "Resetting default rtlib to use platform default")
+  set(CLANG_DEFAULT_RTLIB "")
+endif()
----------------
beanz wrote:
> You'll want this to be:
> 
> 
> ```
> set(CLANG_DEFAULT_RTLIB "" CACHE STRING "Default runtime library to use (libgcc or compiler-rt)" FORCE)
> ```
> 
> Cached variables can only be overwritten by a `FORCE`.
Sorry, I'm not very familiar with cmake. But why isn't `CLANG_DEFAULT_CXX_STDLIB` declared with `FORCE`? Should I fix it as well?

================
Comment at: lib/Driver/ToolChain.cpp:530
@@ -538,1 +529,3 @@
+  const Arg* A = Args.getLastArg(options::OPT_rtlib_EQ);
+  StringRef LibName = A ? A->getValue() : CLANG_DEFAULT_RTLIB;
 
----------------
Hahnfeld wrote:
> What I dislike here is that it falls back to the platform default if the specified argument value is invalid regardless of what was configured. This may be surprising for the user as he gets a different behaviour when specifiying `-rtlib=bla` than completely omitting.
> 
> I just added a comment explaining how this works in `GetCXXStdlibType`. Maybe you can adapt its idea?
If you give an invalid value to `-rtlib`, clang should just abort and give you an error message, instead of falling back to the platform default. I see that's also how `-stdlib` behaves.

Or maybe I misunderstood your point?


https://reviews.llvm.org/D22663