ilya-biryukov added inline comments.
================ Comment at: clang-tools-extra/clangd/refactor/tweaks/ExpandMacro.cpp:57 + if (It == Spelled.begin()) + return Spelled.end(); + // Check the token we found actually touches the cursor position. ---------------- sammccall wrote: > it's pretty weird for a function whose return type is spelled "Token*" to use > Spelled.end() rather than nullptr as a sentinel Yeah, especially given that the function below uses `nullptr` as a sentinel... Thanks for pointing this out! ================ Comment at: clang-tools-extra/clangd/unittests/TweakTests.cpp:288 +#define FUNC(X) X+X+X +^F^O^O^ BAR ^F^O^O^ +^F^U^N^C^(1) ---------------- sammccall wrote: > Can you verify we don't trigger here? `FOO[[ ]]BAR` > > The zero-width range in `FOO^ BAR` is indeed interpreted as pointing at FOO > by SelectionTree, but that's a whitespace-sensitive heuristic. > > Given > ``` > int x(int); > #define B x > int y = B^(42); > ``` > > The `^` points at the `(`. (Maybe we should lift this logic into > Tweak::Selection) It does trigger, unfortunately. There is no way to unbreak this, as we don't have a way to get the selection range in inputs of the tweak. I've added a FIXME, will address with a follow-up. Note that we don't use selection tree, as it's AST-based and we need token-level information for that tweak (that's pretty unique, I expect most tweaks are not like that) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D61681/new/ https://reviews.llvm.org/D61681 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
