[PATCH] D59887: [Syntax] Introduce TokenBuffer, start clangToolingSyntax library

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Not sure what the implications of design changes are, so will defer reviewing details of tokencollector (which generally looks good, but is of course highly coupled to lexer/pp) and range mapping (which I suspect could be simplified, but depends heavily on the model).



================
Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:61
+
+class TokenCollector::Callbacks : public PPCallbacks {
+public:
----------------
I'm afraid this really needs a class comment describing what this is supposed to do (easy!) and the implementation strategy (hard!)

In particular, it looks like macros like this:
 - we expect to see the tokens making up the macro invocation first (... FOO ( 42 ))
 - then we see MacroExpands which allows us to recognize the last N tokens are a macro invocation. We create a MacroInvocation object, and remember where the invocation ends
 - then we see the tokens making up the macro expansion
 - finally, once we see the next token after the invocation, we finalize the MacroInvocation.


================
Comment at: clang/lib/Tooling/Syntax/TokenBuffer.cpp:285
+    std::tie(File, Offset) = SM.getDecomposedLoc(L);
+    if (File != MacroInvocationFile || Offset <= ExpansionEndOffset)
+      return;
----------------
is there a problem if we never see another token in that file after the expansion?
(or do we see the eof token in this case?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D59887