[PATCH] D133357: [Lex/DependencyDirectivesScanner] Keep track of the presence of tokens between the last scanned directive and EOF

[Ben Langmuir via Phabricator via cfe-commits]

benlangmuir added inline comments.


================
Comment at: clang/include/clang/Lex/DependencyDirectivesScanner.h:131
+/// \p dependency_directives_scan::tokens_present_before_eof, otherwise this
+/// directive will be ignored.
 ///
----------------
benlangmuir wrote:
> akyrtzi wrote:
> > benlangmuir wrote:
> > > Why would you want to print without this? It seems important for correctness of the output.  I would have expected we would always print it.
> > The `-print-dependency-directives-minimized-source` clang option is using this function, and if you print the sources with "<TokBEOF>" at the end then the source text will not be parsable.
> > 
> > But the tests that pass `-print-dependency-directives-minimized-source` don't try to parse the code, so I think I can switch the default to be `true` for printing the "tokens-before-eof marker" and if a caller has a need to ignore it it can pass `false` for the parameter.
> If someone uses `-print-dependency-directives-minimized-source` and creates a minimized file for each header, it will "parse", but it won't behave correctly for multiple includes, right?  My preference is we don't allow printing this without the TokBEOF.  If we care about making it parse, we should add a real token of some kind -- maybe there is a no-op `#pragma` or something?
Oops, missed half my comment. Meant to also add:

> But the tests that pass -print-dependency-directives-minimized-source don't try to parse the code, so I think I can switch the default to be true for printing the "tokens-before-eof marker" and if a caller has a need to ignore it it can pass false for the parameter.

SGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D133357