sammccall added a comment.

In D60539#1469012 <https://reviews.llvm.org/D60539#1469012>, @nemanjai wrote:

> > Do you need to build clangd? We explicitly don't aim to support building 
> > everywhere clang can be built, maybe we should just disable in this case?
>
> Our environment includes various OS levels running on PowerPC. We certainly 
> wouldn't want to disable building/testing `clangd` on all our PowerPC 
> machines. Is there a way to disable it only on certain OS levels?
>
> Furthermore, it seems a little too intrusive to disable an otherwise 
> functional component simply because some test cases rely on a specific 
> language standard default.
>
> Would it be an acceptable solution to add another `StringRef` parameter to 
> `ShouldCollectSymbolTest::build()` - let's call it `ExtraArgs`, to which we 
> can add options such as `-std=c++14` if the test being built relies on that 
> option?


My concern is

- a large fraction of our tests, not just those in this file. rely on the 
default std version (I suspect setting it to c++98 will reveal that). I don't 
think maintaining this information alongside each test and plumbing it through 
every test helper is reasonable. If we want to be robust to changes in this 
flag, I think need to make at least TestTU do the right thing by default. 
Unfortunately the most obvious way to do that (adding `-std-default`) won't 
work.
- there's no buildbot coverage of these configurations, so I'm not sure how 
we'll keep them clean.

Given there's no obvious alternative and the patch is small, it seems OK to 
land this (with a suitable comment), but it's hard for us to commit to 
maintaining it e.g.

- if we add non-C++ testcases in SymbolCollectorTests and need to remove the arg
- for other values of `CLANG_DEFAULT_STD_CXX`
- as more tests are added in the future


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D60539/new/

https://reviews.llvm.org/D60539



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

Reply via email to