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

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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 ignored module maps is typically around 70 - 110 on macOS (when 
> you allow system module maps to be treated as non-affecting), and those are 
> at the start of the `SourceManager` block. I might implement this approach 
> and test out if there's a noticeable performance impact. Maybe we could use 
> something similar to the optimization in `SourceManager::getFileIDLoaded()`.

Interesting. In that case it'd probably be profitable to do a pass after 
sorting to merge adjacent SourceLoc ranges. If there are many ignored system 
modules they may well be immediately adjacent and they'd resolve to a single 
range.

> I remember having a separate `SourceManager` came up before, when 
> investigating other issues. Though I think you want to merge `SourceManager`s 
> earlier than on serialization. I think other data structures might hold a 
> `SourceLocation` pointing into the module-map-specific `SourceManager`. How 
> can you tell which `SourceManager` it came from?

I'm curious whether this was a problem before, when there were two 
SourceManagers (before they were combined into one). It's possible that it's 
just obvious from context which `SourceManager` to use; that there isn't 
ambiguity in practice (because of where the SourceLocs are stored, or the 
source language, or something?).

Note that fully splitting the source manager might be worth doing (eventually) 
as a follow up, even if there aren't performance problems with the remapping 
approach. (Unless this patch-and-the-bug-fixes will address all the performance 
problems? I don't think it totally solves the quadratic use of SourceLocation 
bit space by module maps, but maybe it does?)

> This could be prevented by merging the module-map-specific `SourceManager` 
> into the main one when returning non-null result from `Module 
> *ModuleMap::lookupModule(StringRef Name)`, were that the only way to get a 
> module.

Ah, yeah, on-demand, but sooner than I was thinking. SGTM!


Repository:
  rG LLVM Github Monorepo

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.

[Jan Svoboda via Phabricator via cfe-commits]

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 to be treated as non-affecting), and those are at 
the start of the `SourceManager` block. I might implement this approach and 
test out if there's a noticeable performance impact. Maybe we could use 
something similar to the optimization in `SourceManager::getFileIDLoaded()`.

I remember having a separate `SourceManager` came up before, when investigating 
other issues. Though I think you want to merge `SourceManager`s earlier than on 
serialization. I think other data structures might hold a `SourceLocation` 
pointing into the module-map-specific `SourceManager`. How can you tell which 
`SourceManager` it came from? This could be prevented by merging the 
module-map-specific `SourceManager` into the main one when returning non-null 
result from `Module *ModuleMap::lookupModule(StringRef Name)`, were that the 
only way to get a module.


Repository:
  rG LLVM Github Monorepo

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.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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 map source locations would need to be 
> translated during serialization. (Now that FileManager has the capability to 
> remap file contents, I think the commit that merged the SourceManagers could 
> be effectively reverted.)

Throwing another idea out there... As a sort of compromise to avoid changing 
serialization/deserialization too much, maybe a hybrid could make sense:

- During compilation, keep newly-found/discovered module maps in a separate 
SourceManager.
- Translate all serialized module map locations to the main SourceManager 
during serialization. Could do this up front (collected non-ignored module 
maps) or maybe even on-demand (need to translate this location so add it to the 
main source manager).

(Or something along these lines...)


Repository:
  rG LLVM Github Monorepo

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.

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

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. Ideally, we'd find a way to canonicalize the 
output efficiently and correctly... along the lines of this patch, but with a 
bug fix somewhere.

It may not be too hard/expensive to translate the source locations. E.g., you 
could build up a vector:

  SmallVector DroppedMMs;
  
  // fill up DroppedMMs with MMs...
  ...
  
  // sort to create map.
  sort(DroppedMMs);
  
  // accumulate offsets.
  SmallVector AccumulatedOffset;
  uint64_t Total = 0;
  AccumulatedOffset.reserve(DroppedMMs.size() + 1);
  AccumulatedOffset.push_back(0);
  for (MM : DroppedMMs)
AccumulatedOffset.push_back(Total += MM.size());
  
  // later, translate during serialization
  Error serializeSourceLoc(SourceLoc SL) {
if (DroppedMMs.empty())
  return SerializeSourceLocImpl(SL);
auto I = llvm::lower_bound(DroppedMMs, SL);
assert((I == MMs.end() || !I->contains(SL)) &&
   "Serializing a location from an ignored module map???");
return serializeSourceLocImpl(SL - AccumulatedOffset[I - MMs.begin()]);
  }

Then a `std::lower_bound` into `DroppedMMs` tells you how much to adjust any 
given SourceLoc by. Serializing a source location would go through this map. 
Presumably, the number of dropped files will be relatively small (number of 
ignored module maps) so the binary search should be fast. Probably there would 
be good peepholes for common cases (such as tracking `LastDroppedMM` to avoid 
repeating the same search).

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 map source locations would need to be translated during 
serialization. (Now that FileManager has the capability to remap file contents, 
I think the commit that merged the SourceManagers could be effectively 
reverted.)


Repository:
  rG LLVM Github Monorepo

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.

[Jan Svoboda via Phabricator via cfe-commits]

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 system module maps non-affecting? (i.e. always 
consider them affecting)


Repository:
  rG LLVM Github Monorepo

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.

[Jan Svoboda via Phabricator via cfe-commits]

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:
> I don't think this is right: I think we should consider every file that we've 
> entered in this compilation, regardless of whether it's part of a module 
> we're compiling. (We support modes where we'll enter modular headers despite 
> not compiling them.) Can you replace this with just `if (!HFI) continue;` and 
> still get the optimization you're looking for?
@rsmith It appears that between your approval and when the patch got committed, 
your suggested change was reverted. Can you please explain the situation where 
we enter modular headers without compiling them?


Repository:
  rG LLVM Github Monorepo

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.

[Jan Svoboda via Phabricator via cfe-commits]

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 `SourceLocation` is a simple 
offset into `SourceManager`'s buffer of concatenated input files. By not 
serializing some input files, we make that buffer shorter by leaving out some 
of its parts. This means that a `SourceLocation` that used to point at the end 
of the last file might now be pointing outside the new, shorter buffer. More 
generally, any `SourceLocation` pointing after any of the removed input files 
is now bogus. It seems that we should be computing the set of non-affecting 
input files at the start of serialization, and then any `SourceLocation` we 
serialize, we need to adjust to account for the changes in the input file 
buffer. I'm not sure how efficiently we can implement this, given that we might 
need to adjust every `SourceLocation`. And we have lots of them.

We rely on this feature for "implicitly discovered, explicitly built modules". 
For our purpose, the (simpler) downstream patch changes the PCM file so that it 
still contains all input files, preventing the issue I just described, but 
includes a bit that says whether a file was affecting or not. My question is: 
would that approach work for your use-case? You mentioned you want to avoid 
rebuilds in distributed compilation when some non-affecting input files are 
missing. You could avoid that by not validating non-affecting input files in 
`ASTReader`. One potential issue I see with this approach is that two PCMs for 
module M produced on two machines with different sets of non-affecting input 
files are not interchangeable. They will have different contents and therefore 
different signature. This could lead to rebuilds of importers that expect one 
signature for PCM M but (after some distributed re-shuffling), the other PCM 
ends up on the machine, creating a mismatch between the expected and actual PCM 
signature.

Please, let me know your thoughts.


Repository:
  rG LLVM Github Monorepo

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.

[Jan Svoboda via Phabricator via cfe-commits]

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.org/D106876/new/

https://reviews.llvm.org/D106876

Files:
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -161,6 +161,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, 

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

[Ilya Kuteev via Phabricator via cfe-commits]

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

___
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.

[Jan Svoboda via Phabricator via cfe-commits]

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 commit?


Repository:
  rG LLVM Github Monorepo

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.

[Ilya Kuteev via Phabricator via cfe-commits]

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-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.

[Ilya Kuteev via Phabricator via cfe-commits]

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/issues/353). Thanks!


Repository:
  rG LLVM Github Monorepo

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.

[Ilya Kuteev via Phabricator via cfe-commits]

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/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+for (const auto  : 

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+for (const auto  : HS.findAllModulesForHeader(File)) {
+  if (!KH.getModule())
+

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+for (const auto  : HS.findAllModulesForHeader(File)) {
+  if (!KH.getModule())
+

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+for (const auto  : HS.findAllModulesForHeader(File)) {
+  if (!KH.getModule())
+

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+for (const auto  : HS.findAllModulesForHeader(File)) {
+  if (!KH.getModule())
+continue;
+  ModulesToProcess.push_back(KH.getModule());
+}
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto *CurrentModule = ModulesToProcess.pop_back_val();
+ProcessedModules.insert(CurrentModule);
+
+auto *ModuleMapFile =
+HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
+if (!ModuleMapFile) {

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m

Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+for (const auto  : HS.findAllModulesForHeader(File)) {
+  if (!KH.getModule())
+continue;
+  ModulesToProcess.push_back(KH.getModule());
+}
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto *CurrentModule = ModulesToProcess.pop_back_val();
+ProcessedModules.insert(CurrentModule);
+
+auto *ModuleMapFile =
+HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
+if (!ModuleMapFile) {
+  continue;
+}
+
+ModuleMaps.insert(ModuleMapFile);
+
+for (auto *ImportedModule : (CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.push_back(ImportedModule);
+}
+  }
+
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1449,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModuleMaps;
+  if (WritingModule) {
+AffectingModuleMaps =
+GetAllModuleMaps(PP.getHeaderSearchInfo(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModuleMaps);
   Stream.ExitBlock();
 }
 
@@ -1416,9 +1475,9 @@
 
 } // namespace
 
-void ASTWriter::WriteInputFiles(SourceManager ,
-HeaderSearchOptions ,
-bool Modules) {
+void ASTWriter::WriteInputFiles(
+SourceManager , HeaderSearchOptions ,
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1517,16 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(Cache->OrigEntry) ==
+AffectingModuleMaps.end()) {
+  SkippedModuleMaps.insert(Cache->OrigEntry);
+  // Do not emit modulemaps that do not affect current module.
+  

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m

Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,60 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto  : KnownHeaders) {
+  if (!KH.getModule())
+continue;
+  ModulesToProcess.push_back(KH.getModule());
+}
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto *CurrentModule = ModulesToProcess.pop_back_val();
+ProcessedModules.insert(CurrentModule);
+
+auto *ModuleMapFile =
+HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
+if (!ModuleMapFile) {
+  continue;
+}
+
+ModuleMaps.insert(ModuleMapFile);
+
+for (auto *ImportedModule : (CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.push_back(ImportedModule);
+}
+  }
+
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1450,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModuleMaps;
+  if (WritingModule) {
+AffectingModuleMaps =
+GetAllModuleMaps(PP.getHeaderSearchInfo(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModuleMaps);
   Stream.ExitBlock();
 }
 
@@ -1416,9 +1476,9 @@
 
 } // namespace
 
-void ASTWriter::WriteInputFiles(SourceManager ,
-HeaderSearchOptions ,
-bool Modules) {
+void ASTWriter::WriteInputFiles(
+SourceManager , HeaderSearchOptions ,
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1518,19 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+std::find_if(AffectingModuleMaps.begin(), AffectingModuleMaps.end(),
+ [&](const auto ) {
+   return FE->getUniqueID() ==
+  

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap

Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,60 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto  : KnownHeaders) {
+  if (!KH.getModule())
+continue;
+  ModulesToProcess.push_back(KH.getModule());
+}
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto *CurrentModule = ModulesToProcess.pop_back_val();
+ProcessedModules.insert(CurrentModule);
+
+auto *ModuleMapFile =
+HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
+if (!ModuleMapFile) {
+  continue;
+}
+
+ModuleMaps.insert(ModuleMapFile);
+
+for (auto *ImportedModule : (CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.push_back(ImportedModule);
+}
+  }
+
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1450,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModuleMaps;
+  if (WritingModule) {
+AffectingModuleMaps =
+GetAllModuleMaps(PP.getHeaderSearchInfo(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModuleMaps);
   Stream.ExitBlock();
 }
 
@@ -1416,9 +1476,9 @@
 
 } // namespace
 
-void ASTWriter::WriteInputFiles(SourceManager ,
-HeaderSearchOptions ,
-bool Modules) {
+void ASTWriter::WriteInputFiles(
+SourceManager , HeaderSearchOptions ,
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1518,19 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+std::find_if(AffectingModuleMaps.begin(), AffectingModuleMaps.end(),
+ [&](const auto ) {
+   return FE->getUniqueID() ==
+

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,60 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto  : KnownHeaders) {
+  if (!KH.getModule())
+

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/

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.

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build without b.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build with b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto  : KnownHeaders) {
+  if (!KH.getModule())
+

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build with a.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build without b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto  : KnownHeaders) {
+  if (!KH.getModule())
+

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,7 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery -fmodule-map-file=%S/Inputs/compare.modulemap
 
 struct CC { CC(...); };
 
@@ -24,10 +16,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build with a.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build without b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto : KnownHeaders) {
+  if (!KH.getModule())
+continue;

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m
  clang/test/SemaCXX/Inputs/compare.modulemap
  clang/test/SemaCXX/compare-modules-cxx2a.cpp

Index: clang/test/SemaCXX/compare-modules-cxx2a.cpp
===
--- clang/test/SemaCXX/compare-modules-cxx2a.cpp
+++ clang/test/SemaCXX/compare-modules-cxx2a.cpp
@@ -1,15 +1,8 @@
-// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -I%S/Inputs %s -fno-modules-error-recovery
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
 
-#pragma clang module build compare
-module compare {
-  explicit module cmp {}
-  explicit module other {}
-}
-#pragma clang module contents
-#pragma clang module begin compare.cmp
-#include "std-compare.h"
-#pragma clang module end
-#pragma clang module endbuild
+// RUN: %clang_cc1 -triple x86_64-apple-darwin -fcxx-exceptions -verify -std=c++2a -fmodules -fmodules-cache-path=%t.mcp -I%S/Inputs %s -fno-modules-error-recovery
 
 struct CC { CC(...); };
 
@@ -24,10 +17,10 @@
 
 // expected-note@std-compare.h:* 2+{{not reachable}}
 
-void b() { void(0 <=> 0); } // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+void b() { void(0 <=> 0); } // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 
 struct B {
-  CC operator<=>(const B&) const = default; // expected-error 1+{{missing '#include "std-compare.h"'; 'strong_ordering' must be defined}}
+  CC operator<=>(const B&) const = default; // expected-error 1+{{definition of 'strong_ordering' must be imported from module 'compare.cmp' before it is required}}
 };
 auto vb = B() <=> B(); // expected-note {{required here}}
 
Index: clang/test/SemaCXX/Inputs/compare.modulemap
===
--- /dev/null
+++ clang/test/SemaCXX/Inputs/compare.modulemap
@@ -0,0 +1,6 @@
+module compare {
+  explicit module cmp {
+header "std-compare.h"
+  }
+  explicit module other {}
+}
Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build with a.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build without b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto : KnownHeaders) {
+  if (!KH.getModule())

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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

___
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.

[Ilya Kuteev via Phabricator via cfe-commits]

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/AddRemoveIrrelevantModuleMap/a.modulemap
  clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
  clang/test/Modules/add-remove-irrelevant-mobile-map.m

Index: clang/test/Modules/add-remove-irrelevant-mobile-map.m
===
--- /dev/null
+++ clang/test/Modules/add-remove-irrelevant-mobile-map.m
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t
+// RUN: rm -rf %t.mcp
+// RUN: mkdir -p %t
+
+// Build with a.modulemap
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap %s -verify
+// RUN: cp %t.mcp/a.pcm %t/a.pcm
+
+// Build without b.modulemap
+// RUN: rm -rf %t.mcp
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap -fmodule-map-file=%S/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap %s -verify
+// RUN: not diff %t.mcp/a.pcm %t/a.pcm
+
+// expected-no-diagnostics
+
+@import a;
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/b.modulemap
@@ -0,0 +1 @@
+module b { }
Index: clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
===
--- /dev/null
+++ clang/test/Modules/Inputs/AddRemoveIrrelevantModuleMap/a.modulemap
@@ -0,0 +1 @@
+module a { }
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,59 @@
 
 namespace {
 
+std::set GetAllModuleMaps(const HeaderSearch ,
+ Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  SmallVector ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/ false);
+if (!HFI)
+  continue;
+
+const auto KnownHeaders = HS.findAllModulesForHeader(File);
+for (const auto& KH: KnownHeaders) {
+  if (!KH.getModule())
+continue;
+  ModulesToProcess.push_back(KH.getModule());
+}
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto CurrentModule = ModulesToProcess.pop_back_val();
+ProcessedModules.insert(CurrentModule);
+
+auto *ModuleMapFile =
+HS.getModuleMap().getModuleMapFileForUniquing(CurrentModule);
+if (!ModuleMapFile) {
+  continue;
+}
+
+ModuleMaps.insert(ModuleMapFile);
+
+for (auto *ImportedModule : (CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.push_back(ImportedModule);
+}
+  }
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1449,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModuleMaps;
+  if (WritingModule) {
+AffectingModuleMaps =
+GetAllModuleMaps(PP.getHeaderSearchInfo(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModuleMaps);
   Stream.ExitBlock();
 }
 
@@ -1416,9 +1475,10 @@
 
 } // namespace
 
-void ASTWriter::WriteInputFiles(SourceManager ,
-HeaderSearchOptions ,
-bool Modules) {
+void ASTWriter::WriteInputFiles(
+SourceManager ,
+HeaderSearchOptions ,
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1518,16 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(Cache->OrigEntry) ==
+AffectingModuleMaps.end()) {
+  SkippedModuleMaps.insert(Cache->OrigEntry);
+  // Do not emit modulemaps that do not affect current module.
+  continue;

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

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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 area for the test case files, build a module 
there, then copy in an irrelevant module map file into a directory named by a 
`-I` and build the module again.)




Comment at: clang/lib/Serialization/ASTWriter.cpp:164
+
+std::set GetAllModulemaps(const HeaderSearch ,
+   Module *RootModule) {

`Modulemap` -> `ModuleMap` for consistency please.



Comment at: clang/lib/Serialization/ASTWriter.cpp:168
+  std::set ProcessedModules;
+  std::set ModulesToProcess{RootModule};
+

You're walking this list in a non-deterministic order; consider using a 
`SmallVector` here and popping elements from the end instead of the front in 
the loop below (ie, depth-first traversal instead of breadth-first).



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

I don't think this is right: I think we should consider every file that we've 
entered in this compilation, regardless of whether it's part of a module we're 
compiling. (We support modes where we'll enter modular headers despite not 
compiling them.) Can you replace this with just `if (!HFI) continue;` and still 
get the optimization you're looking for?



Comment at: clang/lib/Serialization/ASTWriter.cpp:186
+
+const auto KH = HS.findModuleForHeader(File, /*AllowTextual*/true);
+if (!KH.getModule())

This should be `findAllModulesForHeader`: in the case where there is more than 
one module covering a header, the additional module maps can matter in some 
contexts.



Comment at: clang/lib/Serialization/ASTWriter.cpp:1537-1538
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(std::string(Cache->OrigEntry->getName())) ==
+AffectingModuleMaps.end()) {
+  // Do not emit modulemaps that do not affect current module.

This will not work correctly if a module map file is reachable via multiple 
different paths. Consider using `FileEntry*` comparisons instead here.


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.

[Ilya Kuteev via Phabricator via cfe-commits]

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

___
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.

[Ilya Kuteev via Phabricator via cfe-commits]

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.

[Ilya Kuteev via Phabricator via cfe-commits]

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 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.

[Ilya Kuteev via Phabricator via cfe-commits]

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.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Ilya Kuteev via Phabricator via cfe-commits]

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
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,72 @@
 
 namespace {
 
+std::string ModuleMapFilePathForModule(const ModuleMap ,
+   Module *GivenModule) {
+  if (!GivenModule->PresumedModuleMapFile.empty()) {
+return GivenModule->PresumedModuleMapFile;
+  }
+  auto *ModuleMapFile = Map.getModuleMapFileForUniquing(GivenModule);
+  if (!ModuleMapFile) {
+return std::string();
+  }
+  return std::string(ModuleMapFile->getName());
+}
+
+std::set GetAllModulemaps(const HeaderSearch ,
+   Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  std::set ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+const auto KH = HS.findModuleForHeader(File, /*AllowTextual*/true);
+if (!KH.getModule())
+  continue;
+
+ModulesToProcess.insert(KH.getModule());
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto CurrentModule = ModulesToProcess.begin();
+ProcessedModules.insert(*CurrentModule);
+
+const std::string CurrentModuleMapFile =
+ModuleMapFilePathForModule(HS.getModuleMap(), *CurrentModule);
+if (CurrentModuleMapFile.empty()) {
+  ModulesToProcess.erase(CurrentModule);
+  continue;
+}
+
+ModuleMaps.insert(CurrentModuleMapFile);
+
+for (auto *ImportedModule : (*CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.insert(ImportedModule);
+}
+ModulesToProcess.erase(CurrentModule);
+  }
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1462,16 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModulemaps;
+  if (WritingModule) {
+AffectingModulemaps = GetAllModulemaps(
+PP.getHeaderSearchInfo(),
+WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModulemaps);
   Stream.ExitBlock();
 }
 
@@ -1418,7 +1491,7 @@
 
 void ASTWriter::WriteInputFiles(SourceManager ,
 HeaderSearchOptions ,
-bool Modules) {
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1531,15 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(std::string(Cache->OrigEntry->getName())) ==
+AffectingModuleMaps.end()) {
+  // Do not emit modulemaps that do not affect current module.
+  continue;
+}
+
 InputFileEntry Entry;
 Entry.File = Cache->OrigEntry;
 Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
@@ -1971,11 +2053,15 @@
 Record.push_back(SLoc->getOffset() - 2);
 if (SLoc->isFile()) {
   const SrcMgr::FileInfo  = SLoc->getFile();
+  const SrcMgr::ContentCache *Content = ();
+  if (Content->OrigEntry && InputFileIDs[Content->OrigEntry] == 0) {
+// Do not emit files that were not listed as inputs.
+continue;
+  }
   AddSourceLocation(File.getIncludeLoc(), Record);
   Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
   Record.push_back(File.hasLineDirectives());
 
-  const SrcMgr::ContentCache *Content = ();
   bool EmitBlob = false;
   if (Content->OrigEntry) {
 assert(Content->OrigEntry == Content->ContentsEntry &&
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -475,7 +475,7 @@
   createSignature(StringRef AllBytes, StringRef ASTBlockBytes);
 
   

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,72 @@
 
 namespace {
 
+std::string ModuleMapFilePathForModule(const ModuleMap ,
+   Module *GivenModule) {
+  if (!GivenModule->PresumedModuleMapFile.empty()) {
+return GivenModule->PresumedModuleMapFile;
+  }
+  auto *ModuleMapFile = Map.getModuleMapFileForUniquing(GivenModule);
+  if (!ModuleMapFile) {
+return std::string();
+  }
+  return std::string(ModuleMapFile->getName());
+}
+
+std::set GetAllModulemaps(const HeaderSearch ,
+   Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  std::set ModulesToProcess{RootModule};
+
+  SmallVector FilesByUID;
+  HS.getFileMgr().GetUniqueIDMapping(FilesByUID);
+
+  if (FilesByUID.size() > HS.header_file_size())
+FilesByUID.resize(HS.header_file_size());
+
+  for (unsigned UID = 0, LastUID = FilesByUID.size(); UID != LastUID; ++UID) {
+const FileEntry *File = FilesByUID[UID];
+if (!File)
+  continue;
+
+const HeaderFileInfo *HFI =
+HS.getExistingFileInfo(File, /*WantExternal*/false);
+if (!HFI || (HFI->isModuleHeader && !HFI->isCompilingModuleHeader))
+  continue;
+
+const auto KH = HS.findModuleForHeader(File, /*AllowTextual*/true);
+if (!KH.getModule())
+  continue;
+
+ModulesToProcess.insert(KH.getModule());
+  }
+
+  while (!ModulesToProcess.empty()) {
+auto CurrentModule = ModulesToProcess.begin();
+ProcessedModules.insert(*CurrentModule);
+
+const std::string CurrentModuleMapFile =
+ModuleMapFilePathForModule(HS.getModuleMap(), *CurrentModule);
+if (CurrentModuleMapFile.empty()) {
+  ModulesToProcess.erase(CurrentModule);
+  continue;
+}
+
+ModuleMaps.insert(CurrentModuleMapFile);
+
+for (auto *ImportedModule : (*CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.insert(ImportedModule);
+}
+ModulesToProcess.erase(CurrentModule);
+  }
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1462,16 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModulemaps;
+  if (WritingModule) {
+AffectingModulemaps = GetAllModulemaps(
+PP.getHeaderSearchInfo(),
+WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModulemaps);
   Stream.ExitBlock();
 }
 
@@ -1418,7 +1491,7 @@
 
 void ASTWriter::WriteInputFiles(SourceManager ,
 HeaderSearchOptions ,
-bool Modules) {
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1531,15 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(std::string(Cache->OrigEntry->getName())) ==
+AffectingModuleMaps.end()) {
+  // Do not emit modulemaps that do not affect current module.
+  continue;
+}
+
 InputFileEntry Entry;
 Entry.File = Cache->OrigEntry;
 Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
@@ -1971,11 +2053,15 @@
 Record.push_back(SLoc->getOffset() - 2);
 if (SLoc->isFile()) {
   const SrcMgr::FileInfo  = SLoc->getFile();
+  const SrcMgr::ContentCache *Content = ();
+  if (Content->OrigEntry && InputFileIDs[Content->OrigEntry] == 0) {
+// Do not emit files that were not listed as inputs.
+continue;
+  }
   AddSourceLocation(File.getIncludeLoc(), Record);
   Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
   Record.push_back(File.hasLineDirectives());
 
-  const SrcMgr::ContentCache *Content = ();
   bool EmitBlob = false;
   if (Content->OrigEntry) {
 assert(Content->OrigEntry == Content->ContentsEntry &&
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -475,7 

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

[Ilya Kuteev via Phabricator via cfe-commits]

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/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,46 @@
 
 namespace {
 
+std::string ModuleMapFilePathForModule(ModuleMap , Module *GivenModule) {
+  if (!GivenModule->PresumedModuleMapFile.empty()) {
+return GivenModule->PresumedModuleMapFile;
+  }
+  auto *ModuleMapFile = Map.getModuleMapFileForUniquing(GivenModule);
+  if (!ModuleMapFile) {
+return std::string();
+  }
+  return std::string(ModuleMapFile->getName());
+}
+
+std::set GetAllModulemaps(ModuleMap , Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  std::set ModulesToProcess{RootModule};
+  while (!ModulesToProcess.empty()) {
+auto CurrentModule = ModulesToProcess.begin();
+ProcessedModules.insert(*CurrentModule);
+
+const std::string CurrentModuleMapFile =
+ModuleMapFilePathForModule(Map, *CurrentModule);
+if (CurrentModuleMapFile.empty()) {
+  ModulesToProcess.erase(CurrentModule);
+  continue;
+}
+
+ModuleMaps.insert(CurrentModuleMapFile);
+
+for (auto *ImportedModule : (*CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.insert(ImportedModule);
+}
+ModulesToProcess.erase(CurrentModule);
+  }
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1436,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModulemaps;
+  if (WritingModule) {
+AffectingModulemaps = GetAllModulemaps(
+PP.getHeaderSearchInfo().getModuleMap(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModulemaps);
   Stream.ExitBlock();
 }
 
@@ -1418,7 +1464,7 @@
 
 void ASTWriter::WriteInputFiles(SourceManager ,
 HeaderSearchOptions ,
-bool Modules) {
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1504,15 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(std::string(Cache->OrigEntry->getName())) ==
+AffectingModuleMaps.end()) {
+  // Do not emit modulemaps that do not affect current module.
+  continue;
+}
+
 InputFileEntry Entry;
 Entry.File = Cache->OrigEntry;
 Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
@@ -1971,11 +2026,15 @@
 Record.push_back(SLoc->getOffset() - 2);
 if (SLoc->isFile()) {
   const SrcMgr::FileInfo  = SLoc->getFile();
+  const SrcMgr::ContentCache *Content = ();
+  if (Content->OrigEntry && InputFileIDs[Content->OrigEntry] == 0) {
+// Do not emit files that were not listed as inputs.
+continue;
+  }
   AddSourceLocation(File.getIncludeLoc(), Record);
   Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
   Record.push_back(File.hasLineDirectives());
 
-  const SrcMgr::ContentCache *Content = ();
   bool EmitBlob = false;
   if (Content->OrigEntry) {
 assert(Content->OrigEntry == Content->ContentsEntry &&
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -475,7 +475,7 @@
   createSignature(StringRef AllBytes, StringRef ASTBlockBytes);
 
   void WriteInputFiles(SourceManager , HeaderSearchOptions ,
-   bool Modules);
+   std::set );
   void WriteSourceManagerBlock(SourceManager ,
const Preprocessor );
   void WritePreprocessor(const Preprocessor , bool IsModule);
___
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.

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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 headers list and add the module maps 
corresponding to the modules of all headers that were entered?


Repository:
  rG LLVM Github Monorepo

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.

[Ilya Kuteev via Phabricator via cfe-commits]

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/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,47 @@
 
 namespace {
 
+std::string ModuleMapFilePathForModule(ModuleMap , Module *GivenModule) {
+  if (GivenModule->PresumedModuleMapFile.empty()) {
+auto *ModuleMapFile = Map.getModuleMapFileForUniquing(GivenModule);
+if (!ModuleMapFile) {
+  return std::string();
+}
+return std::string(ModuleMapFile->getName());
+  } else {
+return GivenModule->PresumedModuleMapFile;
+  }
+}
+
+std::set GetAllModulemaps(ModuleMap , Module *RootModule) {
+  std::set ModuleMaps{};
+  std::set ProcessedModules;
+  std::set ModulesToProcess{RootModule};
+  while (!ModulesToProcess.empty()) {
+auto CurrentModule = ModulesToProcess.begin();
+ProcessedModules.insert(*CurrentModule);
+
+const std::string CurrentModuleMapFile =
+ModuleMapFilePathForModule(Map, *CurrentModule);
+if (CurrentModuleMapFile.empty()) {
+  ModulesToProcess.erase(CurrentModule);
+  continue;
+}
+
+ModuleMaps.insert(CurrentModuleMapFile);
+
+for (auto *ImportedModule : (*CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.insert(ImportedModule);
+}
+ModulesToProcess.erase(CurrentModule);
+  }
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1437,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }
 
+  std::set AffectingModulemaps;
+  if (WritingModule) {
+AffectingModulemaps = GetAllModulemaps(
+PP.getHeaderSearchInfo().getModuleMap(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModulemaps);
   Stream.ExitBlock();
 }
 
@@ -1418,7 +1465,7 @@
 
 void ASTWriter::WriteInputFiles(SourceManager ,
 HeaderSearchOptions ,
-bool Modules) {
+std::set ) {
   using namespace llvm;
 
   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1505,15 @@
 if (!Cache->OrigEntry)
   continue;
 
+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(std::string(Cache->OrigEntry->getName())) ==
+AffectingModuleMaps.end()) {
+  // Do not emit modulemaps that do not affect current module.
+  continue;
+}
+
 InputFileEntry Entry;
 Entry.File = Cache->OrigEntry;
 Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
@@ -1971,11 +2027,15 @@
 Record.push_back(SLoc->getOffset() - 2);
 if (SLoc->isFile()) {
   const SrcMgr::FileInfo  = SLoc->getFile();
+  const SrcMgr::ContentCache *Content = ();
+  if (Content->OrigEntry && InputFileIDs[Content->OrigEntry] == 0) {
+// Do not emit files that were not listed as inputs.
+continue;
+  }
   AddSourceLocation(File.getIncludeLoc(), Record);
   Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
   Record.push_back(File.hasLineDirectives());
 
-  const SrcMgr::ContentCache *Content = ();
   bool EmitBlob = false;
   if (Content->OrigEntry) {
 assert(Content->OrigEntry == Content->ContentsEntry &&
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -475,7 +475,7 @@
   createSignature(StringRef AllBytes, StringRef ASTBlockBytes);
 
   void WriteInputFiles(SourceManager , HeaderSearchOptions ,
-   bool Modules);
+   std::set );
   void WriteSourceManagerBlock(SourceManager ,
const Preprocessor );
   void WritePreprocessor(const Preprocessor , bool IsModule);
___
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.

[Ilya Kuteev via Phabricator via cfe-commits]

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 which 
created PCM. This problem leads to PCM-rebuilds in distributed compilations as 
some module maps could be missing in isolated compilation. (For example in our 
distributed build system we create a temp folder for every compilation with 
only modules and headers that are needed for that particular command).

Solution:
Add only affecting module map files to a PCM-file.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D106876

Files:
  clang/include/clang/Serialization/ASTWriter.h
  clang/lib/Serialization/ASTWriter.cpp

Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -149,6 +149,48 @@

 namespace {

+std::string ModuleMapFilePathForModule(ModuleMap , Module* GivenModule) {
+  if (GivenModule->PresumedModuleMapFile.empty()) {
+auto *ModuleMapFile = Map.getModuleMapFileForUniquing(GivenModule);
+if (ModuleMapFile) {
+  return std::string(ModuleMapFile->getName());
+} else {
+  return std::string();
+}
+  } else {
+return GivenModule->PresumedModuleMapFile;
+  }
+}
+
+std::set GetAllModulemaps(ModuleMap , Module *RootModule) {
+  std::set ModuleMaps {};
+  std::set ProcessedModules;
+  std::set ModulesToProcess {RootModule};
+  while (!ModulesToProcess.empty()) {
+auto CurrentModule = ModulesToProcess.begin();
+ProcessedModules.insert(*CurrentModule);
+
+const std::string CurrentModuleMapFile =
+ModuleMapFilePathForModule(Map, *CurrentModule);
+if (CurrentModuleMapFile.empty()) {
+  ModulesToProcess.erase(CurrentModule);
+  continue;
+}
+
+ModuleMaps.insert(CurrentModuleMapFile);
+
+for (auto *ImportedModule : (*CurrentModule)->Imports) {
+  if (!ImportedModule ||
+  ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
+continue;
+  }
+  ModulesToProcess.insert(ImportedModule);
+}
+ModulesToProcess.erase(CurrentModule);
+  }
+  return ModuleMaps;
+}
+
 class ASTTypeWriter {
   ASTWriter 
   ASTWriter::RecordData Record;
@@ -1396,9 +1438,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, origDir);
   }

+  std::set AffectingModulemaps;
+  if (WritingModule) {
+AffectingModulemaps = GetAllModulemaps(
+PP.getHeaderSearchInfo().getModuleMap(), WritingModule);
+  }
+
   WriteInputFiles(Context.SourceMgr,
   PP.getHeaderSearchInfo().getHeaderSearchOpts(),
-  PP.getLangOpts().Modules);
+  AffectingModulemaps);
   Stream.ExitBlock();
 }

@@ -1418,7 +1466,7 @@

 void ASTWriter::WriteInputFiles(SourceManager ,
 HeaderSearchOptions ,
-bool Modules) {
+std::set& AffectingModuleMaps) {
   using namespace llvm;

   Stream.EnterSubblock(INPUT_FILES_BLOCK_ID, 4);
@@ -1458,6 +1506,15 @@
 if (!Cache->OrigEntry)
   continue;

+if (isModuleMap(File.getFileCharacteristic()) &&
+!isSystem(File.getFileCharacteristic()) &&
+!AffectingModuleMaps.empty() &&
+AffectingModuleMaps.find(std::string(Cache->OrigEntry->getName())) ==
+   AffectingModuleMaps.end()) {
+  // Do not emit modulemaps that do not affect current module.
+  continue;
+}
+
 InputFileEntry Entry;
 Entry.File = Cache->OrigEntry;
 Entry.IsSystemFile = isSystem(File.getFileCharacteristic());
@@ -1971,11 +2028,15 @@
 Record.push_back(SLoc->getOffset() - 2);
 if (SLoc->isFile()) {
   const SrcMgr::FileInfo  = SLoc->getFile();
+  const SrcMgr::ContentCache *Content = ();
+  if (Content->OrigEntry && InputFileIDs[Content->OrigEntry] == 0) {
+// Do not emit files that were not listed as inputs.
+continue;
+  }
   AddSourceLocation(File.getIncludeLoc(), Record);
   Record.push_back(File.getFileCharacteristic()); // FIXME: stable encoding
   Record.push_back(File.hasLineDirectives());

-  const SrcMgr::ContentCache *Content = ();
   bool EmitBlob = false;
   if (Content->OrigEntry) {
 assert(Content->OrigEntry == Content->ContentsEntry &&
Index: clang/include/clang/Serialization/ASTWriter.h
===
--- clang/include/clang/Serialization/ASTWriter.h
+++ clang/include/clang/Serialization/ASTWriter.h
@@ -475,7 +475,7 @@
   createSignature(StringRef AllBytes, StringRef ASTBlockBytes);

   void WriteInputFiles(SourceManager , HeaderSearchOptions ,
-   bool Modules);
+   std::set& AffectingModuleMaps);
   void