[clang-tools-extra] 46cc7ce - [clangd] Add semanticTokens modifiers for function/class/file/global scope

Sam McCall via cfe-commits cfe-commits at lists.llvm.org
Tue Feb 9 08:57:45 PST 2021


Author: Sam McCall
Date: 2021-02-09T17:57:36+01:00
New Revision: 46cc7ce35ade2d0b3b49301fbf8135c1eca5b048

URL: https://github.com/llvm/llvm-project/commit/46cc7ce35ade2d0b3b49301fbf8135c1eca5b048
DIFF: https://github.com/llvm/llvm-project/commit/46cc7ce35ade2d0b3b49301fbf8135c1eca5b048.diff

LOG: [clangd] Add semanticTokens modifiers for function/class/file/global scope

These allow (function-) local variables to be distinguished, but also a
bunch more cases.
It's not quite independent with existing information (e.g. the
field/variable distinction is redundant if you have class-scope + static
attributes) but I don't think this is terribly important.

Depends on D77811

Differential Revision: https://reviews.llvm.org/D95701

Added: 
    

Modified: 
    clang-tools-extra/clangd/SemanticHighlighting.cpp
    clang-tools-extra/clangd/SemanticHighlighting.h
    clang-tools-extra/clangd/test/initialize-params.test
    clang-tools-extra/clangd/test/semantic-tokens.test
    clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
    clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp

Removed: 
    


################################################################################
diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp
index 00c6ed65d157..29afae3746f8 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -357,6 +357,46 @@ class HighlightingsBuilder {
   HighlightingToken Dummy; // returned from addToken(InvalidLoc)
 };
 
+llvm::Optional<HighlightingModifier> scopeModifier(const NamedDecl *D) {
+  const DeclContext *DC = D->getDeclContext();
+  // Injected "Foo" within the class "Foo" has file scope, not class scope.
+  if (auto *R = dyn_cast_or_null<RecordDecl>(D))
+    if (R->isInjectedClassName())
+      DC = DC->getParent();
+  // Lambda captures are considered function scope, not class scope.
+  if (llvm::isa<FieldDecl>(D))
+    if (const auto *RD = llvm::dyn_cast<RecordDecl>(DC))
+      if (RD->isLambda())
+        return HighlightingModifier::FunctionScope;
+  // Walk up the DeclContext hierarchy until we find something interesting.
+  for (; !DC->isFileContext(); DC = DC->getParent()) {
+    if (DC->isFunctionOrMethod())
+      return HighlightingModifier::FunctionScope;
+    if (DC->isRecord())
+      return HighlightingModifier::ClassScope;
+  }
+  // Some template parameters (e.g. those for variable templates) don't have
+  // meaningful DeclContexts. That doesn't mean they're global!
+  if (DC->isTranslationUnit() && D->isTemplateParameter())
+    return llvm::None;
+  // ExternalLinkage threshold could be tweaked, e.g. module-visible as global.
+  if (D->getLinkageInternal() < ExternalLinkage)
+    return HighlightingModifier::FileScope;
+  return HighlightingModifier::GlobalScope;
+}
+
+llvm::Optional<HighlightingModifier> scopeModifier(const Type *T) {
+  if (!T)
+    return llvm::None;
+  if (T->isBuiltinType())
+    return HighlightingModifier::GlobalScope;
+  if (auto *TD = dyn_cast<TemplateTypeParmType>(T))
+    return scopeModifier(TD->getDecl());
+  if (auto *TD = T->getAsTagDecl())
+    return scopeModifier(TD);
+  return llvm::None;
+}
+
 /// Produces highlightings, which are not captured by findExplicitReferences,
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
@@ -365,9 +405,12 @@ class CollectExtraHighlightings
   CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
   bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
-    if (auto K = kindForType(L.getTypePtr()))
-      H.addToken(L.getBeginLoc(), *K)
-          .addModifier(HighlightingModifier::Deduced);
+    if (auto K = kindForType(L.getTypePtr())) {
+      auto &Tok = H.addToken(L.getBeginLoc(), *K)
+                      .addModifier(HighlightingModifier::Deduced);
+      if (auto Mod = scopeModifier(L.getTypePtr()))
+        Tok.addModifier(*Mod);
+    }
     return true;
   }
 
@@ -375,38 +418,47 @@ class CollectExtraHighlightings
     auto *AT = D->getType()->getContainedAutoType();
     if (!AT)
       return true;
-    if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull()))
-      H.addToken(D->getTypeSpecStartLoc(), *K)
-          .addModifier(HighlightingModifier::Deduced);
+    if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull())) {
+      auto &Tok = H.addToken(D->getTypeSpecStartLoc(), *K)
+                      .addModifier(HighlightingModifier::Deduced);
+      if (auto Mod = scopeModifier(AT->getDeducedType().getTypePtrOrNull()))
+        Tok.addModifier(*Mod);
+    }
     return true;
   }
 
   bool VisitOverloadExpr(OverloadExpr *E) {
     if (!E->decls().empty())
       return true; // handled by findExplicitReferences.
-    H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
+    auto &Tok = H.addToken(E->getNameLoc(), HighlightingKind::DependentName);
+    if (llvm::isa<UnresolvedMemberExpr>(E))
+      Tok.addModifier(HighlightingModifier::ClassScope);
+    // other case is UnresolvedLookupExpr, scope is unknown.
     return true;
   }
 
   bool VisitCXXDependentScopeMemberExpr(CXXDependentScopeMemberExpr *E) {
-    H.addToken(E->getMemberNameInfo().getLoc(),
-               HighlightingKind::DependentName);
+    H.addToken(E->getMemberNameInfo().getLoc(), HighlightingKind::DependentName)
+        .addModifier(HighlightingModifier::ClassScope);
     return true;
   }
 
   bool VisitDependentScopeDeclRefExpr(DependentScopeDeclRefExpr *E) {
-    H.addToken(E->getNameInfo().getLoc(), HighlightingKind::DependentName);
+    H.addToken(E->getNameInfo().getLoc(), HighlightingKind::DependentName)
+        .addModifier(HighlightingModifier::ClassScope);
     return true;
   }
 
   bool VisitDependentNameTypeLoc(DependentNameTypeLoc L) {
-    H.addToken(L.getNameLoc(), HighlightingKind::DependentType);
+    H.addToken(L.getNameLoc(), HighlightingKind::DependentType)
+        .addModifier(HighlightingModifier::ClassScope);
     return true;
   }
 
   bool VisitDependentTemplateSpecializationTypeLoc(
       DependentTemplateSpecializationTypeLoc L) {
-    H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
+    H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType)
+        .addModifier(HighlightingModifier::ClassScope);
     return true;
   }
 
@@ -414,6 +466,7 @@ class CollectExtraHighlightings
     switch (L.getArgument().getKind()) {
     case TemplateArgument::Template:
     case TemplateArgument::TemplateExpansion:
+      // FIXME: this isn't *always* a dependent template name.
       H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType);
       break;
     default:
@@ -430,7 +483,8 @@ class CollectExtraHighlightings
   bool TraverseNestedNameSpecifierLoc(NestedNameSpecifierLoc Q) {
     if (NestedNameSpecifier *NNS = Q.getNestedNameSpecifier()) {
       if (NNS->getKind() == NestedNameSpecifier::Identifier)
-        H.addToken(Q.getLocalBeginLoc(), HighlightingKind::DependentType);
+        H.addToken(Q.getLocalBeginLoc(), HighlightingKind::DependentType)
+            .addModifier(HighlightingModifier::ClassScope);
     }
     return RecursiveASTVisitor::TraverseNestedNameSpecifierLoc(Q);
   }
@@ -484,6 +538,8 @@ std::vector<HighlightingToken> getSemanticHighlightings(ParsedAST &AST) {
         if (auto *Templated = TD->getTemplatedDecl())
           Decl = Templated;
       }
+      if (auto Mod = scopeModifier(Decl))
+        Tok.addModifier(*Mod);
       if (isConst(Decl))
         Tok.addModifier(HighlightingModifier::Readonly);
       if (isStatic(Decl))
@@ -499,6 +555,7 @@ std::vector<HighlightingToken> getSemanticHighlightings(ParsedAST &AST) {
   // Add highlightings for macro references.
   auto AddMacro = [&](const MacroOccurrence &M) {
     auto &T = Builder.addToken(M.Rng, HighlightingKind::Macro);
+    T.addModifier(HighlightingModifier::GlobalScope);
     if (M.IsDefinition)
       T.addModifier(HighlightingModifier::Declaration);
   };
@@ -724,7 +781,16 @@ llvm::StringRef toSemanticTokenModifier(HighlightingModifier Modifier) {
     return "deduced"; // nonstandard
   case HighlightingModifier::Abstract:
     return "abstract";
+  case HighlightingModifier::FunctionScope:
+    return "functionScope"; // nonstandard
+  case HighlightingModifier::ClassScope:
+    return "classScope"; // nonstandard
+  case HighlightingModifier::FileScope:
+    return "fileScope"; // nonstandard
+  case HighlightingModifier::GlobalScope:
+    return "globalScope"; // nonstandard
   }
+  llvm_unreachable("unhandled HighlightingModifier");
 }
 
 std::vector<TheiaSemanticHighlightingInformation>

diff  --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h
index 27db811d98a4..e88835385399 100644
--- a/clang-tools-extra/clangd/SemanticHighlighting.h
+++ b/clang-tools-extra/clangd/SemanticHighlighting.h
@@ -76,7 +76,12 @@ enum class HighlightingModifier {
   Static,
   Abstract,
 
-  LastModifier = Abstract
+  FunctionScope,
+  ClassScope,
+  FileScope,
+  GlobalScope,
+
+  LastModifier = GlobalScope
 };
 static_assert(static_cast<unsigned>(HighlightingModifier::LastModifier) < 32,
               "Increase width of modifiers bitfield!");

diff  --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test
index eae7456fd431..605584187c44 100644
--- a/clang-tools-extra/clangd/test/initialize-params.test
+++ b/clang-tools-extra/clangd/test/initialize-params.test
@@ -88,6 +88,10 @@
 # CHECK-NEXT:            "readonly",
 # CHECK-NEXT:            "static",
 # CHECK-NEXT:            "abstract"
+# CHECK-NEXT:            "functionScope",
+# CHECK-NEXT:            "classScope",
+# CHECK-NEXT:            "fileScope",
+# CHECK-NEXT:            "globalScope"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          "tokenTypes": [
 # CHECK-NEXT:            "variable",

diff  --git a/clang-tools-extra/clangd/test/semantic-tokens.test b/clang-tools-extra/clangd/test/semantic-tokens.test
index e94eb243590a..50897c3c1208 100644
--- a/clang-tools-extra/clangd/test/semantic-tokens.test
+++ b/clang-tools-extra/clangd/test/semantic-tokens.test
@@ -18,12 +18,12 @@
 # CHECK-NEXT:  "jsonrpc": "2.0",
 # CHECK-NEXT:  "result": {
 # CHECK-NEXT:    "data": [
-#                  First line, char 5, variable, declaration
+#                  First line, char 5, variable, declaration+globalScope
 # CHECK-NEXT:      0,
 # CHECK-NEXT:      4,
 # CHECK-NEXT:      1,
 # CHECK-NEXT:      0,
-# CHECK-NEXT:      1
+# CHECK-NEXT:      513
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "resultId": "1"
 # CHECK-NEXT:  }
@@ -44,12 +44,12 @@
 # CHECK-NEXT:    "edits": [
 # CHECK-NEXT:      {
 # CHECK-NEXT:        "data": [
-#                      Next line, char 5, variable, declaration
+#                      Next line, char 5, variable, declaration+globalScope
 # CHECK-NEXT:          1,
 # CHECK-NEXT:          4,
 # CHECK-NEXT:          1,
 # CHECK-NEXT:          0,
-# CHECK-NEXT:          1
+# CHECK-NEXT:          513
 # CHECK-NEXT:        ],
 #                    Inserted at position 1
 # CHECK-NEXT:        "deleteCount": 0,
@@ -72,12 +72,12 @@
 # CHECK-NEXT:      4,
 # CHECK-NEXT:      1,
 # CHECK-NEXT:      0,
-# CHECK-NEXT:      1,
+# CHECK-NEXT:      513,
 # CHECK-NEXT:      1,
 # CHECK-NEXT:      4,
 # CHECK-NEXT:      1,
 # CHECK-NEXT:      0,
-# CHECK-NEXT:      1
+# CHECK-NEXT:      513
 # CHECK-NEXT:    ],
 # CHECK-NEXT:    "resultId": "3"
 # CHECK-NEXT:  }

diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index 385ad1d34553..f2afd5634681 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -113,7 +113,8 @@ std::string annotate(llvm::StringRef Input,
 void checkHighlightings(llvm::StringRef Code,
                         std::vector<std::pair</*FileName*/ llvm::StringRef,
                                               /*FileContent*/ llvm::StringRef>>
-                            AdditionalFiles = {}) {
+                            AdditionalFiles = {},
+                        uint32_t ModifierMask = -1) {
   Annotations Test(Code);
   TestTU TU;
   TU.Code = std::string(Test.code());
@@ -126,8 +127,11 @@ void checkHighlightings(llvm::StringRef Code,
   for (auto File : AdditionalFiles)
     TU.AdditionalFiles.insert({File.first, std::string(File.second)});
   auto AST = TU.build();
+  auto Actual = getSemanticHighlightings(AST);
+  for (auto &Token : Actual)
+    Token.Modifiers &= ModifierMask;
 
-  EXPECT_EQ(Code, annotate(Test.code(), getSemanticHighlightings(AST)));
+  EXPECT_EQ(Code, annotate(Test.code(), Actual));
 }
 
 // Any annotations in OldCode and NewCode are converted into their corresponding
@@ -163,6 +167,12 @@ void checkDiffedHighlights(llvm::StringRef OldCode, llvm::StringRef NewCode) {
       << OldCode;
 }
 
+constexpr static uint32_t ScopeModifierMask =
+    1 << unsigned(HighlightingModifier::FunctionScope) |
+    1 << unsigned(HighlightingModifier::ClassScope) |
+    1 << unsigned(HighlightingModifier::FileScope) |
+    1 << unsigned(HighlightingModifier::GlobalScope);
+
 TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
       R"cpp(
@@ -720,9 +730,10 @@ sizeof...($TemplateParameter[[Elements]]);
       <:[deprecated]:> int $Variable_decl_deprecated[[x]];
       )cpp",
   };
-  for (const auto &TestCase : TestCases) {
-    checkHighlightings(TestCase);
-  }
+  for (const auto &TestCase : TestCases)
+    // Mask off scope modifiers to keep the tests manageable.
+    // They're tested separately.
+    checkHighlightings(TestCase, {}, ~ScopeModifierMask);
 
   checkHighlightings(R"cpp(
     class $Class_decl[[A]] {
@@ -732,7 +743,8 @@ sizeof...($TemplateParameter[[Elements]]);
                      {{"imp.h", R"cpp(
     int someMethod();
     void otherMethod();
-  )cpp"}});
+  )cpp"}},
+                     ~ScopeModifierMask);
 
   // A separate test for macros in headers.
   checkHighlightings(R"cpp(
@@ -745,7 +757,59 @@ sizeof...($TemplateParameter[[Elements]]);
     #define DXYZ_Y(Y) DXYZ(x##Y)
     #define DEFINE(X) int X;
     #define DEFINE_Y DEFINE(Y)
-  )cpp"}});
+  )cpp"}},
+                     ~ScopeModifierMask);
+}
+
+TEST(SemanticHighlighting, ScopeModifiers) {
+  const char *TestCases[] = {
+      R"cpp(
+        static int $Variable_fileScope[[x]];
+        namespace $Namespace_globalScope[[ns]] {
+          class $Class_globalScope[[x]];
+        }
+        namespace {
+          void $Function_fileScope[[foo]]();
+        }
+      )cpp",
+      R"cpp(
+        void $Function_globalScope[[foo]](int $Parameter_functionScope[[y]]) {
+          int $LocalVariable_functionScope[[z]];
+        }
+      )cpp",
+      R"cpp(
+        // Lambdas are considered functions, not classes.
+        auto $Variable_fileScope[[x]] = [m(42)] { // FIXME: annotate capture
+          return $LocalVariable_functionScope[[m]];
+        };
+      )cpp",
+      R"cpp(
+        // Classes in functions are classes.
+        void $Function_globalScope[[foo]]() {
+          class $Class_functionScope[[X]] {
+            int $Field_classScope[[x]];
+          };
+        };
+      )cpp",
+      R"cpp(
+        template <int $TemplateParameter_classScope[[T]]>
+        class $Class_globalScope[[X]] {
+        };
+      )cpp",
+      R"cpp(
+        // No useful scope for template parameters of variable templates.
+        template <typename $TemplateParameter[[A]]>
+        unsigned $Variable_globalScope[[X]] =
+          $TemplateParameter[[A]]::$DependentName_classScope[[x]];
+      )cpp",
+      R"cpp(
+        #define $Macro_globalScope[[X]] 1
+        int $Variable_globalScope[[Y]] = $Macro_globalScope[[X]];
+      )cpp",
+  };
+
+  for (const char *Test : TestCases)
+    checkHighlightings(Test, {}, ScopeModifierMask);
 }
 
 TEST(SemanticHighlighting, GeneratesHighlightsWhenFileChange) {

diff  --git a/clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp b/clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
index 13cfbd923772..e8ccef9947fe 100644
--- a/clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp
@@ -18,17 +18,18 @@ TWEAK_TEST(AnnotateHighlightings);
 TEST_F(AnnotateHighlightingsTest, Test) {
   EXPECT_AVAILABLE("^vo^id^ ^f(^) {^}^"); // available everywhere.
   EXPECT_AVAILABLE("[[int a; int b;]]");
-  EXPECT_EQ("void /* Function [decl] */f() {}", apply("void ^f() {}"));
+  EXPECT_EQ("void /* Function [decl] [globalScope] */f() {}",
+            apply("void ^f() {}"));
 
-  EXPECT_EQ(
-      apply("[[int f1(); const int x = f1();]]"),
-      "int /* Function [decl] */f1(); "
-      "const int /* Variable [decl] [readonly] */x = /* Function */f1();");
+  EXPECT_EQ(apply("[[int f1(); const int x = f1();]]"),
+            "int /* Function [decl] [globalScope] */f1(); "
+            "const int /* Variable [decl] [readonly] [fileScope] */x = "
+            "/* Function [globalScope] */f1();");
 
   // Only the targeted range is annotated.
   EXPECT_EQ(apply("void f1(); void f2() {^}"),
             "void f1(); "
-            "void /* Function [decl] */f2() {}");
+            "void /* Function [decl] [globalScope] */f2() {}");
 }
 
 } // namespace


        


More information about the cfe-commits mailing list