[PATCH] D136925: [clangd] Index scoped enums for code completion

[Tom Praschan via Phabricator via cfe-commits]

tom-anders created this revision.
tom-anders added a reviewer: nridge.
Herald added subscribers: kadircet, arphaman.
Herald added a project: All.
tom-anders requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

As discussed in https://github.com/clangd/clangd/issues/1082:

"Without all-scopes-completion, such nested symbols would indeed be unnecessary in the
index, because the only way you could get them as a completion result is if you're
invoking the completion in the nested scope (e.g. Foo::^), in which case result can be
obtained from the AST."

I tested this on the LLVM codebase and there was no significant increase in index size
(less than 1MB). I think this makes sense if we look at the additional data that is stored
in SymbolCollector::addDeclaration when IndexedForCodeCompletion is set:

- Signature: Empty string for enum constants
- SnippetSuffix: Empty
- Documentation: Empty most of the time
- ReturnType and Type: Same string for all enum constants within a enum, so only two additional strings per indexed enum


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D136925

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/CodeComplete.h
  clang-tools-extra/clangd/index/Serialization.cpp
  clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp


Index: clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
+++ clang-tools-extra/clangd/unittests/SymbolCollectorTests.cpp
@@ -1316,6 +1316,11 @@
       Black
     };
     }
+    class Color3 {
+      enum {
+        Blue
+      };
+    };
   )";
   runSymbolCollector(Header, /*Main=*/"");
   EXPECT_THAT(Symbols,
@@ -1324,7 +1329,9 @@
                   AllOf(qName("Color"), forCodeCompletion(true)),
                   AllOf(qName("Green"), forCodeCompletion(true)),
                   AllOf(qName("Color2"), forCodeCompletion(true)),
-                  AllOf(qName("Color2::Yellow"), forCodeCompletion(false)),
+                  AllOf(qName("Color2::Yellow"), forCodeCompletion(true)),
+                  AllOf(qName("Color3"), forCodeCompletion(true)),
+                  AllOf(qName("Color3::Blue"), forCodeCompletion(true)),
                   AllOf(qName("ns"), forCodeCompletion(true)),
                   AllOf(qName("ns::Black"), forCodeCompletion(true))));
 }
Index: clang-tools-extra/clangd/index/Serialization.cpp
===================================================================
--- clang-tools-extra/clangd/index/Serialization.cpp
+++ clang-tools-extra/clangd/index/Serialization.cpp
@@ -455,7 +455,7 @@
 // The current versioning scheme is simple - non-current versions are rejected.
 // If you make a breaking change, bump this version number to invalidate stored
 // data. Later we may want to support some backward compatibility.
-constexpr static uint32_t Version = 17;
+constexpr static uint32_t Version = 18;
 
 llvm::Expected<IndexFileIn> readRIFF(llvm::StringRef Data,
                                      SymbolOrigin Origin) {
Index: clang-tools-extra/clangd/CodeComplete.h
===================================================================
--- clang-tools-extra/clangd/CodeComplete.h
+++ clang-tools-extra/clangd/CodeComplete.h
@@ -291,7 +291,7 @@
 // For index-based completion, we only consider:
 //   * symbols in namespaces or translation unit scopes (e.g. no class
 //     members, no locals)
-//   * enum constants in unscoped enum decl (e.g. "red" in "enum {red};")
+//   * enum constants (both scoped and unscoped)
 //   * primary templates (no specializations)
 // For the other cases, we let Clang do the completion because it does not
 // need any non-local information and it will be much better at following
Index: clang-tools-extra/clangd/CodeComplete.cpp
===================================================================
--- clang-tools-extra/clangd/CodeComplete.cpp
+++ clang-tools-extra/clangd/CodeComplete.cpp
@@ -2129,8 +2129,10 @@
   if (InTopLevelScope(ND))
     return true;
 
+  // Always index enum constants, even if they're not in the top level scope: when
+  // --all-scopes-completion is set, we'll want to complete those as well.
   if (const auto *EnumDecl = dyn_cast<clang::EnumDecl>(ND.getDeclContext()))
-    return InTopLevelScope(*EnumDecl) && !EnumDecl->isScoped();
+    return true;
 
   return false;
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D136925.471438.patch
Type: text/x-patch
Size: 3118 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20221028/606d344a/attachment-0001.bin>