Page MenuHomePhabricator

[Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics.
ClosedPublic

Authored by Bigcheese on Oct 4 2019, 4:58 PM.

Diff Detail

Event Timeline

Bigcheese created this revision.Oct 4 2019, 4:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptOct 4 2019, 4:58 PM
Herald added a subscriber: dexonsmith. · View Herald Transcript
Bigcheese updated this revision to Diff 223345.Oct 4 2019, 5:31 PM
Bigcheese retitled this revision from [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths. to [Implicit Modules] Add -cc1 option -fmodules-strict-hash which includes search paths and diagnostics..

Add diagnostics.

bruno added a comment.Oct 14 2019, 9:51 AM

Hi Michael, thanks for working on this!

include/clang/Lex/HeaderSearchOptions.h
206 ↗(On Diff #223345)

*whether

209 ↗(On Diff #223345)

What else do you plan to add in the future as part of "all the things that could impact"? It seems to me that by default this should always be the case, but header search related things are special because one might want to handwave on correctness to have smaller caches (default behavior right now).

I wonder if we should instead have fmodules-strict-header-seach and later on add a more generic thing that group such cases? WDYT?

Bigcheese marked an inline comment as done.Oct 14 2019, 1:51 PM
Bigcheese added inline comments.
include/clang/Lex/HeaderSearchOptions.h
209 ↗(On Diff #223345)

This also includes diagnostic options. I'm not sure it's useful for users to pick each individual thing they care about for the hash. The intent here is just to capture every possible difference we find.

Bigcheese updated this revision to Diff 225326.Wed, Oct 16, 4:04 PM
Bigcheese marked an inline comment as done.

Fixed spelling and updated comment.

bruno accepted this revision.Thu, Oct 17, 2:51 PM

LGTM with one minor change. Can you add an entry in the modules docs for this flag and mention that using it can lead to more PCMs in an implicit build?

This revision is now accepted and ready to land.Thu, Oct 17, 2:51 PM
bruno added a comment.Thu, Oct 17, 2:53 PM

*using implicit modules in a build where compiler flags in different invocations aren't homogeneous, or something along those lines.

While adding the documentation I realized that a better name for this option would be -fmodules-strict-context-hash to make it clear which hash it's referring to.

bruno added a comment.Fri, Oct 18, 6:28 AM

While adding the documentation I realized that a better name for this option would be -fmodules-strict-context-hash to make it clear which hash it's referring to.

-fmodules-strict-context-hash SGTM!

This revision was automatically updated to reflect the committed changes.

Looks like this fails on win: http://45.33.8.238/win/841/step_6.txt

Ptal!

Maybe just cat'ing all files instead of echoing the first and piping into cat works?