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

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

Reply via email to