[clangd] Add support for the `defaultLibrary` semantic token modifier

[dgoldman]

Summary

This allows us to differentiate symbols from the system (e.g. system
includes or sysroot) differently than symbols defined in the user's
project, which can be used by editors to display them differently.

This is currently based on FileCharacteristic, but we can
consider alternatives such as Sysroot and file paths in the future.

[dgoldman]

Fix casing warnings

[kadircet]

clang-tools-extra/clangd/SemanticHighlighting.cpp

413

you can make this a free function by accepting a decl instead. as sourcemanager is accessible via D->getASTContext().getSourceManager(). this way you don't need to modify signatures for scopeModifier overloads (and get rid of the duplicate in CollectExtraHighlightings).

433

why the null check now?

436

I don't use semantic highlighting enough to judge whether it is more useful to say DefaultLibrary than {Class,Function}Scope. (i.e. having the check here vs at the end of the function as a fallback). Can you provide some rationale about the decision (probably as comments in code)?

By looking at the testcase, I suppose you are trying to indicate "overriden"/"extended" attribute of a symbol, which makes sense but would be nice to know if there's more. I am just worried that this is very obj-c specific and won't really generalize to c/c++. As most of the system headers only provide platform specific structs (e.g. posix) and they are almost never extended. So it might be confusing for user to see different colors on some memberexprs.

474

should this be put in a separate patch?

[sammccall]

clang-tools-extra/clangd/SemanticHighlighting.cpp

436

I think default-library isn't a scope modifier at all, it should be independent.

e.g. std::string is defaultlLibrary | globalScope, while std::string::push_back() is defaultLibrary | classScope.

465

I'm confused about the interface change - looks like the new parameter never actually used, just passed around

clang-tools-extra/clangd/SemanticHighlighting.h

75

This isn't a scope modifier so shouldn't be grouped with them

[dgoldman]

clang-tools-extra/clangd/SemanticHighlighting.cpp

413

TIL decls can reference the ASTContext. I can swap over to that if you'd like, the one advantage the current approach has is that we can easily add a caching layer into the Builder to say cache the results by FileID (but I'm not sure how useful/necessary such caching would be)

436

I can break it up if you'd like - just let me know what you had in mind. I can change the scopeModifier function's name and then have it return a list of modifiers or I can just add a new function which all existing callers of scopeModifier will also need to call.

And what we're really trying to do here is color the system/SDK symbols differently - this is definitely useful for iOS dev since Apple has a very large SDK, but if it's not useful for C++ maybe we could just make this configurable?

465

It's used (currently) as the HighlightingsBuilder is the one who checks for default/systemness

474

If you want I can move it over

[sammccall]

clang-tools-extra/clangd/SemanticHighlighting.cpp

436

just let me know what you had in mind

I think this should follow isStatic() etc: just a boolean function that gets called in the minimum number of places needed.

scopeModifier is slightly different pattern because there are a bunch of mutually-exclusive modifiers. defaultLibrary is (mostly) independent of other modifiers.

if it's not useful for C++ maybe we could just make this configurable

No need really - sending more modifiers is cheap (one string at startup and one bit in an integer we're sending anyway).
It's up to clients to style the ones they care about, and at least in VSCode this can be customized per-lang I think. Important thing is that they don't interact with other modifiers.

465

Oops yes, in the other overload.

Please do remove this as Kadir suggests, it's noisy and makes inputs/outputs less clear.

[dgoldman]

Split defaultLibrary check to separate function

[dgoldman]

clang-tools-extra/clangd/SemanticHighlighting.cpp

433

removed and check for it when visit an obj method expr

436

Swapped over, currently don't check on deduced types but we could add our isDefaultLibrary check there as well

[dgoldman]

Remove unnecessary null check

[kadircet]

LG from my side.

clang-tools-extra/clangd/SemanticHighlighting.cpp

475

is this guaranteed to be non-null ?

[dgoldman]

Remove unnecessary scope modifier

[dgoldman]

clang-tools-extra/clangd/SemanticHighlighting.cpp

475

Went ahead and removed it, it's unnecessary now

[sammccall]

clang-tools-extra/clangd/SemanticHighlighting.cpp

436

I think it'd be useful to do this for consistency, just VisitDecltypeTypeLoc and VisitDeclaratorDecl need to be changed I think.

We'd want a version of isDefaultLibrary that works on Type. We're only going to see canonical types so I think we need to:

• unwrap pointers (getPointeeOrArrayElementType)
• if BuiltinType -> return true
• for types: find the decl and dispatch to isDefaultLibrary(Decl)

to find the decl it'd be nice to use targetDecl() but also sufficient to handle TagType + ObjCInterfaceType for now I think.

[dgoldman]

Add isDefaultLibrary(const Type *) and support auto/decl types

[dgoldman]

clang-tools-extra/clangd/SemanticHighlighting.cpp

436

Done, I think this is fine for a v1 - we don't see much auto/decltype use in objc, can improve it later

clang-tools-extra/clangd/SemanticHighlighting.h

enum class HighlightingModifier {
   Static,
   Abstract,
   DependentName,
 
   FunctionScope,
   ClassScope,
   FileScope,
   GlobalScope,
+  DefaultLibrary,
 
-  LastModifier = GlobalScope
+  LastModifier = DefaultLibrary
 };
 static_assert(static_cast<unsigned>(HighlightingModifier::LastModifier) < 32,
               "Increase width of modifiers bitfield!");
 llvm::raw_ostream &operator<<(llvm::raw_ostream &OS, HighlightingModifier K);
 
 // Contains all information needed for the highlighting a token.
 struct HighlightingToken {
   HighlightingKind Kind;

clang-tools-extra/clangd/SemanticHighlighting.cpp

for (const Range &R : SortedSkippedRanges) {
       }
     }
     // Copy tokens after the last inactive line
     for (; It != NonConflicting.end(); ++It)
       WithInactiveLines.push_back(std::move(*It));
     return WithInactiveLines;
   }
 
+  /// Returns true if `Loc` is considered to be from a default/system library.
+  /// This currently checks the systemness of the file by include type, although
+  /// different heuristics may be used in the future (e.g. sysroot paths).
+  bool InDefaultLibrary(SourceLocation Loc) const {
+    if (!Loc.isValid())
+      return false;
+    return SourceMgr.isInSystemHeader(Loc);
+  }
+
   const HeuristicResolver *getResolver() const { return Resolver; }
 
 private:
   const syntax::TokenBuffer &TB;
   const SourceManager &SourceMgr;
   const LangOptions &LangOpts;
   std::vector<HighlightingToken> Tokens;
   const HeuristicResolver *Resolver;
   // returned from addToken(InvalidLoc)
   HighlightingToken InvalidHighlightingToken;
 };
 
-llvm::Optional<HighlightingModifier> scopeModifier(const NamedDecl *D) {
+llvm::Optional<HighlightingModifier>
+scopeModifier(const NamedDecl *D, const HighlightingsBuilder &HB) {
+  if (!D)
+    return llvm::None;
+  if (HB.InDefaultLibrary(D->getLocation()))
+    return HighlightingModifier::DefaultLibrary;
   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))
scopeModifier(const NamedDecl *D, const HighlightingsBuilder &HB) {
   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) {
+llvm::Optional<HighlightingModifier>
+scopeModifier(const Type *T, const HighlightingsBuilder &HB) {
   if (!T)
     return llvm::None;
   if (T->isBuiltinType())
-    return HighlightingModifier::GlobalScope;
+    return HighlightingModifier::DefaultLibrary;
   if (auto *TD = dyn_cast<TemplateTypeParmType>(T))
-    return scopeModifier(TD->getDecl());
+    return scopeModifier(TD->getDecl(), HB);
   if (auto *TD = T->getAsTagDecl())
-    return scopeModifier(TD);
+    return scopeModifier(TD, HB);
+  if (auto *OT = dyn_cast<ObjCObjectType>(T))
+    return scopeModifier(OT->getInterface(), HB);
   return llvm::None;
 }
 
 /// Produces highlightings, which are not captured by findExplicitReferences,
 /// e.g. highlights dependent names and 'auto' as the underlying type.
 class CollectExtraHighlightings
     : public RecursiveASTVisitor<CollectExtraHighlightings> {
 public:
   CollectExtraHighlightings(HighlightingsBuilder &H) : H(H) {}
 
+  bool IsFromDefaultLibrary(const NamedDecl *D) {
+    if (!D)
+      return false;
+    return H.InDefaultLibrary(D->getLocation());
+  }
+
   bool VisitDecltypeTypeLoc(DecltypeTypeLoc L) {
     if (auto K = kindForType(L.getTypePtr(), H.getResolver())) {
       auto &Tok = H.addToken(L.getBeginLoc(), *K)
                       .addModifier(HighlightingModifier::Deduced);
-      if (auto Mod = scopeModifier(L.getTypePtr()))
+      if (auto Mod = scopeModifier(L.getTypePtr(), H))
         Tok.addModifier(*Mod);
     }
     return true;
   }
 
   bool VisitDeclaratorDecl(DeclaratorDecl *D) {
     auto *AT = D->getType()->getContainedAutoType();
     if (!AT)
       return true;
     if (auto K = kindForType(AT->getDeducedType().getTypePtrOrNull(),
                              H.getResolver())) {
       auto &Tok = H.addToken(D->getTypeSpecStartLoc(), *K)
                       .addModifier(HighlightingModifier::Deduced);
-      if (auto Mod = scopeModifier(AT->getDeducedType().getTypePtrOrNull()))
+      if (auto Mod = scopeModifier(AT->getDeducedType().getTypePtrOrNull(), H))
         Tok.addModifier(*Mod);
     }
     return true;
   }
 
   // We handle objective-C selectors specially, because one reference can
   // cover several non-contiguous tokens.
   void highlightObjCSelector(const ArrayRef<SourceLocation> &Locs, bool Decl,
-                             bool Class) {
+                             bool Class, bool DefaultLibrary) {
     HighlightingKind Kind =
         Class ? HighlightingKind::StaticMethod : HighlightingKind::Method;
     for (SourceLocation Part : Locs) {
       auto &Tok =
           H.addToken(Part, Kind).addModifier(HighlightingModifier::ClassScope);
       if (Decl)
         Tok.addModifier(HighlightingModifier::Declaration);
       if (Class)
         Tok.addModifier(HighlightingModifier::Static);
+      if (DefaultLibrary)
+        Tok.addModifier(HighlightingModifier::DefaultLibrary);
     }
   }
 
   bool VisitObjCMethodDecl(ObjCMethodDecl *OMD) {
     llvm::SmallVector<SourceLocation> Locs;
     OMD->getSelectorLocs(Locs);
-    highlightObjCSelector(Locs, /*Decl=*/true, OMD->isClassMethod());
+    highlightObjCSelector(Locs, /*Decl=*/true, OMD->isClassMethod(),
+                          IsFromDefaultLibrary(OMD));
     return true;
   }
 
   bool VisitObjCMessageExpr(ObjCMessageExpr *OME) {
     llvm::SmallVector<SourceLocation> Locs;
     OME->getSelectorLocs(Locs);
-    highlightObjCSelector(Locs, /*Decl=*/false, OME->isClassMessage());
+    highlightObjCSelector(Locs, /*Decl=*/false, OME->isClassMessage(),
+                          IsFromDefaultLibrary(OME->getMethodDecl()));
     return true;
   }
 
   bool VisitOverloadExpr(OverloadExpr *E) {
     if (!E->decls().empty())
       return true; // handled by findExplicitReferences.
     auto &Tok = H.addToken(E->getNameLoc(), HighlightingKind::Unknown)
                     .addModifier(HighlightingModifier::DependentName);
findExplicitReferences(
             continue;
           auto &Tok = Builder.addToken(R.NameLoc, *Kind);
 
           // The attribute tests don't want to look at the template.
           if (auto *TD = dyn_cast<TemplateDecl>(Decl)) {
             if (auto *Templated = TD->getTemplatedDecl())
               Decl = Templated;
           }
-          if (auto Mod = scopeModifier(Decl))
+          if (auto Mod = scopeModifier(Decl, Builder))
             Tok.addModifier(*Mod);
           if (isConst(Decl))
             Tok.addModifier(HighlightingModifier::Readonly);
           if (isStatic(Decl))
             Tok.addModifier(HighlightingModifier::Static);
           if (isAbstract(Decl))
             Tok.addModifier(HighlightingModifier::Abstract);
           if (isDependent(Decl))
llvm::StringRef toSemanticTokenModifier(HighlightingModifier Modifier) {
   case HighlightingModifier::FunctionScope:
     return "functionScope"; // nonstandard
   case HighlightingModifier::ClassScope:
     return "classScope"; // nonstandard
   case HighlightingModifier::FileScope:
     return "fileScope"; // nonstandard
   case HighlightingModifier::GlobalScope:
     return "globalScope"; // nonstandard
+  case HighlightingModifier::DefaultLibrary:
+    return "defaultLibrary";
   }
   llvm_unreachable("unhandled HighlightingModifier");
 }
 
 std::vector<SemanticTokensEdit>
 diffTokens(llvm::ArrayRef<SemanticToken> Old,
            llvm::ArrayRef<SemanticToken> New) {
   // For now, just replace everything from the first-last modification.

clang-tools-extra/clangd/test/initialize-params.test

 # CHECK-NEXT:            "deduced",
 # CHECK-NEXT:            "readonly",
 # CHECK-NEXT:            "static",
 # CHECK-NEXT:            "abstract",
 # CHECK-NEXT:            "dependentName",
 # CHECK-NEXT:            "functionScope",
 # CHECK-NEXT:            "classScope",
 # CHECK-NEXT:            "fileScope",
-# CHECK-NEXT:            "globalScope"
+# CHECK-NEXT:            "globalScope",
+# CHECK-NEXT:            "defaultLibrary"
 # CHECK-NEXT:          ],
 # CHECK-NEXT:          "tokenTypes": [
 # CHECK-NEXT:            "variable",
 # CHECK:               ]
 # CHECK-NEXT:        },
 # CHECK-NEXT:        "range": false
 # CHECK-NEXT:      },
 # CHECK-NEXT:      "signatureHelpProvider": {

clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp

std::string annotate(llvm::StringRef Input,
   OS << Input.substr(NextChar);
   return std::move(OS.str());
 }
 
 void checkHighlightings(llvm::StringRef Code,
                         std::vector<std::pair</*FileName*/ llvm::StringRef,
                                               /*FileContent*/ llvm::StringRef>>
                             AdditionalFiles = {},
-                        uint32_t ModifierMask = -1) {
+                        uint32_t ModifierMask = -1,
+                        std::vector<std::string> AdditionalArgs = {}) {
   Annotations Test(Code);
   TestTU TU;
   TU.Code = std::string(Test.code());
 
   TU.ExtraArgs.push_back("-std=c++20");
   TU.ExtraArgs.push_back("-xobjective-c++");
+  TU.ExtraArgs.insert(std::end(TU.ExtraArgs), std::begin(AdditionalArgs),
+                      std::end(AdditionalArgs));
 
   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;
 
TEST(SemanticHighlighting, GetsCorrectTokens) {
   const char *TestCases[] = {
       R"cpp(
       struct $Class_decl[[AS]] {
         double $Field_decl[[SomeMember]];
       };
       struct {
       } $Variable_decl[[S]];
       void $Function_decl[[foo]](int $Parameter_decl[[A]], $Class[[AS]] $Parameter_decl[[As]]) {
-        $Primitive_deduced[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
+        $Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[VeryLongVariableName]] = 12312;
         $Class[[AS]]     $LocalVariable_decl[[AA]];
-        $Primitive_deduced[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
+        $Primitive_deduced_defaultLibrary[[auto]] $LocalVariable_decl[[L]] = $LocalVariable[[AA]].$Field[[SomeMember]] + $Parameter[[A]];
         auto $LocalVariable_decl[[FN]] = [ $LocalVariable[[AA]]](int $Parameter_decl[[A]]) -> void {};
         $LocalVariable[[FN]](12312);
       }
     )cpp",
       R"cpp(
       void $Function_decl[[foo]](int);
       void $Function_decl[[Gah]]();
       void $Function_decl[[foo]]() {
)cpp",
         $EnumConstant_decl_readonly[[E]],
       };
       class $Class_decl[[Foo]] {};
       $Enum_deduced[[auto]] $Variable_decl[[AE]] = $Enum[[E]]::$EnumConstant_readonly[[E]];
       $Class_deduced[[auto]] $Variable_decl[[AF]] = $Class[[Foo]]();
       $Class_deduced[[decltype]](auto) $Variable_decl[[AF2]] = $Class[[Foo]]();
       $Class_deduced[[auto]] *$Variable_decl[[AFP]] = &$Variable[[AF]];
       $Enum_deduced[[auto]] &$Variable_decl[[AER]] = $Variable[[AE]];
-      $Primitive_deduced[[auto]] $Variable_decl[[Form]] = 10.2 + 2 * 4;
-      $Primitive_deduced[[decltype]]($Variable[[Form]]) $Variable_decl[[F]] = 10;
+      $Primitive_deduced_defaultLibrary[[auto]] $Variable_decl[[Form]] = 10.2 + 2 * 4;
+      $Primitive_deduced_defaultLibrary[[decltype]]($Variable[[Form]]) $Variable_decl[[F]] = 10;
       auto $Variable_decl[[Fun]] = []()->void{};
     )cpp",
       R"cpp(
       class $Class_decl[[G]] {};
       template<$Class[[G]] *$TemplateParameter_decl_readonly[[U]]>
       class $Class_decl[[GP]] {};
       template<$Class[[G]] &$TemplateParameter_decl_readonly[[U]]>
       class $Class_decl[[GR]] {};
sizeof...($TemplateParameter[[Elements]]);
   )cpp",
                      {{"imp.h", R"cpp(
     #define DXYZ(X) class X {};
     #define DXYZ_Y(Y) DXYZ(x##Y)
     #define DEFINE(X) int X;
     #define DEFINE_Y DEFINE(Y)
   )cpp"}},
                      ~ScopeModifierMask);
+
+  checkHighlightings(R"cpp(
+    #include "SYSObject.h"
+    @interface $Class_defaultLibrary[[SYSObject]] ($Namespace_decl[[UserCategory]])
+    @property(nonatomic, readonly) int $Field_decl_readonly[[user_property]];
+    @end
+    int $Function_decl[[somethingUsingSystemSymbols]]() {
+      $Class_defaultLibrary[[SYSObject]] *$LocalVariable_decl[[obj]] = [$Class_defaultLibrary[[SYSObject]] $StaticMethod_static_defaultLibrary[[new]]];
+      return $LocalVariable[[obj]].$Field_defaultLibrary[[value]] + $LocalVariable[[obj]].$Field_readonly[[user_property]];
+    }
+  )cpp",
+                     {{"SystemSDK/SYSObject.h", R"cpp(
+    @interface SYSObject
+    @property(nonatomic, assign) int value;
+    + (instancetype)new;
+    @end
+  )cpp"}},
+                     ~ScopeModifierMask, {"-isystemSystemSDK/"});
 }
 
 TEST(SemanticHighlighting, ScopeModifiers) {
   const char *TestCases[] = {
       R"cpp(
         static int $Variable_fileScope[[x]];
         namespace $Namespace_globalScope[[ns]] {
           class $Class_globalScope[[x]];