gribozavr2 added inline comments.
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:239
+
+ auto It = llvm::partition_point(F.Mappings, [SpelledI](const Mapping &M) {
+ return M.BeginSpelled <= SpelledI;
----------------
It would be great to add an is_sorted assertion to the builder to check
ordering of mappings based on both spelled and expanded token indices.
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:259
+ const MarkedFile &File = It->second;
+ // assert that Spelled is a subarray of File.SpelledTokens.
+ assert(File.SpelledTokens.data() <= Spelled.data());
----------------
s/subarray/subrange/
Also no need to repeat "assert".
"`Spelled` must be a subrange of `File.SpelledTokens`."
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:261
+ assert(File.SpelledTokens.data() <= Spelled.data());
+ assert(Spelled.size() <= File.SpelledTokens.size());
+
----------------
I think we can improve the precision of this assertion. Comparing the sizes
does not ensure that we really have a subrange. I think the second assert
should be comparing end pointers instead of sizes. Something like:
`assert(&Spelled.back() <= &File.SpelledTokens.back());`
================
Comment at: clang/lib/Tooling/Syntax/Tokens.cpp:264
+ auto *FrontMapping = mappingStartingBeforeSpelled(File, &Spelled.front());
+ unsigned SpelledFrontI = &Spelled.front() - File.SpelledTokens.data();
+ unsigned ExpandedBegin;
----------------
Or assert that SpelledFrontI is less than File.SpelledTokens.size().
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77209/new/
https://reviews.llvm.org/D77209
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
