This is an archive of the discontinued LLVM Phabricator instance.

[Pass] Ensure we don't include PassSupport.h/PassAnalysisSupport.h directly
ClosedPublic

Authored by RKSimon on Apr 24 2020, 9:35 AM.

Details

Summary

Both PassSupport.h and PassAnalysisSupport.h are only supposed to be included via Pass.h.

This patch fixes the case where PassRegistry.cpp was including PassSupport.h and adds #ifndef error checks to help ensure it doesn't happen again.

Diff Detail

Event Timeline

RKSimon created this revision.Apr 24 2020, 9:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptApr 24 2020, 9:36 AM
Herald added a subscriber: hiraditya. · View Herald Transcript

There are still a bunch of other users of the headers in tree, but the error doesn't trigger since Pass.h gets included before. Is that worth changing as well?

There are still a bunch of other users of the headers in tree, but the error doesn't trigger since Pass.h gets included before. Is that worth changing as well?

Sure, I'll take a look

RKSimon updated this revision to Diff 260080.Apr 25 2020, 4:08 AM

Add check that PassSupport.h/PassAnalysisSupport.h aren't included again after Pass.h has been included.

Many of the additional file changes have implicit Pass.h includes but since the PassSupport.h/PassAnalysisSupport.h include was explicit I've added a Pass.h include.

RKSimon marked an inline comment as done.Apr 25 2020, 4:09 AM
RKSimon added inline comments.
llvm/include/llvm/PassAnalysisSupport.h
22

Technically these include guards are now redundant but I've kept them for clarity

thegameg accepted this revision.Apr 25 2020, 10:51 PM

LGTM, thanks!

This revision is now accepted and ready to land.Apr 25 2020, 10:51 PM
This revision was automatically updated to reflect the committed changes.