sammccall marked 2 inline comments as done. sammccall added inline comments.
================ Comment at: clang-tools-extra/pseudo/lib/DirectiveTree.cpp:373 + case DirectiveTree::Chunk::K_Empty: + break; + } ---------------- hokein wrote: > return, otherwise we we hit the unreachable code below. That's deliberate, K_Empty is invalid. ================ Comment at: clang-tools-extra/pseudo/tool/ClangPseudo.cpp:40 +static opt<bool> + Preprocess("preprocess", + desc("Strip directives and select conditional sections")); ---------------- hokein wrote: > `preprocess` seems a little confusing (the name reminds me of the traditional > preprocessor process where comments are stripped, but not in our case here). > may be `strip-directives` but it doesn't reflect the conditional-#if token > work, but I think it is fine. > > This flag is a feature-control flag for `print-source` and `print-tokens`, > can we group three of them together? > may be strip-directives but it doesn't reflect the conditional-#if token > work, but I think it is fine Agreed, renamed it. > This flag is a feature-control flag for print-source and print-tokens, can we > group three of them together? It's not really, it also affects the GLR parser etc. Generally I think we should try to make the flags in this tool more orthogonal rather than clustering them by effects. Thematically it's more closely related to -print-directive-tree. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D123243/new/ https://reviews.llvm.org/D123243 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits