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

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.


================
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,
----------------
sammccall wrote:
> 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)
The problem is that we currently add the template brackets everywhere.
I forgot to mention that I didn't solve this issue yet, it requires adding a new completion context and seems like a good independent change.

I've been collecting missing cases in the github issue, will make a note there.


================
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);
----------------
sammccall wrote:
> nit: does anything break if we just set c++20 unconditionally?
Yeah, good point. Other tests don't depend on it.


================
Comment at: clang-tools-extra/clangd/unittests/XRefsTests.cpp:2080
+
+    [[IsSmall]] auto i = 8;
+    template<[[IsSmal^l]] U> void foo();
----------------
sammccall wrote:
> maybe `= 'c'` and use `char` below? Don't make me think :-)
Done. Just to make sure we're safe against new architectures for the next 250 years of C++ history.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124441