sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
This looks great!
Only thing I'm uncomfortable with is the lack of test coverage outside clangd.
I think the most valuable thing would be to have some direct testing of how
RecursiveASTVisitor behaves.
Do you think you could add something to
`unittests/Tooling/RecursiveASTVisitorTests/Concept.cpp` at least for your
change and if there are any other bits that seem critical & missing?
================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3577
+ template<class T>
+ concept b = $expr^A<T> && $expr^sizeof(T) % 2 == 0 || $expr^A<T> &&
sizeof(T) == 1;
+ )cpp");
----------------
maybe also a constrained auto usage?
================
Comment at: clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp:3584
+ std::vector<Symbol> Syms = {conceptSym("same_as")};
+ for (auto P : Code.points("tparam")) {
+ ASSERT_THAT(completions(TU, P, Syms).Completions,
----------------
I think it'd be useful to assert the presence/absence of template
brackets/placeholders here, even if the behavior is not ideal today (in which
case, comment to that effect).
Similarly it's probably worth having a concept B with two type params as these
are different cases for template brackets when used as type (even if the
behavior is the same)
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:1851
+void checkFindRefs(llvm::StringRef Test, bool UseIndex = false,
+ std::vector<std::string> ExtraArgs = {}) {
Annotations T(Test);
----------------
nit: does anything break if we just set c++20 unconditionally?
================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:2080
+
+ [[IsSmall]] auto i = 8;
+ template<[[IsSmal^l]] U> void foo();
----------------
maybe `= 'c'` and use `char` below? Don't make me think :-)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D124441/new/
https://reviews.llvm.org/D124441
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
