Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Exclude prefix headers from macro-defined-by-includer heuristics (issue #368). #410

Conversation

vsapsai
Copy link
Contributor

@vsapsai vsapsai commented Feb 13, 2017

Also considered options of not populating macro_users_ or
direct_includes_as_fileentries_ for prefix headers but decided not to
do so. I think it is better to have correct information for prefix
headers but not to give recommendations based on that instead of having
incomplete information.

include-what-you-use#368).

Also considered options of not populating `macro_users_` or
`direct_includes_as_fileentries_` for prefix headers but decided not to
do so. I think it is better to have correct information for prefix
headers but not to give recommendations based on that instead of having
incomplete information.
Copy link
Contributor

@kimgr kimgr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great to me! Comment nit inline, let's merge with that fixed.

@@ -1889,6 +1889,13 @@ size_t PrintableDiffs(const string& filename,
void IwyuFileInfo::HandlePreprocessingDone() {
// Check macros defined by includer. Requires file preprocessing to be
// finished to know all direct includes and all macro usages.
//
// Exclude prefix headers from mapping heuristics. Includes in prefix
// headers are kept regardless of their usage in includer. And entire
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And the entire

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will fix.

@kimgr
Copy link
Contributor

kimgr commented Feb 19, 2017

Looks like our mailbot doesn't pick up the new GitHub reviews. See above.

@kimgr kimgr modified the milestone: iwyu 0.8 Feb 19, 2017
vsapsai added a commit to vsapsai/include-what-you-use that referenced this pull request Feb 20, 2017
include-what-you-use#368).

Also considered options of not populating `macro_users_` or
`direct_includes_as_fileentries_` for prefix headers but decided not to
do so. I think it is better to have correct information for prefix
headers but not to give recommendations based on that instead of having
incomplete information.

PR include-what-you-use#410
@vsapsai
Copy link
Contributor Author

vsapsai commented Feb 20, 2017

Merged 7cb24d2 with comment fix.

@vsapsai vsapsai closed this Feb 20, 2017
huizongsong pushed a commit to huizongsong/include-what-you-use that referenced this pull request Aug 26, 2022
…ue #368).

Also considered options of not populating `macro_users_` or
`direct_includes_as_fileentries_` for prefix headers but decided not to
do so. I think it is better to have correct information for prefix
headers but not to give recommendations based on that instead of having
incomplete information.

PR include-what-you-use/include-what-you-use#410
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants