[PATCH] D77614: [Syntax] Simplify TokenCollector::Builder, use captured expansion bounds. NFC

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:560
+           SpelledTokens[NextSpelled].location() < Target) {
+      // If we know of mapping bounds at [Next, KnownEnd] (e.g. macro expansion)
+      // then we want to partition our (empty) mapping.
----------------
s/Next/NextSpelled

in following lines as well


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:588
+    auto &NextSpelled = this->NextSpelled[File];
+
+    if (Tok.location().isFileID()) {
----------------
nit: maybe create mapping here and increment NextSpelled and NextExpanded explicitly here.


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:594
+             SpelledTokens[NextSpelled].location() ==
+                 Result.ExpandedTokens[NextExpanded].location()) {
+        ++NextSpelled;
----------------
maybe also explicitly spell the assumption:
`assert(ExpandedTok.location().isFileID())` ?


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:645
   TokenBuffer Result;
-  /// For each file, a position of the next spelled token we will consume.
+  // Cursor within the expanded token stream, and each file.
+  unsigned NextExpanded = 0;
----------------
i suppose `and each file` part refers to `NextSpelled` but it seems to be confusing maybe have a separate one below ?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D77614/new/

https://reviews.llvm.org/D77614