[PATCH] D123243: [pseudo] Strip directives from a token stream

[Sam McCall via Phabricator via cfe-commits]

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