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