aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D131465#3772803 <https://reviews.llvm.org/D131465#3772803>, @MaskRay wrote:

> Update clang-tools-extra/clangd/unittests/SelectionTests.cpp @sammccall

This one looks to still be failing on Windows according to precommit CI.

Otherwise, the changes LGTM



================
Comment at: clang/test/lit.site.cfg.py.in:27
 config.clang_enable_opaque_pointers = @CLANG_ENABLE_OPAQUE_POINTERS_INTERNAL@
+config.clang_default_std_cxx = "@CLANG_DEFAULT_STD_CXX@"
 config.clang_default_cxx_stdlib = "@CLANG_DEFAULT_CXX_STDLIB@"
----------------
MaskRay wrote:
> aaron.ballman wrote:
> > MaskRay wrote:
> > > aaron.ballman wrote:
> > > > Should we add some documentation for this in a follow-up? (I know 
> > > > `CLANG_DEFAULT_STD_CXX` already exists, but it seems like it'd be 
> > > > helpful to tell users about it too.)
> > > Where shall we place the documentation? My understanding is that we don't 
> > > document lit.* config changes.
> > > 
> > > Personally I think we probably should get rid of `CLANG_DEFAULT_STD_CXX` 
> > > (D34365 @mgorny), which may require improvements to Clang configuration 
> > > file (https://discourse.llvm.org/t/configuration-files/42529 
> > > https://clang.llvm.org/docs/UsersManual.html#configuration-files).
> > Oh, I thought this was a CMake variable that users could set, so I was 
> > thinking it would have gone into 
> > https://llvm.org/docs/CMake.html#rarely-used-cmake-variables. Is this not 
> > something users set to try test out in a different language standard mode 
> > than the default?
> AIUI we currently document none of `CLANG_*` cmake variables, because we 
> don't have a Clang counterpart of  `llvm/docs/CMake.rst`.
> 
> For `CLANG_DEFAULT_STD_CXX`, I decide to remove it in D133375. But this patch 
> has to add minimum test support to make tests pass when it is still present.
> AIUI we currently document none of CLANG_* cmake variables, because we don't 
> have a Clang counterpart of llvm/docs/CMake.rst.

Yup, we should definitely address that at some point (not part of this patch).

> For CLANG_DEFAULT_STD_CXX, I decide to remove it in D133375. But this patch 
> has to add minimum test support to make tests pass when it is still present.

Okay, we can discuss the removal over there, thanks for the info!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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

Reply via email to