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

Jonas Hahnfeld via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 25 01:30:55 PDT 2016


Hahnfeld requested changes to this revision.
Hahnfeld added a comment.
This revision now requires changes to proceed.

In general the idea looks good and takes this point off my personal todo list :-)

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.

Just a minor comment: Please include the full context in your diff as explained in the docs: http://llvm.org/docs/Phabricator.html#requesting-a-review-via-the-web-interface
This makes reviewing easier because one can browse the surrounding code ;-)


================
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;
 
----------------
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?


https://reviews.llvm.org/D22663





More information about the cfe-commits mailing list