This is an archive of the discontinued LLVM Phabricator instance.

Cleanup llvm/DebugInfo/PDB headers
ClosedPublic

Authored by serge-sans-paille on Feb 19 2022, 3:05 PM.

Details

Summary

accumulated preprocessed size:
before: 1065515095
after: 1065629059

Discourse thread: https://discourse.llvm.org/t/include-what-you-use-include-cleanup

Diff Detail

Unit TestsFailed

Event Timeline

serge-sans-paille requested review of this revision.Feb 19 2022, 3:05 PM
Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptFeb 19 2022, 3:05 PM
before: 1065515095
after: 1065629059

An increase?

MaskRay accepted this revision.Feb 19 2022, 11:11 PM

It'd be good to test -DLLVM_ENABLE_MODULES=on build.

Some files get pure new headers. I still think it is good thing to do it separately. There is a risk that someone may revert the change if it breaks some build modes.
Splitting the change can be mechanical, perhaps with some git commands to detect what files get pure addition.

This revision is now accepted and ready to land.Feb 19 2022, 11:11 PM
before: 1065515095
after: 1065629059

An increase?

Ooopsie, a typo :-)

It'd be good to test -DLLVM_ENABLE_MODULES=on build.

Sure, I'll add that to my local test setup.

Some files get pure new headers.

That's expected. It happens a lot when some headers gets a forward declaration instead of a header include when referencing a type.

I still think it is good thing to do it separately. There is a risk that someone may revert the change if it breaks some build modes.
Splitting the change can be mechanical, perhaps with some git commands to detect what files get pure addition.

I fear I don't have the energy to go at that grain of detail :-/ I'm currently testing with all projects enabled, in release mode. I'll add a setup with ENABLE_MODULE and DEBUG mode to increase the coverage of my pre-commit test

This revision was landed with ongoing or failed builds.Feb 23 2022, 1:32 AM
This revision was automatically updated to reflect the committed changes.
thakis added a subscriber: thakis.Feb 23 2022, 3:49 AM

Breaks building on win: http://45.33.8.238/win/53749/step_4.txt

Please take a look and revert for now if it takes a while to fix.

@thakis should be fixed by 57c6012213b50804ed78530b89bae30c0ee4fe82 , the new failure (seems) unrelated to this change.

It'd be good to test -DLLVM_ENABLE_MODULES=on build.

Sure, I'll add that to my local test setup.

Some files get pure new headers.

That's expected. It happens a lot when some headers gets a forward declaration instead of a header include when referencing a type.

I still think it is good thing to do it separately. There is a risk that someone may revert the change if it breaks some build modes.
Splitting the change can be mechanical, perhaps with some git commands to detect what files get pure addition.

I fear I don't have the energy to go at that grain of detail :-/ I'm currently testing with all projects enabled, in release mode. I'll add a setup with ENABLE_MODULE and DEBUG mode to increase the coverage of my pre-commit test

OK, that should be fine. Thanks for the efforts :)
I was just thinking of potential build problems (like thakis') if you happened not to be around to fix...