This is an archive of the discontinued LLVM Phabricator instance.

[include-cleaner] Fix the `fixIncludes` API not respect main-file header.
ClosedPublic

Authored by hokein on Jul 11 2023, 4:44 AM.

Details

Summary

The fixIncludes was using the input as the main file path, this will
results in inserting header at wrong places.

We need the main file path to so that we can get the real main-file
header.

Diff Detail

Event Timeline

hokein created this revision.Jul 11 2023, 4:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 4:44 AM
hokein requested review of this revision.Jul 11 2023, 4:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 11 2023, 4:44 AM
kadircet accepted this revision.Jul 11 2023, 5:05 AM
kadircet added inline comments.
clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
303–304

can you also have a test in which we insert "d.h" and it gets put into the right place?

This revision is now accepted and ready to land.Jul 11 2023, 5:05 AM
hokein updated this revision to Diff 539045.Jul 11 2023, 5:56 AM

add a testcase.

clang-tools-extra/include-cleaner/unittests/AnalysisTest.cpp
303–304

added a test with a FIXME -- unfortunately, this case doesn't work yet (I believe this is a bug in the tooling::HeaderIncludes, I plan to fix it afterwards.)

This revision was landed with ongoing or failed builds.Jul 11 2023, 6:02 AM
This revision was automatically updated to reflect the committed changes.