[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG8401713b3ef1: [clangd] Ignore ObjC `id` and `instancetype` 
in FindTarget (authored by dgoldman).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]] {
   return self;
 }
++(instancetype)$StaticMethod_decl_static[[sharedInstance]] { return 0; 
}
 +(void) $StaticMethod_decl_static[[explode]] {}
 @end
 
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -995,6 +995,20 @@
 }
   )cpp";
   EXPECT_DECLS("ObjCPropertyRefExpr", "+ (id)sharedInstance");
+
+  Code = R"cpp(
+@interface Foo
++ ([[id]])sharedInstance;
+@end
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc");
+
+  Code = R"cpp(
+@interface Foo
++ ([[instancetype]])sharedInstance;
+@end
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc");
 }
 
 class FindExplicitReferencesTest : public ::testing::Test {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -94,6 +94,16 @@
   return nullptr;
 }
 
+// Returns true if the `TypedefNameDecl` should not be reported.
+bool shouldSkipTypedef(const TypedefNameDecl *TD) {
+  // These should be treated as keywords rather than decls - the typedef is an
+  // odd implementation detail.
+  if (TD == TD->getASTContext().getObjCInstanceTypeDecl() ||
+  TD == TD->getASTContext().getObjCIdDecl())
+return true;
+  return false;
+}
+
 // TargetFinder locates the entities that an AST node refers to.
 //
 // Typically this is (possibly) one declaration and (possibly) one type, but
@@ -395,6 +405,8 @@
 }
   }
   void VisitTypedefType(const TypedefType *TT) {
+if (shouldSkipTypedef(TT->getDecl()))
+  return;
 Outer.add(TT->getDecl(), Flags);
   }
   void
@@ -903,6 +915,8 @@
 }
 
 void VisitTypedefTypeLoc(TypedefTypeLoc L) {
+  if (shouldSkipTypedef(L.getTypedefNameDecl()))
+return;
   Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
   L.getNameLoc(),
   /*IsDecl=*/false,


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]] {
   return self;
 }
++(instancetype)$StaticMethod_decl_static[[sharedInstance]] { return 0; }
 +(void) $StaticMethod_decl_static[[explode]] {}
 @end
 
Index: 

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet accepted this revision.
kadircet added a comment.
This revision is now accepted and ready to land.

sorry, I thought I've already LGTM'd it in previous iteration. Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

dgoldman marked an inline comment as done.
dgoldman added a comment.

Friendly ping, PTAL


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 2 inline comments as done.
dgoldman added inline comments.



Comment at: clang-tools-extra/clangd/FindTarget.cpp:188
 if (const TypedefNameDecl *TND = dyn_cast(D)) {
+  if (shouldSkipTypedef(TND))
+return;

kadircet wrote:
> why do we need this despite bailing out in the visitor?
Removed, not necessary


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 371103.
dgoldman added a comment.

Update comment + remove un-needed code


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]] {
   return self;
 }
++(instancetype)$StaticMethod_decl_static[[sharedInstance]] { return 0; 
}
 +(void) $StaticMethod_decl_static[[explode]] {}
 @end
 
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -995,6 +995,20 @@
 }
   )cpp";
   EXPECT_DECLS("ObjCPropertyRefExpr", "+ (id)sharedInstance");
+
+  Code = R"cpp(
+@interface Foo
++ ([[id]])sharedInstance;
+@end
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc");
+
+  Code = R"cpp(
+@interface Foo
++ ([[instancetype]])sharedInstance;
+@end
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc");
 }
 
 class FindExplicitReferencesTest : public ::testing::Test {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -94,6 +94,16 @@
   return nullptr;
 }
 
+// Returns true if the `TypedefNameDecl` should not be reported.
+bool shouldSkipTypedef(const TypedefNameDecl *TD) {
+  // These should be treated as keywords rather than decls - the typedef is an
+  // odd implementation detail.
+  if (TD == TD->getASTContext().getObjCInstanceTypeDecl() ||
+  TD == TD->getASTContext().getObjCIdDecl())
+return true;
+  return false;
+}
+
 // TargetFinder locates the entities that an AST node refers to.
 //
 // Typically this is (possibly) one declaration and (possibly) one type, but
@@ -395,6 +405,8 @@
 }
   }
   void VisitTypedefType(const TypedefType *TT) {
+if (shouldSkipTypedef(TT->getDecl()))
+  return;
 Outer.add(TT->getDecl(), Flags);
   }
   void
@@ -903,6 +915,8 @@
 }
 
 void VisitTypedefTypeLoc(TypedefTypeLoc L) {
+  if (shouldSkipTypedef(L.getTypedefNameDecl()))
+return;
   Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
   L.getNameLoc(),
   /*IsDecl=*/false,


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]] {
   return self;
 }
++(instancetype)$StaticMethod_decl_static[[sharedInstance]] { return 0; }
 +(void) $StaticMethod_decl_static[[explode]] {}
 @end
 
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- 

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, this looks great! just a question about the extra handling




Comment at: clang-tools-extra/clangd/FindTarget.cpp:100
+  // Even though ObjC `id` and `instancetype` are *implemented* via typedefs, 
we
+  // don't want to treat them like typedefs - instead let the editor treat
+  // them like keywords.

i think comment needs updating. rather than mentioning the editor, can we just 
talk about "these should be treated as keywords rather than decls" ?



Comment at: clang-tools-extra/clangd/FindTarget.cpp:188
 if (const TypedefNameDecl *TND = dyn_cast(D)) {
+  if (shouldSkipTypedef(TND))
+return;

why do we need this despite bailing out in the visitor?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

In D108556#2981712 , @kadircet wrote:

> Hmm, it sounds like you want them to be treated one way during semantic 
> highlighting and another during other features, which is fine but somewhat 
> confusing. (e.g. we want to surface hover/goto on these identifiers, but 
> we're making them less visible by encouraging editors to highlight them as 
> keywords).

I went ahead and just ignored them in FindTarget (LMK if that's right) - I 
think you're right that it's unlikely to provide any real value so we might as 
well not support it.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 370609.
dgoldman added a comment.

Swap to ignore in FindTarget


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

Files:
  clang-tools-extra/clangd/FindTarget.cpp
  clang-tools-extra/clangd/unittests/FindTargetTests.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]] {
   return self;
 }
++(instancetype)$StaticMethod_decl_static[[sharedInstance]] { return 0; 
}
 +(void) $StaticMethod_decl_static[[explode]] {}
 @end
 
Index: clang-tools-extra/clangd/unittests/FindTargetTests.cpp
===
--- clang-tools-extra/clangd/unittests/FindTargetTests.cpp
+++ clang-tools-extra/clangd/unittests/FindTargetTests.cpp
@@ -995,6 +995,20 @@
 }
   )cpp";
   EXPECT_DECLS("ObjCPropertyRefExpr", "+ (id)sharedInstance");
+
+  Code = R"cpp(
+@interface Foo
++ ([[id]])sharedInstance;
+@end
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc");
+
+  Code = R"cpp(
+@interface Foo
++ ([[instancetype]])sharedInstance;
+@end
+  )cpp";
+  EXPECT_DECLS("TypedefTypeLoc");
 }
 
 class FindExplicitReferencesTest : public ::testing::Test {
Index: clang-tools-extra/clangd/FindTarget.cpp
===
--- clang-tools-extra/clangd/FindTarget.cpp
+++ clang-tools-extra/clangd/FindTarget.cpp
@@ -94,6 +94,17 @@
   return nullptr;
 }
 
+// Returns true if the `TypedefNameDecl` should not be reported.
+bool shouldSkipTypedef(const TypedefNameDecl *TD) {
+  // Even though ObjC `id` and `instancetype` are *implemented* via typedefs, 
we
+  // don't want to treat them like typedefs - instead let the editor treat
+  // them like keywords.
+  if (TD == TD->getASTContext().getObjCInstanceTypeDecl() ||
+  TD == TD->getASTContext().getObjCIdDecl())
+return true;
+  return false;
+}
+
 // TargetFinder locates the entities that an AST node refers to.
 //
 // Typically this is (possibly) one declaration and (possibly) one type, but
@@ -174,6 +185,8 @@
   D = UDD->getNominatedNamespaceAsWritten();
 
 if (const TypedefNameDecl *TND = dyn_cast(D)) {
+  if (shouldSkipTypedef(TND))
+return;
   add(TND->getUnderlyingType(), Flags | Rel::Underlying);
   Flags |= Rel::Alias; // continue with the alias.
 } else if (const UsingDecl *UD = dyn_cast(D)) {
@@ -395,6 +408,8 @@
 }
   }
   void VisitTypedefType(const TypedefType *TT) {
+if (shouldSkipTypedef(TT->getDecl()))
+  return;
 Outer.add(TT->getDecl(), Flags);
   }
   void
@@ -903,6 +918,8 @@
 }
 
 void VisitTypedefTypeLoc(TypedefTypeLoc L) {
+  if (shouldSkipTypedef(L.getTypedefNameDecl()))
+return;
   Refs.push_back(ReferenceLoc{NestedNameSpecifierLoc(),
   L.getNameLoc(),
   /*IsDecl=*/false,


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) 

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

Hmm, it sounds like you want them to be treated one way during semantic 
highlighting and another during other features, which is fine but somewhat 
confusing. (e.g. we want to surface hover/goto on these identifiers, but we're 
making them less visible by encouraging editors to highlight them as keywords).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

dgoldman added a comment.

In D108556#2962008 , @kadircet wrote:

> thanks, it looks good as a contained fix. but it feels like we probably don't 
> want these to be treated as decls in other places too (e.g. can we really 
> provide any useful goto/hover on those `id` or `instancetype` decls?) maybe 
> we should update ASTVisitors in FindTarget to not report these at all. WDYT?

That's a good point - I guess it could be marginally useful?

Hovering over instancetype you'll see `typedef id instancetype` and can be 
brought via goto definition to objc.h which defines `typedef struct objc_object 
*id;`. Hovering over id you see `typedef id id` (which is quite confusing) and 
go-to definition brings you to objc.h as well. In theory the hover information 
should show you `A pointer to an instance of a class.` (comment from objc.h), 
seems to have worked previously, not sure why it doesn't atm. References 
doesn't appear to work and you'd really never want to use it if it did - 
there'd be way too many.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

thanks, it looks good as a contained fix. but it feels like we probably don't 
want these to be treated as decls in other places too (e.g. can we really 
provide any useful goto/hover on those `id` or `instancetype` decls?) maybe we 
should update ASTVisitors in FindTarget to not report these at all. WDYT?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D108556/new/

https://reviews.llvm.org/D108556

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D108556: [clangd] Don't highlight ObjC `id` and `instancetype`

[David Goldman via Phabricator via cfe-commits]

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

Even though they're implemented via typedefs, we typically
want to treat them like keywords (default editor behavior).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108556

Files:
  clang-tools-extra/clangd/SemanticHighlighting.cpp
  clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] 
$Method_decl[[y]]:(int)$Parameter_decl[[b]] {
   return self;
 }
++(instancetype)$StaticMethod_decl_static[[sharedInstance]] { return 0; 
}
 +(void) $StaticMethod_decl_static[[explode]] {}
 @end
 
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -78,6 +78,12 @@
   D = Templated;
   }
   if (auto *TD = dyn_cast(D)) {
+// Even though ObjC `id` and `instancetype` are implemented via typedefs, 
we
+// don't want to treat them like typedefs - instead let the editor treat
+// them like keywords.
+if (TD == D->getASTContext().getObjCInstanceTypeDecl() ||
+TD == D->getASTContext().getObjCIdDecl())
+  return llvm::None;
 // We try to highlight typedefs as their underlying type.
 if (auto K =
 kindForType(TD->getUnderlyingType().getTypePtrOrNull(), Resolver))


Index: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
===
--- clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -661,13 +661,15 @@
 @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]];
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]];
++(instancetype)$StaticMethod_decl_static[[sharedInstance]];
 +(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]] {
+-(id) $Method_decl[[x]]:(int)$Parameter_decl[[a]] $Method_decl[[y]]:(int)$Parameter_decl[[b]] {
   return self;
 }
++(instancetype)$StaticMethod_decl_static[[sharedInstance]] { return 0; }
 +(void) $StaticMethod_decl_static[[explode]] {}
 @end
 
Index: clang-tools-extra/clangd/SemanticHighlighting.cpp
===
--- clang-tools-extra/clangd/SemanticHighlighting.cpp
+++ clang-tools-extra/clangd/SemanticHighlighting.cpp
@@ -78,6 +78,12 @@
   D = Templated;
   }
   if (auto *TD = dyn_cast(D)) {
+// Even though ObjC `id` and `instancetype` are implemented via typedefs, we
+// don't want to treat them like typedefs - instead let the editor treat
+// them like keywords.
+if (TD == D->getASTContext().getObjCInstanceTypeDecl() ||
+TD == D->getASTContext().getObjCIdDecl())
+  return llvm::None;
 // We try to highlight typedefs as their underlying type.
 if (auto K =
 kindForType(TD->getUnderlyingType().getTypePtrOrNull(), Resolver))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits