sammccall added a comment.
Hooray!
================
Comment at: clang-tools-extra/clangd/Hover.cpp:537
+ // We prefer the identifiers if any, otherwise make use of the first token.
+ SourceLocation SourceLocationBeg = TokensAroundCursor.front().location();
----------------
first -> last I think, better explained as rightmost. See
SelectionTree::create().
================
Comment at: clang-tools-extra/clangd/Hover.cpp:537
+ // We prefer the identifiers if any, otherwise make use of the first token.
+ SourceLocation SourceLocationBeg = TokensAroundCursor.front().location();
----------------
sammccall wrote:
> first -> last I think, better explained as rightmost. See
> SelectionTree::create().
actually, if this is really only used for macros and auto, you could pull out
those specific cases (define AutoLoc and IdentifierLoc and initialize them by
looping over the touching tokens). Seems a bit simpler/more direct/less risk of
reuse for other things?
================
Comment at: clang-tools-extra/clangd/Hover.cpp:540
+ for (const auto &Tok : TokensAroundCursor) {
+ if (Tok.kind() != tok::identifier)
+ continue;
----------------
This deserves a comment, it's a different strategy than SelectionTree that
should yield the same result.
("In general we prefer the touching token that works over the one that doesn't,
see SelectionTree::create(). This location is used only for triggering on
macros and auto, so simply choosing the lone identifier-or-keyword token is
equivalent")
================
Comment at: clang-tools-extra/clangd/Hover.cpp:540
+ for (const auto &Tok : TokensAroundCursor) {
+ if (Tok.kind() != tok::identifier)
+ continue;
----------------
sammccall wrote:
> This deserves a comment, it's a different strategy than SelectionTree that
> should yield the same result.
>
> ("In general we prefer the touching token that works over the one that
> doesn't, see SelectionTree::create(). This location is used only for
> triggering on macros and auto, so simply choosing the lone
> identifier-or-keyword token is equivalent")
you also need to check for keyword (or specifically kw auto)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75176/new/
https://reviews.llvm.org/D75176
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
