[PATCH] D106876: Remove non-affecting module maps from PCM files.

dexonsmith added a comment. In D106876#3869447 , @jansvoboda11 wrote: > I agree that avoiding serializing non-affecting input files is the better > approach. Your code is more or less what I had in mind, thanks for sketching > it out! > The number of i

[PATCH] D106876: Remove non-affecting module maps from PCM files.

jansvoboda11 added a comment. I agree that avoiding serializing non-affecting input files is the better approach. Your code is more or less what I had in mind, thanks for sketching it out! The number of ignored module maps is typically around 70 - 110 on macOS (when you allow system module maps

[PATCH] D106876: Remove non-affecting module maps from PCM files.

dexonsmith added a comment. In D106876#3869247 , @dexonsmith wrote: > A further (more involved) approach would be to separate module maps into a > separate SourceManager, so that their source locations don't affect other > input files. Then only module

[PATCH] D106876: Remove non-affecting module maps from PCM files.

dexonsmith added a comment. As an end goal, it seems strictly better for build systems / distribution / artifact storage / caching if the output PCM has been canonicalized as-if the module maps weren't found, rather than tracking both which module maps were discovered and which to ignore. Ideal

[PATCH] D106876: Remove non-affecting module maps from PCM files.

jansvoboda11 added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:1549 +if (isModuleMap(File.getFileCharacteristic()) && +!isSystem(File.getFileCharacteristic()) && +!AffectingModuleMaps.empty() && Why do we never consider

[PATCH] D106876: Remove non-affecting module maps from PCM files.

jansvoboda11 added inline comments. Comment at: clang/lib/Serialization/ASTWriter.cpp:183-184 +HS.getExistingFileInfo(File, /*WantExternal*/false); +if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader)) + continue; + rsmith wrote: >

[PATCH] D106876: Remove non-affecting module maps from PCM files.

jansvoboda11 added a comment. Herald added a project: All. Hi @ilyakuteev, this patch is causing some issues downstream. (See `clang/test/Modules/non-affecting-module-maps-source-locations.m` here .) I think they all boil down to the fact that a

[PATCH] D106876: Remove non-affecting module maps from PCM files.

This revision was automatically updated to reflect the committed changes. Closed by commit rG8a4fcfc242a0: Remove non-affecting module maps from PCM files. (authored by ilyakuteev, committed by jansvoboda11). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. Hi, I am very glad for your help. :) Author name: `Ilya Kuteev` email: `ilyakut...@icloud.com` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 _

[PATCH] D106876: Remove non-affecting module maps from PCM files.

jansvoboda11 added a comment. In D106876#3066924 , @ilyakuteev wrote: > Hello everyone, can somebody please help me commit this patch to trunk? Hi, I'm happy to commit this for you. What author name and email would you like to be associated with the co

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. Hello everyone, can somebody please help me commit this patch to trunk? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 ___ cfe-comm

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. @rsmith can you please help with committing patch to trunk? Do not sure if I have correct rights to do it. Were are several failing tests, seems like it is caused by bugs in pre-merge checks (For example this one on win: https://github.com/google/llvm-premerge-checks

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 375272. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/M

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 375187. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 374173. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 374156. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373957. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373936. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373862. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373861. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373859. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. Hello everyone! Ping :), need more approves for this patch. @rsmith Thank you for your helpful tips. Can you please review the latest change? I fixed some tests and applied clang format suggestions. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373179. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373076. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 373048. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 372989. ilyakuteev marked 5 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/

[PATCH] D106876: Remove non-affecting module maps from PCM files.

rsmith accepted this revision. rsmith added a comment. This revision is now accepted and ready to land. Please apply the lint suggestions; otherwise, looks good. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 _

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 372563. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp clang/test/Modules/Inputs/AddRemoveIrrelevantModule

[PATCH] D106876: Remove non-affecting module maps from PCM files.

rsmith added a comment. This should have a testcase. I would imagine you can test this by constructing a situation where you build a module twice with different sets of module map files being considered and checking that you get bit-for-bit identical outputs. (For example: set up a temporary ar

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. Hello everyone! We have very slow remote compilations without this patch. :(. I'll be very grateful for your review. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 ___

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. Ping CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. @rsmith Can you please check is this a correct way to add headers known to preprocessor? (Lines 173-191) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 ___ cfe-commits m

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev added a comment. Added modules for headers known to HeadersSearch, including textual ones. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 ___ cfe-commits mailing list cfe-commits@lists.ll

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 365969. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp Index: clang/lib/Serialization/ASTWriter.cpp ===

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 365968. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Lex/HeaderSearch.h clang/include/clang/Serialization/ASTWriter.h clang/lib/S

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 365381. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp Index: clang/

[PATCH] D106876: Remove non-affecting module maps from PCM files.

rsmith added a comment. I think module maps that specify textual headers should be considered inputs if we entered any of those textual headers; we won't treat those as being imported. In addition to considering the current module and its imports, perhaps we should walk the preprocessor's heade

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev updated this revision to Diff 362348. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106876/new/ https://reviews.llvm.org/D106876 Files: clang/include/clang/Serialization/ASTWriter.h clang/lib/Serialization/ASTWriter.cpp Index: clang/

[PATCH] D106876: Remove non-affecting module maps from PCM files.

ilyakuteev created this revision. ilyakuteev added reviewers: aprantl, dblaikie, rsmith, dexonsmith. ilyakuteev requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Problem: PCM file includes references to all module maps used in compilation whi