sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:91
if (const auto *D = SelectedNode->ASTNode.get<Decl>()) {
- if (D->getLocation() != TokenStartLoc)
- return {};
+ if (D->getLocation() != TokenStartLoc) {
+ // Destructor->getLocation() points to ~. In this case, TokenStartLoc
----------------
Braindump from an offline conversation...
This seems like an unfortunate place to put a complicated special case and it
doesn't generalize well.
The TokenStartLoc check is a bit unfortunate to begin with - it's subject to
all the heuristic-token-rewinding problems we've encountered in the past. Its
one advantage over plain selectiontree is that it works at the end of the
identifier.
Better approach seems to be:
- we only allow rename targeted at an identifier token
- there can be at most one identifier touching the location (if there's one
before and after, they'd be a single identifier)
- so find that identifier spelled token using TokenBuffer
- then look it up as a macro (we get its start location for free)
- if that fails, create a selectiontree for the token's range and rename what
we find
I don't think there's a better TokenBuffer API than calling spelledTokens() and
binary-searching, at the moment.
This touches more code, but is less special-case-y overall. I'd consider
putting the getIdentifierTouching(SourceLocation, TokenBuffer, SourceManager)
in SourceCode.h
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D71247/new/
https://reviews.llvm.org/D71247
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
