[PATCH] D124441: [Index] [clangd] Support for concept declarations and requires expressions

[Sam McCall via Phabricator via cfe-commits]

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