[PATCH] D127403: [clangd] Implement semantic token modifier "definition"

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:847-849
+            } else if (const auto Iface =
+                           dyn_cast<const ObjCInterfaceDecl>(Decl)) {
+              if (Iface->isThisDeclarationADefinition())
----------------
ckandeler wrote:
> dgoldman wrote:
> > Do we need similar code for ObjCProtocolDecl? Also should ObjCImplDecl be considered definitions here?
> Possibly. I know next to nothing about Objective C, so I'll just do as I'm told here. On a related note, the code above regarding ObjCMethodDecl does not seem to do anything, i.e. none of the constructs that to my eye appear to be Objective-C methods get the "def" modifier. 
Gotcha, yeah I think it makes sense to do the same for ObjCImplDecl, ObjCProtocolDecl, and ObjCCategoryDecl.

re: methods, ah yeah, that's because canHighlightName will return false since we need to special case handling for ObjC methods since their names can be split across non contiguous tokens (selectors). Instead, would need to update `VisitObjCMethodDecl` and `highlightObjCSelector`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127403