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

Main use of these is in the standard library, where they generally clutter up
the index.

Certain macros are also common, we don't touch indexing of macros in this patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D115301

Files:
  clang-tools-extra/clangd/AST.cpp
  clang-tools-extra/clangd/AST.h
  clang-tools-extra/clangd/Quality.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/index/FileIndex.cpp
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/clangd/unittests/ASTTests.cpp
  clang-tools-extra/clangd/unittests/QualityTests.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.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
@@ -17,7 +17,6 @@
 #include "clang/Index/IndexingOptions.h"
 #include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
-#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/MemoryBuffer.h"
 #include "llvm/Support/VirtualFileSystem.h"
@@ -1849,6 +1848,22 @@
               UnorderedElementsAre(QName("Foo"), QName("Foo::fun:")));
 }
 
+TEST_F(SymbolCollectorTest, Reserved) {
+  const char *Header = R"cpp(
+    void __foo();
+    namespace _X { int secret; }
+  )cpp";
+
+  CollectorOpts.CollectReserved = true;
+  runSymbolCollector("", Header);
+  EXPECT_THAT(Symbols, UnorderedElementsAre(QName("__foo"), QName("_X"),
+                                            QName("_X::secret")));
+
+  CollectorOpts.CollectReserved = false;
+  runSymbolCollector("", Header); //
+  EXPECT_THAT(Symbols, IsEmpty());
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -315,6 +315,16 @@
   }
 }
 
+TEST(SourceCodeTests, isReservedName) {
+  EXPECT_FALSE(isReservedName(""));
+  EXPECT_FALSE(isReservedName("_"));
+  EXPECT_FALSE(isReservedName("foo"));
+  EXPECT_FALSE(isReservedName("_foo"));
+  EXPECT_TRUE(isReservedName("__foo"));
+  EXPECT_TRUE(isReservedName("_Foo"));
+  EXPECT_FALSE(isReservedName("foo__bar")) << "FIXME";
+}
+
 TEST(SourceCodeTests, CollectIdentifiers) {
   auto Style = format::getLLVMStyle();
   auto IDs = collectIdentifiers(R"cpp(
Index: clang-tools-extra/clangd/unittests/QualityTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/QualityTests.cpp
+++ clang-tools-extra/clangd/unittests/QualityTests.cpp
@@ -40,8 +40,6 @@
 
 TEST(QualityTests, SymbolQualitySignalExtraction) {
   auto Header = TestTU::withHeaderCode(R"cpp(
-    int _X;
-
     [[deprecated]]
     int _f() { return _X; }
 
@@ -54,13 +52,6 @@
   auto AST = Header.build();
 
   SymbolQualitySignals Quality;
-  Quality.merge(findSymbol(Symbols, "_X"));
-  EXPECT_FALSE(Quality.Deprecated);
-  EXPECT_FALSE(Quality.ImplementationDetail);
-  EXPECT_TRUE(Quality.ReservedName);
-  EXPECT_EQ(Quality.References, SymbolQualitySignals().References);
-  EXPECT_EQ(Quality.Category, SymbolQualitySignals::Variable);
-
   Quality.merge(findSymbol(Symbols, "X_Y_Decl"));
   EXPECT_TRUE(Quality.ImplementationDetail);
 
Index: clang-tools-extra/clangd/unittests/ASTTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/ASTTests.cpp
+++ clang-tools-extra/clangd/unittests/ASTTests.cpp
@@ -427,6 +427,29 @@
               Contains(attrKind(attr::Unlikely)));
 }
 
+TEST(ClangdAST, HasReservedName) {
+  ParsedAST AST = TestTU::withCode(R"cpp(
+    void __foo();
+    namespace std {
+      inline namespace __1 { class error_code; }
+      namespace __detail { int secret; }
+    }
+  )cpp")
+                      .build();
+
+  EXPECT_TRUE(hasReservedName(findUnqualifiedDecl(AST, "__foo")));
+  EXPECT_FALSE(
+      hasReservedScope(*findUnqualifiedDecl(AST, "__foo").getDeclContext()));
+
+  EXPECT_FALSE(hasReservedName(findUnqualifiedDecl(AST, "error_code")));
+  EXPECT_FALSE(hasReservedScope(
+      *findUnqualifiedDecl(AST, "error_code").getDeclContext()));
+
+  EXPECT_FALSE(hasReservedName(findUnqualifiedDecl(AST, "secret")));
+  EXPECT_TRUE(
+      hasReservedScope(*findUnqualifiedDecl(AST, "secret").getDeclContext()));
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clang-tools-extra/clangd/index/SymbolCollector.h
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.h
+++ clang-tools-extra/clangd/index/SymbolCollector.h
@@ -81,6 +81,9 @@
     bool CollectMainFileSymbols = true;
     /// Collect references to main-file symbols.
     bool CollectMainFileRefs = false;
+    /// Collect symbols with reserved names, like __Vector_base.
+    /// This does not currently affect macros (many like _WIN32 are important!)
+    bool CollectReserved = false;
     /// If set to true, SymbolCollector will collect doc for all symbols.
     /// Note that documents of symbols being indexed for completion will always
     /// be collected regardless of this option.
Index: clang-tools-extra/clangd/index/SymbolCollector.cpp
===================================================================
--- clang-tools-extra/clangd/index/SymbolCollector.cpp
+++ clang-tools-extra/clangd/index/SymbolCollector.cpp
@@ -357,6 +357,10 @@
   // Avoid indexing internal symbols in protobuf generated headers.
   if (isPrivateProtoDecl(ND))
     return false;
+  if (!Opts.CollectReserved &&
+      (hasReservedName(ND) || hasReservedScope(*ND.getDeclContext())))
+    return false;
+
   return true;
 }
 
Index: clang-tools-extra/clangd/index/FileIndex.cpp
===================================================================
--- clang-tools-extra/clangd/index/FileIndex.cpp
+++ clang-tools-extra/clangd/index/FileIndex.cpp
@@ -56,6 +56,9 @@
   CollectorOpts.CountReferences = false;
   CollectorOpts.Origin = SymbolOrigin::Dynamic;
   CollectorOpts.CollectMainFileRefs = CollectMainFileRefs;
+  // We want stdlib implementation details in the index only if we've opened the
+  // file in question. This does means xrefs won't work, though.
+  CollectorOpts.CollectReserved = IsIndexMainAST;
 
   index::IndexingOptions IndexOpts;
   // We only need declarations, because we don't count references.
Index: clang-tools-extra/clangd/SourceCode.h
===================================================================
--- clang-tools-extra/clangd/SourceCode.h
+++ clang-tools-extra/clangd/SourceCode.h
@@ -16,6 +16,7 @@
 #include "Protocol.h"
 #include "support/Context.h"
 #include "support/ThreadsafeFS.h"
+#include "clang/Basic/CharInfo.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/LangOptions.h"
 #include "clang/Basic/SourceLocation.h"
@@ -27,7 +28,6 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Error.h"
-#include "llvm/Support/SHA1.h"
 #include <string>
 
 namespace clang {
@@ -330,6 +330,13 @@
 bool isSelfContainedHeader(const FileEntry *FE, FileID ID,
                            const SourceManager &SM, HeaderSearch &HeaderInfo);
 
+/// Returns true if Name is reserved, like _Foo or __Vector_base.
+inline bool isReservedName(llvm::StringRef Name) {
+  // This doesn't catch all cases, but the most common.
+  return Name.size() >= 2 && Name[0] == '_' &&
+         (isUppercase(Name[1]) || Name[1] == '_');
+}
+
 } // namespace clangd
 } // namespace clang
 #endif
Index: clang-tools-extra/clangd/Quality.cpp
===================================================================
--- clang-tools-extra/clangd/Quality.cpp
+++ clang-tools-extra/clangd/Quality.cpp
@@ -24,7 +24,6 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/SmallVector.h"
-#include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/FormatVariadic.h"
@@ -35,11 +34,6 @@
 
 namespace clang {
 namespace clangd {
-static bool isReserved(llvm::StringRef Name) {
-  // FIXME: Should we exclude _Bool and others recognized by the standard?
-  return Name.size() >= 2 && Name[0] == '_' &&
-         (isUppercase(Name[1]) || Name[1] == '_');
-}
 
 static bool hasDeclInMainFile(const Decl &D) {
   auto &SourceMgr = D.getASTContext().getSourceManager();
@@ -188,9 +182,10 @@
   if (SemaCCResult.Declaration) {
     ImplementationDetail |= isImplementationDetail(SemaCCResult.Declaration);
     if (auto *ID = SemaCCResult.Declaration->getIdentifier())
-      ReservedName = ReservedName || isReserved(ID->getName());
+      ReservedName = ReservedName || isReservedName(ID->getName());
   } else if (SemaCCResult.Kind == CodeCompletionResult::RK_Macro)
-    ReservedName = ReservedName || isReserved(SemaCCResult.Macro->getName());
+    ReservedName =
+        ReservedName || isReservedName(SemaCCResult.Macro->getName());
 }
 
 void SymbolQualitySignals::merge(const Symbol &IndexResult) {
@@ -198,7 +193,7 @@
   ImplementationDetail |= (IndexResult.Flags & Symbol::ImplementationDetail);
   References = std::max(IndexResult.References, References);
   Category = categorize(IndexResult.SymInfo);
-  ReservedName = ReservedName || isReserved(IndexResult.Name);
+  ReservedName = ReservedName || isReservedName(IndexResult.Name);
 }
 
 float SymbolQualitySignals::evaluateHeuristics() const {
Index: clang-tools-extra/clangd/AST.h
===================================================================
--- clang-tools-extra/clangd/AST.h
+++ clang-tools-extra/clangd/AST.h
@@ -74,6 +74,12 @@
 // `MyClass()`, `MyClass(Category)`, and `MyProtocol`.
 std::string printObjCContainer(const ObjCContainerDecl &C);
 
+/// Returns true if this is a NamedDecl with a reserved name.
+bool hasReservedName(const Decl &);
+/// Returns true if this scope would be written with a reserved name.
+/// This does not include unwritten scope elements like __1 in std::__1::vector.
+bool hasReservedScope(const DeclContext &);
+
 /// Gets the symbol ID for a declaration. Returned SymbolID might be null.
 SymbolID getSymbolID(const Decl *D);
 
Index: clang-tools-extra/clangd/AST.cpp
===================================================================
--- clang-tools-extra/clangd/AST.cpp
+++ clang-tools-extra/clangd/AST.cpp
@@ -376,6 +376,24 @@
   return OS.str();
 }
 
+bool hasReservedName(const Decl &D) {
+  if (const auto *ND = llvm::dyn_cast<NamedDecl>(&D))
+    if (const auto *II = ND->getIdentifier())
+      return isReservedName(II->getName());
+  return false;
+}
+
+bool hasReservedScope(const DeclContext &DC) {
+  for (const DeclContext *D = &DC; D; D = D->getParent()) {
+    if (D->isTransparentContext() || D->isInlineNamespace())
+      continue;
+    if (const auto *ND = llvm::dyn_cast<NamedDecl>(D))
+      if (hasReservedName(*ND))
+        return true;
+  }
+  return false;
+}
+
 QualType declaredType(const TypeDecl *D) {
   if (const auto *CTSD = llvm::dyn_cast<ClassTemplateSpecializationDecl>(D))
     if (const auto *TSI = CTSD->getTypeAsWritten())
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to