[PATCH] D119077: clangd SemanticHighlighting: added support for highlighting overloaded operators

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D119077#3337107 <https://reviews.llvm.org/D119077#3337107>, @nridge wrote:

> 1. The AST nodes that reference operator names should store source ranges associated with the operator names, such that we shouldn't need to do token-by-token manipulation. For example, `functionDecl->getNameInfo().getCXXOperatorNameRange()`.

Fleshing this out a bit more:

- For `FunctionDecl`, `getNameInfo().getCXXOperatorNameRange()` is almost the right thing, but it excludes the `operator` keyword itself. We know `getLocation()` was giving us the location of the `operator` keyword, so we can construct the source range we want as `SourceRange(getLocation(), getNameInfo().getCXXOperatorNameRange().getEnd())`.
- For `DeclRefExpr`, we can do the same thing, using `DeclRefExpr::getNameInfo()`.
- For `MemberExpr`, same thing using `MemberExpr::getMemberNameInfo()` (and `getMemberLoc()` as the start of the range).
- For `CXXOperatorCallExpr`, it seems to only handle single-token cases so we should be good to continue using just `getOperatorLoc()`

I believe that should allow us to avoid token manipulation altogether.

> 2. [...] would it make sense to handle the **single-token** cases with `findExplicitReferences()`? Or would that just end up splitting the logic / duplicating code more?

Thinking more about this, I don't think it would make sense. For example, it's not the case that all //references// to overloaded operators could be handled by `findExplicitReferences()` (there are multi-token reference cases like explicit operator calls). So, let's just keep using `CollectExtraHighlightings` for all cases here.



================
Comment at: clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp:815
+          $LocalVariable[[foo]].$Method[[operator()]]();
+          $LocalVariable[[foo]].$Method[[operator<<]](1);
+
----------------
I'm going to suggest that we exercise a few more implicit-call cases:
  * single-token, e.g. `foo << 1`
  * `operator()`, e.g. `foo()`
  * `operator[]`, e.g. `foo[1]`

(Very interestingly, in the implicit `()` and `[]` cases, the opening one seems to be colored by `VisitDeclRefExpr` but the closing one by `VisitCXXOperatorCallExpr`. Not quite sure why that is.)


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

https://reviews.llvm.org/D119077