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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to