This is an archive of the discontinued LLVM Phabricator instance.

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

Authored by arphaman on Dec 2 2019, 6:09 PM.

Details

Summary

This patch makes the minimizer more conservative to avoid missing dependency files that are brought in by __has_include PP expressions that occur in a condition of an #if/#elif that was previously skipped. The __has_include PP expressions can be used in an #if/#elif either directly, or through macro expansion, so we can't detect them at the time of minimization.

It'll be possible to skip certain #if/#elif in the future by doing more minimizer optimizations (e.g. skip #if 0). Furthermore, @Bigcheese is working on clarifying the allowed usage of __has_include in the standard, as it seems that this kind of usage should actually be disallowed. This will hopefully allow us skip these #if/#elif again in the future.

Diff Detail

Event Timeline

arphaman created this revision.Dec 2 2019, 6:09 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 2 2019, 6:09 PM
dexonsmith accepted this revision.Dec 2 2019, 6:23 PM

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

clang/lib/Lex/DependencyDirectivesSourceMinimizer.cpp
766–773

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.

This revision is now accepted and ready to land.Dec 2 2019, 6:23 PM
This revision was automatically updated to reflect the committed changes.
arphaman marked an inline comment as done.