akyrtzi 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:
> 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.


================
Comment at: clang/lib/Lex/DependencyDirectivesScanner.cpp:868
+        PrintMarkerForTokensBeforeEOF)
+      OS << "<TokBEOF>";
     Optional<tok::TokenKind> PrevTokenKind;
----------------
benlangmuir wrote:
> How about "TokBeforeEOF"?  I think "BEOF" is too cryptic.
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

Reply via email to