[PATCH] D64617: [clangd] Added highlighting for members and methods

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:44
+  bool VisitMemberExpr(MemberExpr *ME) {
+    if (const CXXMethodDecl *MD =
+            dyn_cast<CXXMethodDecl>(ME->getMemberDecl())) {
----------------
nit: this can be simplified like 

```
if (const auto* D = dyn_cast<CXXDestructorDecl>(ME->getMemberDecl())) {
  // When calling the destructor manually like: AAA::~A(); The ~ is a
  // MemberExpr.  
 return true;
}
```


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:53
+
+    // The MemberDecl is VarDecl for static members, therefore getMemberDecl
+    // does not work for all member variables.
----------------
It took me a while to understand how the comment associate with the code here, maybe add `and we use the MemberExpr` in the comment?


================
Comment at: clang-tools-extra/clangd/SemanticHighlighting.cpp:142
     }
+    if(isa<CXXMethodDecl>(D)) {
+      addToken(Loc, HighlightingKind::Method);
----------------
nit: clang-format.

As it is class-related, could you move it to Line 133 (immediately after the `Class` case), the same to the `FieldDecl`.


================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:140
+      struct $Class[[D]] {
+        double $MemberVariable[[C]];
+      };
----------------
could you add a test case for `static member`? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64617