diff --git a/clang-tools-extra/clangd/SemanticHighlighting.h b/clang-tools-extra/clangd/SemanticHighlighting.h --- a/clang-tools-extra/clangd/SemanticHighlighting.h +++ b/clang-tools-extra/clangd/SemanticHighlighting.h @@ -76,7 +76,12 @@ Static, Abstract, - LastModifier = Abstract + FunctionScope, + ClassScope, + FileScope, + GlobalScope, + + LastModifier = GlobalScope }; static_assert(static_cast(HighlightingModifier::LastModifier) < 32, "Increase width of modifiers bitfield!"); diff --git a/clang-tools-extra/clangd/SemanticHighlighting.cpp b/clang-tools-extra/clangd/SemanticHighlighting.cpp --- a/clang-tools-extra/clangd/SemanticHighlighting.cpp +++ b/clang-tools-extra/clangd/SemanticHighlighting.cpp @@ -353,6 +353,47 @@ HighlightingToken Dummy; // returned from addToken(InvalidLoc) }; +llvm::Optional 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(D)) + if (R->isInjectedClassName()) + DC = DC->getParent(); + // Lambda captures are considered function scope, not class scope. + if (llvm::isa(D)) + if (const auto *RD = llvm::dyn_cast(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. + // Avoid caching linkage if it may change after enclosing code completion. + if (hasUnstableLinkage(D) || D->getLinkageInternal() < ExternalLinkage) + return HighlightingModifier::FileScope; + return HighlightingModifier::GlobalScope; +} + +llvm::Optional scopeModifier(const Type *T) { + if (!T) + return llvm::None; + if (T->isBuiltinType()) + return HighlightingModifier::GlobalScope; + if (auto *TD = dyn_cast(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 @@ -361,9 +402,12 @@ 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; } @@ -371,38 +415,47 @@ 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(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; } @@ -410,6 +463,7 @@ switch (L.getArgument().getKind()) { case TemplateArgument::Template: case TemplateArgument::TemplateExpansion: + // FIXME: I don't understand why this is DependentType. H.addToken(L.getTemplateNameLoc(), HighlightingKind::DependentType); break; default: @@ -426,7 +480,8 @@ 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); } @@ -483,6 +538,8 @@ 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)) @@ -498,6 +555,7 @@ // 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); }; @@ -723,7 +781,16 @@ return "deduced"; // nonstandard case HighlightingModifier::Abstract: return "abstract"; + case HighlightingModifier::FunctionScope: + return "functionScope"; + case HighlightingModifier::ClassScope: + return "classScope"; + case HighlightingModifier::FileScope: + return "fileScope"; + case HighlightingModifier::GlobalScope: + return "globalScope"; } + llvm_unreachable("unhandled HighlightingModifier"); } std::vector diff --git a/clang-tools-extra/clangd/test/initialize-params.test b/clang-tools-extra/clangd/test/initialize-params.test --- 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 --- 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 --- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp +++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp @@ -113,7 +113,8 @@ void checkHighlightings(llvm::StringRef Code, std::vector> - AdditionalFiles = {}) { + AdditionalFiles = {}, + uint32_t ModifierMask = -1) { Annotations Test(Code); TestTU TU; TU.Code = std::string(Test.code()); @@ -126,8 +127,11 @@ 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 @@ << 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( @@ -717,9 +727,10 @@ }; )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]] { @@ -729,7 +740,8 @@ {{"imp.h", R"cpp( int someMethod(); void otherMethod(); - )cpp"}}); + )cpp"}}, + ~ScopeModifierMask); // A separate test for macros in headers. checkHighlightings(R"cpp( @@ -742,7 +754,59 @@ #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 + class $Class_globalScope[[X]] { + }; + )cpp", + R"cpp( + // No useful scope for template parameters of variable templates. + template + 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 --- a/clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp +++ b/clang-tools-extra/clangd/unittests/tweaks/AnnotateHighlightingsTests.cpp @@ -18,17 +18,18 @@ 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