[PATCH] D77209: [Syntax] Add mapping from spelled to expanded tokens for TokenBuffer

[Marcel Hlopko via Phabricator via cfe-commits]

hlopko added inline comments.


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:264
+  auto *FrontMapping = mappingStartingBeforeSpelled(File, &Spelled.front());
+  unsigned SpelledFrontI = &Spelled.front() - File.SpelledTokens.data();
+  unsigned ExpandedBegin;
----------------
gribozavr2 wrote:
> hlopko wrote:
> > gribozavr2 wrote:
> > > Or assert that SpelledFrontI is less than File.SpelledTokens.size().
> > I think the assert I added is good enough?
> The assertion that I suggested is stronger, because it would prevent an out-of-bounds read from even happening, and would not rely on `isBeforeInTranslationUnit` returning false on garbage data.
Great points! Done.


================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:602
+    for (auto &pair : Result.Files) {
+      auto &mappings = pair.second.Mappings;
+      assert(std::is_sorted(
----------------
gribozavr2 wrote:
> We'd get an "unused variable" warning here when assertions are disabled. Please wrap the whole loop in `#ifndef NDEBUG`.
Done, added ifndef also above for T1/T2 variables.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77209