This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Include skipped headers in the --trace-includes output
ClosedPublic

Authored by ldionne on Sep 28 2022, 1:13 PM.

Details

Summary

By default, Clang does not include headers that are skipped due to
the include guard optimization in the --trace-includes output, which
breaks the use case that we were trying to use it for.

However, Clang does support the -fshow-skipped-includes flag, which
does exactly what we need and will result in an accurate include
graph.

Diff Detail

Event Timeline

ldionne created this revision.Sep 28 2022, 1:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 28 2022, 1:13 PM
ldionne requested review of this revision.Sep 28 2022, 1:13 PM
Herald added a project: Restricted Project. · View Herald TranscriptSep 28 2022, 1:13 PM
Herald added a reviewer: Restricted Project. · View Herald Transcript

@var-const Can you try applying this change locally and then re-applying your copy patch to see if this solves the issues you were seeing?

Did you look at the effect the change has on the Graphviz ouput?

@var-const Can you try applying this change locally and then re-applying your copy patch to see if this solves the issues you were seeing?

I expect that there will still be an issue, due to the removal of cstring in format and valarray.

I want to have a closer look tomorrow and play with this patch myself.

libcxx/test/libcxx/transitive_includes.sh.cpp
563

You mention you dislike the transitive includes name, should we change it now?

ldionne marked an inline comment as done.Sep 29 2022, 5:28 AM
ldionne added inline comments.
libcxx/test/libcxx/transitive_includes.sh.cpp
563

I would do that in a separate patch.

ldionne marked an inline comment as done.Sep 29 2022, 6:32 AM

Did you look at the effect the change has on the Graphviz ouput?

I just did, and it looks just as overwhelming as it did before :-). For real though, I think it's only going to be more accurate after this patch, since we will faithfully represent the dependencies in an order-independent way. Previously, the output would be dependent of the order in which headers had their #includes, which made little sense.

@var-const Can you try applying this change locally and then re-applying your copy patch to see if this solves the issues you were seeing?

I expect that there will still be an issue, due to the removal of cstring in format and valarray.

Yes, of course, but I think he knows that that part is a real issue.

ldionne updated this revision to Diff 463871.Sep 29 2022, 6:36 AM

Fix output differences in -fno-exceptions mode, and disable for older AppleClang

Mordante accepted this revision.Sep 29 2022, 8:46 AM

I've downloaded the patch and had a look at the raw generated files with the -fshow-skipped-includes flag. With this additional information I think we can look at some other improvements. Not sure whether we want them in these files, but we can in the .dot file. I'll message you on Discord to discuss that. Obviously that's out of the scope of this patch.

Thanks for finding -fshow-skipped-includes, this makes the output better.

LGTM!

This revision is now accepted and ready to land.Sep 29 2022, 8:46 AM
This revision was landed with ongoing or failed builds.Sep 29 2022, 12:07 PM
This revision was automatically updated to reflect the committed changes.