[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-09 Thread Douglas Yung via Phabricator via cfe-commits
dyung added a comment. It also failed on the PS4 linux bot: http://lab.llvm.org:8014/#/builders/125/builds/125 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 __

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-09 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. Looks like this broke the tests because `-stdlib` is not a thing on Windows? Would passing a linux `-target x86_64-linux-gnu` be an appropriate fix for the tests? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ h

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-09 Thread Timm Bäder via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGa6439b52088b: [clang][driver] Only warn once about invalid library values (authored by tbaeder). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ ht

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-09 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl accepted this revision. yaxunl added a comment. This revision is now accepted and ready to land. LGTM. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 ___ cfe-commits mailing list cfe-co

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-09 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. Is this good to go now? Or still something left? @yaxunl? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-08 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 322082. tbaeder added a comment. Switched to `llvm::Optional` and added some tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 Files: clang/include/clang/Driver/ToolChain.h clang/lib/Driver/ToolChain

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-04 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added inline comments. Comment at: clang/include/clang/Driver/ToolChain.h:169-177 + mutable bool isCXXStdlibTypeCached; + mutable CXXStdlibType cxxStdlibType; + + mutable bool isRuntimeLibTypeCached; + mutable RuntimeLibType runtimeLibType; + + mutable bool isUnwind

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-04 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment. Test still missing. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-04 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. LGTM. It also avoid redundant check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailm

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-04 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 321349. tbaeder added a comment. Here a version without the local static bools. If this is good to go, I can add some tests. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 Files: clang/include/clang/Driv

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-03 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. In D95915#2541132 , @tbaeder wrote: > That's what I was looking at right now as well, since using > `std::call_once()` already means the methods can't be `const` anymore anyway. > Might as well just cache the value. You can mak

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-03 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. In D95915#2541129 , @Hahnfeld wrote: > My proposal would be to cache the return value of the three routines in > `ToolChain`. This has the advantage that the values get parsed only once and > there is at most one warning. I don't

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-03 Thread Jonas Hahnfeld via Phabricator via cfe-commits
Hahnfeld added a comment. My proposal would be to cache the return value of the three routines in `ToolChain`. This has the advantage that the values get parsed only once and there is at most one warning. I don't know how this plays with parallelization efforts, but I don't think we should worr

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-03 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder added a comment. In D95915#2539483 , @jdoerfert wrote: > Also, now you don't warn for different missing runtimes, which seems odd. What do you mean? Is the value returned from `GetRuntimeLibType()` ever going to change? CHANGES SINCE LAST ACTIO

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-03 Thread Yaxun Liu via Phabricator via cfe-commits
yaxunl added a comment. In D95915#2539483 , @jdoerfert wrote: > I don't believe a static variable is a good idea. Also the name is not really > informative. Members in `ToolChain` with a proper names would be nicer (IMHO). > > Tests missing. Also, now you

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-03 Thread Johannes Doerfert via Phabricator via cfe-commits
jdoerfert added a comment. Drive by: I don't believe a static variable is a good idea. Also the name is not really informative. A member in `ToolChain` with a proper name would be nicer (IMHO). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 _

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-03 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder updated this revision to Diff 321022. tbaeder added a comment. Noticed similar/worse behavior for -rtlib. Expanded the patch to that and -unwindlib as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95915/new/ https://reviews.llvm.org/D95915 Files: clang/lib/Driver/ToolC

[PATCH] D95915: [clang][driver] Only warn once about invalid -stdlib value

2021-02-02 Thread Timm Bäder via Phabricator via cfe-commits
tbaeder created this revision. tbaeder added reviewers: Hahnfeld, phosek, yaxunl, rsmith. tbaeder requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Since ToolChain::GetCXXStdlibType() is a simple getter that might emit the "invalid library n