Skip to content

Conversation

@akyrtzi
Copy link

@akyrtzi akyrtzi commented Apr 20, 2023

This cherry-picks https://reviews.llvm.org/D148369 along with adding a commit to deal with NeededByPCHOrCompilationUsesPCH (that only exists in apple fork) so the upstream test can actually pass.

akyrtzi added 2 commits April 20, 2023 15:15
This is particularly useful to avoid diverging the modules between a PCH and a translation-unit compilation.

Differential Revision: https://reviews.llvm.org/D148369
For explicit modules workflow `NeededByPCHOrCompilationUsesPCH` is not a
meaningful difference for creation of explicit modules and we canonicalize it away.
Treat it as benign for the purposes of AST reading, when in the context of explicit modules workflow.

Note this commit makes `test/ClangScanDeps/shared-module-for-tu-and-pch.c` pass, which is why
it is not accompanied by its own test.
bool ExistingNeededByPCHOrCompilationUsesPCH =
ExistingLangOpts.NeededByPCHOrCompilationUsesPCH;
if (IsExplicitModulesWorkflow) {
// Match NeededByPCHOrCompilationUsesPCH to make it "benign".
Copy link
Author

Choose a reason for hiding this comment

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

I don't particularly like this hack but I wanted to minimize the differences from upstream when dealing with NeededByPCHOrCompilationUsesPCH.

@akyrtzi
Copy link
Author

akyrtzi commented Apr 21, 2023

I had offline discussion with @jansvoboda11 and he had a suggestion for the build system to treat the prefix header as a module, which would avoid all these hacks that we have to deal with.
I'm putting this PR on draft for now, time may be better spent to explore this kind of approach.

@akyrtzi akyrtzi marked this pull request as draft April 21, 2023 00:57
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.

1 participant