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 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits