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