sammccall accepted this revision.
sammccall added inline comments.
This revision is now accepted and ready to land.
================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:228
+ /// multiple times and this function will return multiple results in those
+ /// cases. This happens when \p Spelled is inside a macro argument.
+ ///
----------------
Nit: move the FIXME up here? Documenting this justifies the signature, but
currently it *never* happens.
================
Comment at: clang/include/clang/Tooling/Syntax/Tokens.h:248
+ llvm::SmallVector<llvm::ArrayRef<syntax::Token>, 1>
+ expandedForSpelled(llvm::ArrayRef<syntax::Token> Spelled) const;
+
----------------
out of curiosity, do you have a motivating use for this function?
I think it's clear enough that it will be useful, completeness dictates it
should be here, and use cases aren't always the best way to think about
designing these libraries. But it'd help me understand some of the details
better.
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:198
+TokenBuffer::expandedForSpelled(llvm::ArrayRef<syntax::Token> Spelled) const {
+ assert(!Spelled.empty());
+ assert(Spelled.front().location().isFileID());
----------------
This is a little surprising (vs returning `{}`).
It seems plausible that you'll map file offsets -> spelled tokens -> expanded
tokens, and that the middle step might "accidentally" be empty. Caller can
always check, but why require them to here?
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:205
+
+ auto &File = It->second;
+ assert(File.SpelledTokens.data() <= Spelled.data());
----------------
nit: mind spelling this type out? it's important, not particularly verbose, and
long-lived
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:209
+
+ auto *FrontMapping = mappingStartingBeforeSpelled(File, &Spelled.front());
+ unsigned SpelledFrontI = &Spelled.front() - File.SpelledTokens.data();
----------------
This section could use some comments about how the index calculations relate to
high-level concepts.
AIUI the branches are: spelled token precedes all PP usage, spelled token is
transformed by PP (subcases: macro args and other PP usage), spelled token
follows PP usage.
The next section probably only needs to comment about the purpose of the
divergence (+1/End to refer to one-past the region of interest).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72581/new/
https://reviews.llvm.org/D72581
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
