sammccall marked 4 inline comments as done.
sammccall added inline comments.
================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:281
+ std::vector<Expansion>
+ expansionsAffecting(llvm::ArrayRef<syntax::Token> Spelled) const;
----------------
kadircet wrote:
> this sounds more like `expansionsAffected` rather than affecting ? maybe my
> mental model requires correction, but it feels like spelled tokens are not
> affected by expansions, it is the other way around.
>
> maybe even `expansionsSpawned` or `triggered`?
>
> this is definitely non-blocking though, i am just bikesheding, comment is
> explanatory enough in any case.
Either mental model works I think. Mine is that expansions are rewrite rules -
you start with a stream of tokens, and then each expansion mutates it, and you
end up with different ones.
Renamed to `expansionsOverlapping` which seems more neutral and consistent with
expansionStartingAt
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:431
+std::vector<TokenBuffer::Expansion>
+TokenBuffer::expansionsAffecting(llvm::ArrayRef<syntax::Token> Spelled) const {
+ if (Spelled.empty())
----------------
kadircet wrote:
> this might be inconistent with spelledForExpanded from time to time, e.g:
>
> ```
> #define FOO(X) 123
> #define BAR
>
> void foo() {
> FOO(BA^R);
> }
> ```
>
> normally BAR has no expansions, but I think it will get merged into outer
> macro expansion e.g. `123` coming from `FOO(BAR)`. (whereas
> spelledForExpanded will treat `BAR` in isolation)
>
> not sure an important limitation but something to keep in mind.
I don't understand the consistency you're looking for - AFAICS these are
different functions that do different things - spelledForExpanded is much
higher level.
Is there something in the name or comment I can add/remove to clarify?
> normally BAR has no expansions
Tokens don't really "have expansions" - they're *part of* expansions. Generally
expansionsOverlapping() will return the expansions themselves that are
overlapping, while spelledForExpanded will include the expanded tokens if the
whole expansion is in-range.
The former is a building-block (returning Expansions seems like a clear
indicator of that) that will usually require some postfiltering, while the
latter is fairly directly usable.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84009/new/
https://reviews.llvm.org/D84009
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
