[PATCH] D70936: [clang-scan-deps] do not skip empty #if/#elif in the minimize to avoid missing `__has_include` dependencies

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith accepted this revision.
dexonsmith added a comment.
This revision is now accepted and ready to land.

LGTM, although I have a suggested change for one of the comments (and I'm still sad about this).



================
Comment at: clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp:766-769
+  // If "#ifdef" is empty, strip it and skip the "#endif".
+  // Avoid stripping "#if" / "#elif" as they could contain a "__has_include"
+  // (either directly, or through a macro expansion) without an "#include"
+  // inside the "#if/elif", which could affect the dependency output.
----------------
I'd prefer to structure this comment differently, something like:
```
// If "#ifdef" is empty, strip it and skip the "#endif".
//
// TODO: Once/if Clang starts disallowing __has_include in macro expansions,
// we can skip empty `#if` and `#elif` blocks as well after scanning for a
// literal __has_include in the condition.  Even without that rule we could
// drop the tokens if we scan for identifiers in the condition and find none.
```
This rephrases it as a potential future optimization instead of one that has to be skipped, which I think makes it easier to reason about.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70936