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

- highlight references to protocols in class/protocol/extension decls
- support multi-token selector highlights in semantic + xref highlights (method 
calls and declarations only)
- In `@interface I(C)`, I now references the interface and C the category
- highlight uses of interfaces as types
- added semantic highlightings of protocol names (as "interface") and category 
names (as "namespace"). These are both standard kinds, maybe "extension" will 
be standardized...
- highlight `auto` as "class" when it resolves to an ObjC pointer
- don't highlight `self` as a variable even though the AST models it as one

Not fixed: uses of protocols in type names (needs some refactoring of
unrelated code first)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D97617

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/SemanticHighlighting.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp

Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -115,10 +115,23 @@
           f.[[^~]]Foo();
         }
       )cpp",
+      R"cpp(// ObjC methods with split selectors.
+        @interface Foo
+          +(void) [[x]]:(int)a [[y]]:(int)b;
+        @end
+        @implementation Foo
+          +(void) [[x]]:(int)a [[y]]:(int)b {}
+        @end
+        void go() {
+          [Foo [[x]]:2 [[^y]]:4];
+        }
+      )cpp",
   };
   for (const char *Test : Tests) {
     Annotations T(Test);
-    auto AST = TestTU::withCode(T.code()).build();
+    auto TU = TestTU::withCode(T.code());
+    TU.ExtraArgs.push_back("-xobjective-c++");
+    auto AST = TU.build();
     EXPECT_THAT(findDocumentHighlights(AST, T.point()), HighlightsFrom(T))
         << Test;
   }
Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -75,6 +75,7 @@
   TU.Code = std::string(Test.code());
 
   TU.ExtraArgs.push_back("-std=c++20");
+  TU.ExtraArgs.push_back("-xobjective-c++");
 
   for (auto File : AdditionalFiles)
     TU.AdditionalFiles.insert({File.first, std::string(File.second)});
@@ -645,6 +646,48 @@
       R"cpp(
       <:[deprecated]:> int $Variable_decl_deprecated[[x]];
       )cpp",
+      R"cpp(
+        // ObjC: Classes and methods
+        @class $Class_decl[[Forward]];
+
+        @interface $Class_decl[[Foo]]
+        @end
+        @interface $Class_decl[[Bar]] : $Class[[Foo]]
+        -($Class[[id]]) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]];
+        +(void) $StaticMethod_decl_static[[explode]];
+        @end
+        @implementation $Class_decl[[Bar]]
+        -($Class[[id]]) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]] {
+          return self;
+        }
+        +(void) $StaticMethod_decl_static[[explode]] {}
+        @end
+
+        void $Function_decl[[m]]($Class[[Bar]] *$Parameter_decl[[b]]) {
+          [$Parameter[[b]] $Method[[x]]:1 $Method[[y]]:2];
+          [$Class[[Bar]] $StaticMethod_static[[explode]]];
+        }
+      )cpp",
+      R"cpp(
+        // ObjC: Protocols
+        @protocol $Interface_decl[[Protocol]]
+        @end
+        @protocol $Interface_decl[[Protocol2]] <$Interface[[Protocol]]>
+        @end
+        @interface $Class_decl[[Klass]] <$Interface[[Protocol]]>
+        @end
+        // FIXME: protocol list in ObjCObjectType should be highlighted.
+        id<Protocol> $Variable_decl[[x]];
+      )cpp",
+      R"cpp(
+        // ObjC: Categories
+        @interface $Class_decl[[Foo]]
+        @end
+        @interface $Class[[Foo]]($Namespace_decl[[Bar]])
+        @end
+        @implementation $Class[[Foo]]($Namespace_decl[[Bar]])
+        @end
+      )cpp",
   };
   for (const auto &TestCase : TestCases)
     // Mask off scope modifiers to keep the tests manageable.
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -1593,6 +1593,35 @@
               "0: targets = {f}\n"
               "1: targets = {I::x}\n"
               "2: targets = {I::setY:}\n"},
+          {
+          // Objective-C: methods
+              R"cpp(
+            @interface I
+              -(void) a:(int)x b:(int)y;
+            @end
+            void foo(I *i) {
+              [$0^i $1^a:1 b:2];
+            }
+          )cpp",
+              "0: targets = {i}\n"
+              "1: targets = {I::a:b:}\n"
+          },
+          {
+          // Objective-C: protocols
+              R"cpp(
+            @interface I
+            @end
+            @protocol P
+            @end
+            void foo() {
+              // FIXME: should reference P
+              $0^I<P> *$1^x;
+            }
+          )cpp",
+              "0: targets = {I}\n"
+              "1: targets = {x}, decl\n"
+          },
+
           // Designated initializers.
           {R"cpp(
             void foo() {
Index: clang-tools-extra/clangd/XRefs.cpp
===================================================================
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -882,8 +882,8 @@
   };
 
   ReferenceFinder(const ParsedAST &AST,
-                  const llvm::DenseSet<SymbolID> &TargetIDs)
-      : AST(AST), TargetIDs(TargetIDs) {}
+                  const llvm::DenseSet<SymbolID> &TargetIDs, bool PerToken)
+      : PerToken(PerToken), AST(AST), TargetIDs(TargetIDs) {}
 
   std::vector<Reference> take() && {
     llvm::sort(References, [](const Reference &L, const Reference &R) {
@@ -915,21 +915,41 @@
     if (!TargetIDs.contains(ID))
       return true;
     const auto &TB = AST.getTokens();
-    Loc = SM.getFileLoc(Loc);
-    if (const auto *Tok = TB.spelledTokenAt(Loc))
-      References.push_back({*Tok, Roles, ID});
+
+    llvm::SmallVector<SourceLocation, 1> Locs;
+    if (PerToken) {
+      if (auto *OME = llvm::dyn_cast_or_null<ObjCMessageExpr>(ASTNode.OrigE)) {
+        OME->getSelectorLocs(Locs);
+      } else if (auto *OMD =
+                     llvm::dyn_cast_or_null<ObjCMethodDecl>(ASTNode.OrigD)) {
+        OMD->getSelectorLocs(Locs);
+      }
+      // Possible it was something else referencing an ObjCMethodDecl.
+      // If the first token doesn't match, assume our guess was wrong.
+      if (!Locs.empty() && Locs.front() != Loc)
+        Locs.clear();
+    }
+    if (Locs.empty())
+      Locs.push_back(Loc);
+
+    for (SourceLocation L : Locs) {
+      L = SM.getFileLoc(L);
+      if (const auto *Tok = TB.spelledTokenAt(L))
+        References.push_back({*Tok, Roles, ID});
+    }
     return true;
   }
 
 private:
+  bool PerToken; // If true, report 3 references for split ObjC selector names.
   std::vector<Reference> References;
   const ParsedAST &AST;
   const llvm::DenseSet<SymbolID> &TargetIDs;
 };
 
 std::vector<ReferenceFinder::Reference>
-findRefs(const llvm::DenseSet<SymbolID> &IDs, ParsedAST &AST) {
-  ReferenceFinder RefFinder(AST, IDs);
+findRefs(const llvm::DenseSet<SymbolID> &IDs, ParsedAST &AST, bool PerToken) {
+  ReferenceFinder RefFinder(AST, IDs, PerToken);
   index::IndexingOptions IndexOpts;
   IndexOpts.SystemSymbolFilter =
       index::IndexingOptions::SystemSymbolFilterKind::All;
@@ -1224,7 +1244,7 @@
         for (const NamedDecl *ND : Decls)
           if (auto ID = getSymbolID(ND))
             Targets.insert(ID);
-        for (const auto &Ref : findRefs(Targets, AST))
+        for (const auto &Ref : findRefs(Targets, AST, /*PerToken=*/true))
           Result.push_back(toHighlight(Ref, SM));
         return true;
       }
@@ -1376,7 +1396,7 @@
     }
 
     // We traverse the AST to find references in the main file.
-    auto MainFileRefs = findRefs(Targets, AST);
+    auto MainFileRefs = findRefs(Targets, AST, /*PerToken=*/false);
     // We may get multiple refs with the same location and different Roles, as
     // cross-reference is only interested in locations, we deduplicate them
     // by the location to avoid emitting duplicated locations.
Index: clang-tools-extra/clangd/SemanticHighlighting.h
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.h
+++ clang-tools-extra/clangd/SemanticHighlighting.h
@@ -37,6 +37,7 @@
   Field,
   StaticField,
   Class,
+  Interface,
   Enum,
   EnumConstant,
   Typedef,
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===================================================================
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -40,11 +40,27 @@
 /// Some names are not written in the source code and cannot be highlighted,
 /// e.g. anonymous classes. This function detects those cases.
 bool canHighlightName(DeclarationName Name) {
-  if (Name.getNameKind() == DeclarationName::CXXConstructorName ||
-      Name.getNameKind() == DeclarationName::CXXUsingDirective)
+  switch (Name.getNameKind()) {
+  case DeclarationName::Identifier: {
+    auto *II = Name.getAsIdentifierInfo();
+    return II && !II->getName().empty();
+  }
+  case DeclarationName::CXXConstructorName:
+  case DeclarationName::CXXDestructorName:
     return true;
-  auto *II = Name.getAsIdentifierInfo();
-  return II && !II->getName().empty();
+  case DeclarationName::ObjCZeroArgSelector:
+  case DeclarationName::ObjCOneArgSelector:
+  case DeclarationName::ObjCMultiArgSelector:
+    // Multi-arg selectors need special handling, and we handle 0/1 arg
+    // selectors there too.
+    return false;
+  case DeclarationName::CXXConversionFunctionName:
+  case DeclarationName::CXXOperatorName:
+  case DeclarationName::CXXDeductionGuideName:
+  case DeclarationName::CXXLiteralOperatorName:
+  case DeclarationName::CXXUsingDirective:
+    return false;
+  }
 }
 
 llvm::Optional<HighlightingKind> kindForType(const Type *TP);
@@ -73,13 +89,20 @@
       return llvm::None;
     return HighlightingKind::Class;
   }
-  if (isa<ClassTemplateDecl>(D) || isa<RecordDecl>(D) ||
-      isa<CXXConstructorDecl>(D))
+  if (isa<ClassTemplateDecl, RecordDecl, CXXConstructorDecl, ObjCInterfaceDecl,
+          ObjCImplementationDecl>(D))
     return HighlightingKind::Class;
+  if (isa<ObjCProtocolDecl>(D))
+    return HighlightingKind::Interface;
+  if (isa<ObjCCategoryDecl>(D))
+    return HighlightingKind::Namespace;
   if (auto *MD = dyn_cast<CXXMethodDecl>(D))
     return MD->isStatic() ? HighlightingKind::StaticMethod
                           : HighlightingKind::Method;
-  if (isa<FieldDecl>(D))
+  if (auto *OMD = dyn_cast<ObjCMethodDecl>(D))
+    return OMD->isClassMethod() ? HighlightingKind::StaticMethod
+                                : HighlightingKind::Method;
+  if (isa<FieldDecl, ObjCPropertyDecl>(D))
     return HighlightingKind::Field;
   if (isa<EnumDecl>(D))
     return HighlightingKind::Enum;
@@ -87,11 +110,14 @@
     return HighlightingKind::EnumConstant;
   if (isa<ParmVarDecl>(D))
     return HighlightingKind::Parameter;
-  if (auto *VD = dyn_cast<VarDecl>(D))
+  if (auto *VD = dyn_cast<VarDecl>(D)) {
+    if (isa<ImplicitParamDecl>(VD)) // e.g. ObjC Self
+      return llvm::None;
     return VD->isStaticDataMember()
                ? HighlightingKind::StaticField
                : VD->isLocalVarDecl() ? HighlightingKind::LocalVariable
                                       : HighlightingKind::Variable;
+  }
   if (const auto *BD = dyn_cast<BindingDecl>(D))
     return BD->getDeclContext()->isFunctionOrMethod()
                ? HighlightingKind::LocalVariable
@@ -115,6 +141,8 @@
     return HighlightingKind::Primitive;
   if (auto *TD = dyn_cast<TemplateTypeParmType>(TP))
     return kindForDecl(TD->getDecl());
+  if (isa<ObjCObjectPointerType>(TP))
+    return HighlightingKind::Class;
   if (auto *TD = TP->getAsTagDecl())
     return kindForDecl(TD);
   return llvm::None;
@@ -439,6 +467,36 @@
     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) {
+    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);
+    }
+  }
+
+  bool VisitObjCMethodDecl(ObjCMethodDecl *OMD) {
+    llvm::SmallVector<SourceLocation> Locs;
+    OMD->getSelectorLocs(Locs);
+    highlightObjCSelector(Locs, /*Decl=*/true, OMD->isClassMethod());
+    return true;
+  }
+
+  bool VisitObjCMessageExpr(ObjCMessageExpr *OME) {
+    llvm::SmallVector<SourceLocation> Locs;
+    OME->getSelectorLocs(Locs);
+    highlightObjCSelector(Locs, /*Decl=*/false, OME->isClassMessage());
+    return true;
+  }
+
   bool VisitOverloadExpr(OverloadExpr *E) {
     if (!E->decls().empty())
       return true; // handled by findExplicitReferences.
@@ -602,6 +660,8 @@
     return OS << "StaticField";
   case HighlightingKind::Class:
     return OS << "Class";
+  case HighlightingKind::Interface:
+    return OS << "Interface";
   case HighlightingKind::Enum:
     return OS << "Enum";
   case HighlightingKind::EnumConstant:
@@ -695,6 +755,8 @@
     return "property";
   case HighlightingKind::Class:
     return "class";
+  case HighlightingKind::Interface:
+    return "interface";
   case HighlightingKind::Enum:
     return "enum";
   case HighlightingKind::EnumConstant:
Index: clang-tools-extra/clangd/FindTarget.cpp
===================================================================
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -12,6 +12,7 @@
 #include "support/Logger.h"
 #include "clang/AST/ASTTypeTraits.h"
 #include "clang/AST/Decl.h"
+#include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/AST/DeclVisitor.h"
@@ -633,6 +634,60 @@
                                   /*IsDecl=*/false,
                                   {DG->getDeducedTemplate()}});
     }
+
+    void VisitObjCMethodDecl(const ObjCMethodDecl *OMD) {
+      Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+                                  OMD->getSelectorStartLoc(),
+                                  /*IsDecl=*/true,
+                                  {OMD}});
+    }
+
+    void visitProtocolList(
+        llvm::iterator_range<ObjCProtocolList::iterator> Protocols,
+        llvm::iterator_range<const SourceLocation *> Locations) {
+      for (const auto &P : llvm::zip(Protocols, Locations)) {
+        Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+                                    std::get<1>(P),
+                                    /*IsDecl=*/false,
+                                    {std::get<0>(P)}});
+      }
+    }
+
+    void VisitObjCInterfaceDecl(const ObjCInterfaceDecl *OID) {
+      if (OID->hasDefinition())
+        visitProtocolList(OID->protocols(), OID->protocol_locs());
+      Base::VisitObjCInterfaceDecl(OID); // Visit the interface itself.
+    }
+
+    void VisitObjCCategoryDecl(const ObjCCategoryDecl *OCD) {
+      visitProtocolList(OCD->protocols(), OCD->protocol_locs());
+      // getLocation is the extended class's location, not the category's.
+      Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+                                  OCD->getLocation(),
+                                  /*IsDecl=*/false,
+                                  {OCD->getClassInterface()}});
+      Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+                                  OCD->getCategoryNameLoc(),
+                                  /*IsDecl=*/true,
+                                  {OCD}});
+    }
+
+    void VisitObjCCategoryImplDecl(const ObjCCategoryImplDecl *OCID) {
+      Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+                                  OCID->getLocation(),
+                                  /*IsDecl=*/false,
+                                  {OCID->getClassInterface()}});
+      Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+                                  OCID->getCategoryNameLoc(),
+                                  /*IsDecl=*/true,
+                                  {OCID->getCategoryDecl()}});
+    }
+
+    void VisitObjCProtocolDecl(const ObjCProtocolDecl *OPD) {
+      if (OPD->hasDefinition())
+        visitProtocolList(OPD->protocols(), OPD->protocol_locs());
+      Base::VisitObjCProtocolDecl(OPD); // Visit the protocol itself.
+    }
   };
 
   Visitor V{Resolver};
@@ -711,6 +766,15 @@
           explicitReferenceTargets(DynTypedNode::create(*E), {}, Resolver)});
     }
 
+    void VisitObjCMessageExpr(const ObjCMessageExpr *E) {
+      llvm::SmallVector<const NamedDecl *, 1> Targets;
+      if (E->getMethodDecl())
+        Targets.push_back(E->getMethodDecl());
+      Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
+                                  E->getSelectorStartLoc(),
+                                  /*IsDecl=*/false, Targets});
+    }
+
     void VisitDesignatedInitExpr(const DesignatedInitExpr *DIE) {
       for (const DesignatedInitExpr::Designator &D : DIE->designators()) {
         if (!D.isFieldDesignator())
@@ -826,6 +890,16 @@
                          /*IsDecl=*/false,
                          {L.getTypedefNameDecl()}};
     }
+
+    void VisitObjCInterfaceTypeLoc(ObjCInterfaceTypeLoc L) {
+      Ref = ReferenceLoc{NestedNameSpecifierLoc(),
+                         L.getNameLoc(),
+                         /*IsDecl=*/false,
+                         {L.getIFaceDecl()}};
+    }
+
+    // FIXME: add references to protocols in ObjCObjectTypeLoc and maybe
+    // ObjCObjectPointerTypeLoc.
   };
 
   Visitor V{Resolver};
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to