[clang][modules] Report module maps affecting `no_undeclared_includes` modules

[jansvoboda11]

Summary

Since D106876, PCM files don't report module maps as input files unless they contributed to the compilation.

Reporting only module maps of (transitively) imported modules is not enough, though. For modules marked with [no_undeclared_includes], other module maps affect the compilation by introducing anti-dependencies.

This patch makes sure such module maps are being reported as input files.

Depends on D120463.

[dexonsmith]

LGTM, with one test suggestion.

clang/test/Modules/add-remove-irrelevant-module-map.m

36–38

Would it be useful to also add a #error or similar inside the #if?

[jansvoboda11]

clang/test/Modules/add-remove-irrelevant-module-map.m

36–38

Yes, that makes sense. I'll add it before comitting.

[Orlando]

Hi, the test add-remove-irrelevant-module-map.m starts failing with this commit on the llvm-clang-x86_64-sie-win builder: https://lab.llvm.org/buildbot/#/builders/216/builds/929. Please can you take a look when you get the chance?

[jansvoboda11]

In D120464#3363258, @Orlando wrote:

Hi, the test add-remove-irrelevant-module-map.m starts failing with this commit on the llvm-clang-x86_64-sie-win builder: https://lab.llvm.org/buildbot/#/builders/216/builds/929. Please can you take a look when you get the chance?

Hi, it's already fixed in https://lab.llvm.org/buildbot/#/builders/216/builds/932.

[Orlando]

In D120464#3363274, @jansvoboda11 wrote:

In D120464#3363258, @Orlando wrote:

Hi, the test add-remove-irrelevant-module-map.m starts failing with this commit on the llvm-clang-x86_64-sie-win builder: https://lab.llvm.org/buildbot/#/builders/216/builds/929. Please can you take a look when you get the chance?

Hi, it's already fixed in https://lab.llvm.org/buildbot/#/builders/216/builds/932.

Aha, sorry for the noise, thanks!

clang/include/clang/Basic/Module.h

public:
   SmallVector<UnresolvedExportDecl, 2> UnresolvedExports;
 
   /// The directly used modules.
   SmallVector<Module *, 2> DirectUses;
 
   /// The set of use declarations that have yet to be resolved.
   SmallVector<ModuleId, 2> UnresolvedDirectUses;
 
+  /// When \c NoUndeclaredIncludes is true, the set of modules this module tried
+  /// to import but didn't because they are not direct uses.
+  llvm::SmallSetVector<const Module *, 2> UndeclaredUses;
+
   /// A library or framework to link against when an entity from this
   /// module is used.
   struct LinkLibrary {
     LinkLibrary() = default;
     LinkLibrary(const std::string &Library, bool IsFramework)
         : Library(Library), IsFramework(IsFramework) {}
 
     /// The library to link against.
void addTopHeaderFilename(StringRef Filename) {
     TopHeaderNames.push_back(std::string(Filename));
   }
 
   /// The top-level headers associated with this module.
   ArrayRef<const FileEntry *> getTopHeaders(FileManager &FileMgr);
 
   /// Determine whether this module has declared its intention to
   /// directly use another module.
-  bool directlyUses(const Module *Requested) const;
+  bool directlyUses(const Module *Requested);
 
   /// Add the given feature requirement to the list of features
   /// required by this module.
   ///
   /// \param Feature The feature that is required by this module (and
   /// its submodules).
   ///
   /// \param RequiredState The required state of this feature: \c true

clang/lib/Basic/Module.cpp

for (std::vector<std::string>::iterator
         TopHeaders.insert(*FE);
     }
     TopHeaderNames.clear();
   }
 
   return llvm::makeArrayRef(TopHeaders.begin(), TopHeaders.end());
 }
 
-bool Module::directlyUses(const Module *Requested) const {
+bool Module::directlyUses(const Module *Requested) {
   auto *Top = getTopLevelModule();
 
   // A top-level module implicitly uses itself.
   if (Requested->isSubModuleOf(Top))
     return true;
 
   for (auto *Use : Top->DirectUses)
     if (Requested->isSubModuleOf(Use))
       return true;
 
   // Anyone is allowed to use our builtin stddef.h and its accompanying module.
   if (!Requested->Parent && Requested->Name == "_Builtin_stddef_max_align_t")
     return true;
 
+  if (NoUndeclaredIncludes)
+    UndeclaredUses.insert(Requested);
+
   return false;
 }
 
 void Module::addRequirement(StringRef Feature, bool RequiredState,
                             const LangOptions &LangOpts,
                             const TargetInfo &Target) {
   Requirements.push_back(Requirement(std::string(Feature), RequiredState));
 

clang/lib/Serialization/ASTWriter.cpp

#include "clang/Serialization/TypeBitCodes.def"
   llvm_unreachable("bad type kind");
 }
 
 namespace {
 
 std::set<const FileEntry *> GetAllModuleMaps(const HeaderSearch &HS,
                                              Module *RootModule) {
   std::set<const FileEntry *> ModuleMaps{};
-  std::set<Module *> ProcessedModules;
-  SmallVector<Module *> ModulesToProcess{RootModule};
+  std::set<const Module *> ProcessedModules;
+  SmallVector<const Module *> ModulesToProcess{RootModule};
 
   SmallVector<const FileEntry *, 16> 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) {
while (!ModulesToProcess.empty()) {
 
     for (auto *ImportedModule : (CurrentModule)->Imports) {
       if (!ImportedModule ||
           ProcessedModules.find(ImportedModule) != ProcessedModules.end()) {
         continue;
       }
       ModulesToProcess.push_back(ImportedModule);
     }
+
+    for (const Module *UndeclaredModule : CurrentModule->UndeclaredUses)
+      if (UndeclaredModule &&
+          ProcessedModules.find(UndeclaredModule) == ProcessedModules.end())
+        ModulesToProcess.push_back(UndeclaredModule);
   }
 
   return ModuleMaps;
 }
 
 class ASTTypeWriter {
   ASTWriter &Writer;
   ASTWriter::RecordData Record;
if (!Mod->Exports.empty()) {
       }
       Stream.EmitRecord(SUBMODULE_EXPORTS, Record);
     }
 
     //FIXME: How do we emit the 'use'd modules?  They may not be submodules.
     // Might be unnecessary as use declarations are only used to build the
     // module itself.
 
+    // TODO: Consider serializing undeclared uses of modules.
+
     // Emit the link libraries.
     for (const auto &LL : Mod->LinkLibraries) {
       RecordData::value_type Record[] = {SUBMODULE_LINK_LIBRARY,
                                          LL.IsFramework};
       Stream.EmitRecordWithBlob(LinkLibraryAbbrev, Record, LL.Library);
     }
 
     // Emit the conflicts.

clang/test/Modules/add-remove-irrelevant-module-map.m

 // Build with b.modulemap:
 //
 // RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/cache -fdisable-module-hash \
 // RUN:   -fmodule-map-file=%t/a.modulemap -fmodule-map-file=%t/b.modulemap %t/test-simple.m -verify
 // RUN: mv %t/cache %t/cache-with-b
 
 // Neither PCM file considers 'b.modulemap' an input:
 //
-// RUN: %clang_cc1 -module-file-info %t/cache-without-b/a.pcm | FileCheck %s
-// RUN: %clang_cc1 -module-file-info %t/cache-with-b/a.pcm | FileCheck %s
-// CHECK-NOT: Input file: {{.*}}/b.modulemap
+// RUN: %clang_cc1 -module-file-info %t/cache-without-b/a.pcm | FileCheck %s --check-prefix=CHECK-B
+// RUN: %clang_cc1 -module-file-info %t/cache-with-b/a.pcm | FileCheck %s --check-prefix=CHECK-B
+// CHECK-B-NOT: Input file: {{.*}}/b.modulemap
+
+//--- c.modulemap
+module c [no_undeclared_includes] { header "c.h" }
+
+//--- c.h
+#if __has_include("d.h") // This should use 'd.modulemap' in order to determine that 'd.h'
+                         // doesn't exist for 'c' because of its '[no_undeclared_includes]'.
+#endif
+
+//--- d.modulemap
+module d { header "d.h" }
+
+//--- d.h
+// empty
+
+//--- test-no-undeclared-includes.m
+// expected-no-diagnostics
+@import c;
+
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t/cache -fdisable-module-hash \
+// RUN:   -fmodule-map-file=%t/c.modulemap -fmodule-map-file=%t/d.modulemap \
+// RUN:   %t/test-no-undeclared-includes.m -verify
+
+// The PCM file considers 'd.modulemap' an input because it affects the compilation,
+// although it doesn't describe the built module or its imports.
+//
+// RUN: %clang_cc1 -module-file-info %t/cache/c.pcm | FileCheck %s --check-prefix=CHECK-D -DDIR=%t
+// CHECK-D: Input file: [[DIR]]/d.modulemap