[clang][modules] Build inferred modules

[jansvoboda11]

Summary

This patch enables explicitly building inferred modules.

Effectively a cherry-pick of https://github.com/apple/llvm-project/pull/699 authored by @Bigcheese with libclang changes omitted and adapted to use the new full canonical command-line clang-scan-deps produces since D100534.

Contains the following changes:

1. [Clang][ScanDeps] Ignore __inferred_module.map dependency.
   • This shows up with inferred modules, but it doesn't exist on disk, so don't report it as a dependency.

2. [Clang] Fix the header paths in clang::Module for inferred modules.
   • The UmbrellaAsWritten and NameAsWritten fields in clang::Module are a lie for framework modules. For those they actually are the path to the header or umbrella relative to the clang::Module::Directory.
   • The exception to this case is for inferred modules. Here it actually is the name as written, because we print out the module and read it back in when implicitly building modules. This causes a problem when explicitly building an inferred module, as we skip the printing out step.
   • In order to fix this issue this patch adds a new field for the path we want to use in getInputBufferForModule. It also makes NameAsWritten actually be the name written in the module map file (or that would be, in the case of an inferred module).

3. [Clang] Allow explicitly building an inferred module.
   • Building the actual module still fails, but make sure it fails for the right reason.

4. [Clang][ScanDeps] Use the module map a module was inferred from for inferred modules.

[jansvoboda11]

Apply clang-format suggestions

[jansvoboda11]

Rebase on top of D100942

[jansvoboda11]

Rebase on top of D101051

[jansvoboda11]

Fix clang-tidy warnings, add fixme

[jansvoboda11]

clang/lib/Serialization/ASTReader.cpp

5621

Not sure what's the best way to write/read the two strings (NameAsWritten, PathRelativeToRootModuleDirectory). Can we put them into a single blob and separate them with \0?

[jansvoboda11]

@dexonsmith ping

clang/lib/Serialization/ASTReader.cpp

5621

I think we can store them in whatever format we want and separating them by '\0' should be fine.

However, I'm not convinced we need to do so in this patch. Deserialized NameAsWritten isn't being used anyway, meaning we have no interesting test case for serialization that would ensure it works as expected.

[dexonsmith]

Given that there are four different things being done in this commit, it sounds naively like it should be four separate commits. If the logic is too intertwined to land each of the four pieces separately (certainly possible), can you quickly explain why, to motivate landing them together? (Or maybe there's an NFC refactoring that could be separated as a prep, to make the functional changes more isolated / easy to reason about?)

Either way, please split the testing between clang's ability to build inferred modules (clang/test/Modules) and clang-scan-deps ability to find them (clang/test/ClangScanDeps).

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

10–12

I took me a while to figure out that .rsp and -to-rsp means "response file". Can we just use response (or even response-file?) in both cases, or is .rsp a well-known extension I'm just not aware of?

I'm also not sure we want to use this in a test (see my next comment). If it's a useful utility regardless perhaps it could have a home in clang/utils.

13–17

I feel like the clang invocations that build/use modules should be in clang/test/Modules. Two independent things:

• Can clang build inferred modules explicitly? (tested in clang/test/Modules)
• Can clang-scan-deps generate deps for inferred modules? (tested in clang/test/ClangScanDeps)

[jansvoboda11]

In D100934#2733857, @dexonsmith wrote:

Given that there are four different things being done in this commit, it sounds naively like it should be four separate commits. If the logic is too intertwined to land each of the four pieces separately (certainly possible), can you quickly explain why, to motivate landing them together? (Or maybe there's an NFC refactoring that could be separated as a prep, to make the functional changes more isolated / easy to reason about?)

I think that landing everything together provides more context for people going through git log in the future.

If you really think splitting this up would be better, how about this?

1. Fix AsWritten for umbrella headers/directories.
2. Add tests to clang/test/Modules.
3. Add tests to clang/test/ClangScanDeps, make the necessary changes to Tooling/DependencyScanning, add response file Python script.

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

10–12

The Clang driver already has an option --rsp-quoting={posix,windows}, so I think the shortcut is somewhat known. I can rename the script to module-deps-to-response-file, if you think that's clearer. It could be useful in general, co moving this to clang/utils sounds fine.

13–17

I agree that we should test explicit build of inferred modules in clang/test/Modules (without clang-scan-deps). I'll look into it.

I'm not sure I'd be happy with only checking the dependencies produced by clang-scan-deps here. Testing that the generated command-line actually works is as important IMO, and it will be even more so when we start optimizing the command line (stripping out unused header search paths, definitions etc.).

[dexonsmith]

In D100934#2735955, @jansvoboda11 wrote:

In D100934#2733857, @dexonsmith wrote:

Given that there are four different things being done in this commit, it sounds naively like it should be four separate commits. If the logic is too intertwined to land each of the four pieces separately (certainly possible), can you quickly explain why, to motivate landing them together? (Or maybe there's an NFC refactoring that could be separated as a prep, to make the functional changes more isolated / easy to reason about?)

I think that landing everything together provides more context for people going through git log in the future.

If you really think splitting this up would be better, how about this?

1. Fix AsWritten for umbrella headers/directories.
2. Add tests to clang/test/Modules.

It sounds like these two pieces could be done together (since the refactor for AsWritten seems like it deserves a test), as a commit that adds support for building inferred modules explicitly.

3. Add tests to clang/test/ClangScanDeps, make the necessary changes to Tooling/DependencyScanning, add response file Python script.

Followed by this, for changing clang-scan-deps to support scanning for inferred modules.

WDYT?

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

10–12

It's probably fine as rsp then, just a name I personally didn't know.

13–17

I'm not sure precisely what you mean by "works". I'm not sure just checking if it still compiles tells us much. What's important is that the modified options have the same semantics, and I think a subtle change that still compiles is more likely than preventing compilation entirely.

I don't think it would scale to check for semantic problems here -- that needs a body of testcases that covers all the things modules support.

One option would be to use the testcases (or a selection of them) in clang/test/Modules, by adding an extra RUN line that builds clang-scan-deps-discovered modules both with and without command-line stripping. For most changes, we can arrange the AST block such that skipped options won't affect it, and we could compare the hash of just that block. If and when we start stripping ignored -D options we'll need something smarter, but we can solve that problem later. (Ideally, this would just be a mode in clang, clang -Xclang -fmodules-depscan, which does an initial depscan and builds the modules in order. This might actually be an improvement on the existing implicit modules.)

@Bigcheese, maybe you can weigh in? If you both think clang -cc1 should be tested here, I'm open to it. (In that case, though, this should not be invoking %clang, but %clang_cc1, I think... or does the response file create a driver command-line?)

[jansvoboda11]

In D100934#2737130, @dexonsmith wrote:

In D100934#2735955, @jansvoboda11 wrote:

1. Fix AsWritten for umbrella headers/directories.
2. Add tests to clang/test/Modules.

It sounds like these two pieces could be done together (since the refactor for AsWritten seems like it deserves a test), as a commit that adds support for building inferred modules explicitly.

3. Add tests to clang/test/ClangScanDeps, make the necessary changes to Tooling/DependencyScanning, add response file Python script.

Followed by this, for changing clang-scan-deps to support scanning for inferred modules.

WDYT?

That sounds good to me.

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

13–17

I'm not sure precisely what you mean by "works". I'm not sure just checking if it still compiles tells us much. What's important is that the modified options have the same semantics, and I think a subtle change that still compiles is more likely than preventing compilation entirely.

I see, that's a good point.

One option would be to use the testcases (or a selection of them) in clang/test/Modules, by adding an extra RUN line that builds clang-scan-deps-discovered modules both with and without command-line stripping. For most changes, we can arrange the AST block such that skipped options won't affect it, and we could compare the hash of just that block. If and when we start stripping ignored -D options we'll need something smarter, but we can solve that problem later.

Using tests from clang/test/Modules sounds nice. What are your concerns regarding stripping -D options?

In that case, though, this should not be invoking %clang, but %clang_cc1, I think... or does the response file create a driver command-line?

The Tooling/DependencyScanning library already prepends the "-cc1" argument so that build tools using the API don't have to do that on their own.

[dexonsmith]

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

13–17

Using tests from clang/test/Modules sounds nice. What are your concerns regarding stripping -D options?

Oh, it’s a bit mundane, but that’ll affect the identifier info, which will in turn change the AST block and its hash. I think?

For a truly explicit module, removing “unused” -D options from the identifier table might not be correct, since for semantics you might want importers to pick up those definitions (I think? Are they considered exported?). For implicitly-discovered modules, we know all transitive importers have a superset of -Ds on their command-lines so it’s safe.

Maybe what we can do at the time is turn on a flag to avoid writing unused -Ds to the serialized identifier table so the hashes will match? Anyway, I’m sure there is a solution.

[dexonsmith]

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

13–17

In that case, though, this should not be invoking %clang, but %clang_cc1, I think... or does the response file create a driver command-line?

The Tooling/DependencyScanning library already prepends the "-cc1" argument so that build tools using the API don't have to do that on their own.

I suggest adding a .cc1 extension to the response files to make that clear to anyone reading the test. Maybe t.inferred.cc1.rsp.

The third clang invocation doesn't have the response file as the first argument, though, so I imagine that one doesn't contain a -cc1? (Or if it did, I'm not sure how it would work... I'm guessing it only adds arguments that happen to match between -cc1 and driver?)

[jansvoboda11]

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

13–17

I suggest adding a .cc1 extension to the response files to make that clear to anyone reading the test. Maybe t.inferred.cc1.rsp.

Fair enough.

The third clang invocation doesn't have the response file as the first argument, though, so I imagine that one doesn't contain a -cc1? (Or if it did, I'm not sure how it would work... I'm guessing it only adds arguments that happen to match between -cc1 and driver?)

That's correct, it only contains additional arguments. The input to clang-scan-deps is a compilation database, so the assumption there is that it will contain driver command lines. Therefore, we're generating driver arguments.

@Bigcheese, tests in your original PR invoke -cc1, what was the reasoning behind that?

[jansvoboda11]

I split out the support for building inferred modules explicitly: D102491.

Patch with changes to the dependency scanner will follow.

clang/include/clang/Basic/Module.h

public:
   llvm::PointerUnion<const FileEntry *, const DirectoryEntry *> Umbrella;
 
   /// The module signature.
   ASTFileSignature Signature;
 
   /// The name of the umbrella entry, as written in the module map.
   std::string UmbrellaAsWritten;
 
+  // The path to the umbrella entry relative to the root module's \c Directory.
+  std::string UmbrellaRelativeToRootModuleDirectory;
+
   /// The module through which entities defined in this module will
   /// eventually be exposed, for use in "private" modules.
   std::string ExportAsModule;
 
   /// Does this Module scope describe part of the purview of a named C++ module?
   bool isModulePurview() const {
     return Kind == ModuleInterfaceUnit || Kind == PrivateModuleFragment;
   }
enum HeaderKind {
     HK_Excluded
   };
   static const int NumHeaderKinds = HK_Excluded + 1;
 
   /// Information about a header directive as found in the module map
   /// file.
   struct Header {
     std::string NameAsWritten;
+    std::string PathRelativeToRootModuleDirectory;
     const FileEntry *Entry;
 
     explicit operator bool() { return Entry; }
   };
 
   /// Information about a directory name as found in the module map
   /// file.
   struct DirectoryName {
     std::string NameAsWritten;
+    std::string PathRelativeToRootModuleDirectory;
     const DirectoryEntry *Entry;
 
     explicit operator bool() { return Entry; }
   };
 
   /// The headers that are part of this module.
   SmallVector<Header, 2> Headers[5];
 
public:
   /// Retrieve the directory for which this module serves as the
   /// umbrella.
   DirectoryName getUmbrellaDir() const;
 
   /// Retrieve the header that serves as the umbrella header for this
   /// module.
   Header getUmbrellaHeader() const {
     if (auto *FE = Umbrella.dyn_cast<const FileEntry *>())
-      return Header{UmbrellaAsWritten, FE};
+      return Header{UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
+                    FE};
     return Header{};
   }
 
   /// Determine whether this module has an umbrella directory that is
   /// not based on an umbrella header.
   bool hasUmbrellaDir() const {
     return Umbrella && Umbrella.is<const DirectoryEntry *>();
   }

clang/include/clang/Lex/ModuleMap.h

public:
   ///
   /// \returns true if any errors were encountered while resolving conflicts,
   /// false otherwise.
   bool resolveConflicts(Module *Mod, bool Complain);
 
   /// Sets the umbrella header of the given module to the given
   /// header.
   void setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
-                         Twine NameAsWritten);
+                         const Twine &NameAsWritten,
+                         const Twine &PathRelativeToRootModuleDirectory);
 
   /// Sets the umbrella directory of the given module to the given
   /// directory.
   void setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
-                      Twine NameAsWritten);
+                      const Twine &NameAsWritten,
+                      const Twine &PathRelativeToRootModuleDirectory);
 
   /// Adds this header to the given module.
   /// \param Role The role of the header wrt the module.
   void addHeader(Module *Mod, Module::Header Header,
                  ModuleHeaderRole Role, bool Imported = false);
 
   /// Marks this header as being excluded from the given module.
   void excludeHeader(Module *Mod, Module::Header Header);

clang/lib/Basic/Module.cpp

if (nameParts.empty() || M->Name != nameParts.back())
       return false;
     nameParts = nameParts.drop_back();
   }
   return nameParts.empty();
 }
 
 Module::DirectoryName Module::getUmbrellaDir() const {
   if (Header U = getUmbrellaHeader())
-    return {"", U.Entry->getDir()};
+    return {"", "", U.Entry->getDir()};
 
-  return {UmbrellaAsWritten, Umbrella.dyn_cast<const DirectoryEntry *>()};
+  return {UmbrellaAsWritten, UmbrellaRelativeToRootModuleDirectory,
+          Umbrella.dyn_cast<const DirectoryEntry *>()};
 }
 
 void Module::addTopHeader(const FileEntry *File) {
   assert(File);
   TopHeaders.insert(File);
 }
 
 ArrayRef<const FileEntry *> Module::getTopHeaders(FileManager &FileMgr) {

clang/lib/Frontend/FrontendAction.cpp

static std::error_code collectModuleHeaderIncludes(
   // Add includes for each of these headers.
   for (auto HK : {Module::HK_Normal, Module::HK_Private}) {
     for (Module::Header &H : Module->Headers[HK]) {
       Module->addTopHeader(H.Entry);
       // Use the path as specified in the module map file. We'll look for this
       // file relative to the module build directory (the directory containing
       // the module map file) so this will find the same file that we found
       // while parsing the module map.
-      addHeaderInclude(H.NameAsWritten, Includes, LangOpts, Module->IsExternC);
+      addHeaderInclude(H.PathRelativeToRootModuleDirectory, Includes, LangOpts,
+                       Module->IsExternC);
     }
   }
   // Note that Module->PrivateHeaders will not be a TopHeader.
 
   if (Module::Header UmbrellaHeader = Module->getUmbrellaHeader()) {
     Module->addTopHeader(UmbrellaHeader.Entry);
     if (Module->Parent)
       // Include the umbrella header for submodules.
-      addHeaderInclude(UmbrellaHeader.NameAsWritten, Includes, LangOpts,
-                       Module->IsExternC);
+      addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
+                       Includes, LangOpts, Module->IsExternC);
   } else if (Module::DirectoryName UmbrellaDir = Module->getUmbrellaDir()) {
     // Add all of the headers we find in this subdirectory.
     std::error_code EC;
     SmallString<128> DirNative;
     llvm::sys::path::native(UmbrellaDir.Entry->getName(), DirNative);
 
     llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
     SmallVector<std::pair<std::string, const FileEntry *>, 8> Headers;
for (llvm::vfs::recursive_directory_iterator Dir(FS, DirNative, EC), End;
       if (ModMap.isHeaderUnavailableInModule(*Header, Module))
         continue;
 
       // Compute the relative path from the directory to this file.
       SmallVector<StringRef, 16> Components;
       auto PathIt = llvm::sys::path::rbegin(Dir->path());
       for (int I = 0; I != Dir.level() + 1; ++I, ++PathIt)
         Components.push_back(*PathIt);
-      SmallString<128> RelativeHeader(UmbrellaDir.NameAsWritten);
+      SmallString<128> RelativeHeader(
+          UmbrellaDir.PathRelativeToRootModuleDirectory);
       for (auto It = Components.rbegin(), End = Components.rend(); It != End;
            ++It)
         llvm::sys::path::append(RelativeHeader, *It);
 
       std::string RelName = RelativeHeader.c_str();
       Headers.push_back(std::make_pair(RelName, *Header));
     }
 
if (CI.getLangOpts().CurrentModule.empty()) {
     // default. Then it would be fairly trivial to just "compile" a module
     // map with a single module (the common case).
     return nullptr;
   }
 
   // Dig out the module definition.
   HeaderSearch &HS = CI.getPreprocessor().getHeaderSearchInfo();
   Module *M = HS.lookupModule(CI.getLangOpts().CurrentModule,
-                              /*AllowSearch=*/false);
+                              /*AllowSearch=*/true);
   if (!M) {
     CI.getDiagnostics().Report(diag::err_missing_module)
       << CI.getLangOpts().CurrentModule << ModuleMapFilename;
 
     return nullptr;
   }
 
   // Check whether we can build this module at all.

 static std::unique_ptr<llvm::MemoryBuffer>
 getInputBufferForModule(CompilerInstance &CI, Module *M) {
   FileManager &FileMgr = CI.getFileManager();
 
   // Collect the set of #includes we need to build the module.
   SmallString<256> HeaderContents;
   std::error_code Err = std::error_code();
   if (Module::Header UmbrellaHeader = M->getUmbrellaHeader())
-    addHeaderInclude(UmbrellaHeader.NameAsWritten, HeaderContents,
-                     CI.getLangOpts(), M->IsExternC);
+    addHeaderInclude(UmbrellaHeader.PathRelativeToRootModuleDirectory,
+                     HeaderContents, CI.getLangOpts(), M->IsExternC);
   Err = collectModuleHeaderIncludes(
       CI.getLangOpts(), FileMgr, CI.getDiagnostics(),
       CI.getPreprocessor().getHeaderSearchInfo().getModuleMap(), M,
       HeaderContents);
 
   if (Err) {
     CI.getDiagnostics().Report(diag::err_module_cannot_create_includes)
       << M->getFullModuleName() << Err.message();

clang/lib/Frontend/FrontendActions.cpp

for (StringRef Name : ModuleHeaders) {
     Optional<FileEntryRef> FE = HS.LookupFile(
         Name, SourceLocation(), /*Angled*/ false, nullptr, CurDir, None,
         nullptr, nullptr, nullptr, nullptr, nullptr, nullptr);
     if (!FE) {
       CI.getDiagnostics().Report(diag::err_module_header_file_not_found)
         << Name;
       continue;
     }
-    Headers.push_back({std::string(Name), &FE->getFileEntry()});
+    Headers.push_back(
+        {std::string(Name), std::string(Name), &FE->getFileEntry()});
   }
   HS.getModuleMap().createHeaderModule(CI.getLangOpts().CurrentModule, Headers);
 
   return GenerateModuleAction::BeginSourceFileAction(CI);
 }
 
 std::unique_ptr<raw_pwrite_stream>
 GenerateHeaderModuleAction::CreateOutputFile(CompilerInstance &CI,

clang/lib/Lex/ModuleMap.cpp

if (Optional<FileEntryRef> File =
           findHeader(Mod, Header, RelativePathName, NeedsFramework)) {
     if (Header.IsUmbrella) {
       const DirectoryEntry *UmbrellaDir = &File->getDir().getDirEntry();
       if (Module *UmbrellaMod = UmbrellaDirs[UmbrellaDir])
         Diags.Report(Header.FileNameLoc, diag::err_mmap_umbrella_clash)
           << UmbrellaMod->getFullModuleName();
       else
         // Record this umbrella header.
-        setUmbrellaHeader(Mod, *File, RelativePathName.str());
+        setUmbrellaHeader(Mod, *File, Header.FileName, RelativePathName.str());
     } else {
-      Module::Header H = {std::string(RelativePathName.str()), *File};
+      Module::Header H = {Header.FileName, std::string(RelativePathName.str()),
+                          *File};
       if (Header.Kind == Module::HK_Excluded)
         excludeHeader(Mod, H);
       else
         addHeader(Mod, H, headerKindToRole(Header.Kind));
     }
   } else if (Header.HasBuiltinHeader && !Header.Size && !Header.ModTime) {
     // There's a builtin header but no corresponding on-disk header. Assume
     // this was supposed to modularize the builtin header alone.
bool ModuleMap::resolveAsBuiltinHeader(
   // supplied by Clang. Find that builtin header.
   SmallString<128> Path;
   llvm::sys::path::append(Path, BuiltinIncludeDir->getName(), Header.FileName);
   auto File = SourceMgr.getFileManager().getFile(Path);
   if (!File)
     return false;
 
   auto Role = headerKindToRole(Header.Kind);
-  Module::Header H = {std::string(Path.str()), *File};
+  Module::Header H = {Header.FileName, std::string(Path.str()), *File};
   addHeader(Mod, H, Role);
   return true;
 }
 
 ModuleMap::ModuleMap(SourceManager &SourceMgr, DiagnosticsEngine &Diags,
                      const LangOptions &LangOpts, const TargetInfo *Target,
                      HeaderSearch &HeaderInfo)
     : SourceMgr(SourceMgr), Diags(Diags), LangOpts(LangOpts), Target(Target),
Module *ModuleMap::inferFrameworkModule(const DirectoryEntry *FrameworkDir,
   }
 
   Result->IsSystem |= Attrs.IsSystem;
   Result->IsExternC |= Attrs.IsExternC;
   Result->ConfigMacrosExhaustive |= Attrs.IsExhaustive;
   Result->NoUndeclaredIncludes |= Attrs.NoUndeclaredIncludes;
   Result->Directory = FrameworkDir;
 
+  // Chop off the first framework bit, as that is implied.
+  StringRef RelativePath = UmbrellaName.str().substr(
+      Result->getTopLevelModule()->Directory->getName().size());
+  RelativePath = llvm::sys::path::relative_path(RelativePath);
+
   // umbrella header "umbrella-header-name"
-  //
-  // The "Headers/" component of the name is implied because this is
-  // a framework module.
-  setUmbrellaHeader(Result, *UmbrellaHeader, ModuleName + ".h");
+  setUmbrellaHeader(Result, *UmbrellaHeader, ModuleName + ".h", RelativePath);
 
   // export *
   Result->Exports.push_back(Module::ExportDecl(nullptr, true));
 
   // module * { export * }
   Result->InferSubmodules = true;
   Result->InferExportWildcard = true;
 
Module *ModuleMap::createShadowedModule(StringRef Name, bool IsFramework,
   Result->ShadowingModule = ShadowingModule;
   Result->markUnavailable(/*Unimportable*/true);
   ModuleScopeIDs[Result] = CurrentModuleScopeID;
   ShadowModules.push_back(Result);
 
   return Result;
 }
 
-void ModuleMap::setUmbrellaHeader(Module *Mod, const FileEntry *UmbrellaHeader,
-                                  Twine NameAsWritten) {
+void ModuleMap::setUmbrellaHeader(
+    Module *Mod, const FileEntry *UmbrellaHeader, const Twine &NameAsWritten,
+    const Twine &PathRelativeToRootModuleDirectory) {
   Headers[UmbrellaHeader].push_back(KnownHeader(Mod, NormalHeader));
   Mod->Umbrella = UmbrellaHeader;
   Mod->UmbrellaAsWritten = NameAsWritten.str();
+  Mod->UmbrellaRelativeToRootModuleDirectory =
+      PathRelativeToRootModuleDirectory.str();
   UmbrellaDirs[UmbrellaHeader->getDir()] = Mod;
 
   // Notify callbacks that we just added a new header.
   for (const auto &Cb : Callbacks)
     Cb->moduleMapAddUmbrellaHeader(&SourceMgr.getFileManager(), UmbrellaHeader);
 }
 
 void ModuleMap::setUmbrellaDir(Module *Mod, const DirectoryEntry *UmbrellaDir,
-                               Twine NameAsWritten) {
+                               const Twine &NameAsWritten,
+                               const Twine &PathRelativeToRootModuleDirectory) {
   Mod->Umbrella = UmbrellaDir;
   Mod->UmbrellaAsWritten = NameAsWritten.str();
+  Mod->UmbrellaRelativeToRootModuleDirectory =
+      PathRelativeToRootModuleDirectory.str();
   UmbrellaDirs[UmbrellaDir] = Mod;
 }
 
 void ModuleMap::addUnresolvedHeader(Module *Mod,
                                     Module::UnresolvedHeaderDirective Header,
                                     bool &NeedsFramework) {
   // If there is a builtin counterpart to this file, add it now so it can
   // wrap the system header.
void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
   if (!Tok.is(MMToken::StringLiteral)) {
     Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header)
       << "umbrella";
     HadError = true;
     return;
   }
 
   std::string DirName = std::string(Tok.getString());
+  std::string DirNameAsWritten = DirName;
   SourceLocation DirNameLoc = consumeToken();
 
   // Check whether we already have an umbrella.
   if (ActiveModule->Umbrella) {
     Diags.Report(DirNameLoc, diag::err_mmap_umbrella_clash)
       << ActiveModule->getFullModuleName();
     HadError = true;
     return;
if (UsesRequiresExcludedHack.count(ActiveModule)) {
     // uncommonly used Tcl module on Darwin platforms.
     std::error_code EC;
     SmallVector<Module::Header, 6> Headers;
     llvm::vfs::FileSystem &FS =
         SourceMgr.getFileManager().getVirtualFileSystem();
     for (llvm::vfs::recursive_directory_iterator I(FS, Dir->getName(), EC), E;
          I != E && !EC; I.increment(EC)) {
       if (auto FE = SourceMgr.getFileManager().getFile(I->path())) {
-        Module::Header Header = {std::string(I->path()), *FE};
+        Module::Header Header = {"", std::string(I->path()), *FE};
         Headers.push_back(std::move(Header));
       }
     }
 
     // Sort header paths so that the pcm doesn't depend on iteration order.
     llvm::array_pod_sort(Headers.begin(), Headers.end(), compareModuleHeaders);
 
     for (auto &Header : Headers)
       Map.addHeader(ActiveModule, std::move(Header), ModuleMap::TextualHeader);
     return;
   }
 
   if (Module *OwningModule = Map.UmbrellaDirs[Dir]) {
     Diags.Report(UmbrellaLoc, diag::err_mmap_umbrella_clash)
       << OwningModule->getFullModuleName();
     HadError = true;
     return;
   }
 
   // Record this umbrella directory.
-  Map.setUmbrellaDir(ActiveModule, Dir, DirName);
+  Map.setUmbrellaDir(ActiveModule, Dir, DirNameAsWritten, DirName);
 }
 
 /// Parse a module export declaration.
 ///
 ///   export-declaration:
 ///     'export' wildcard-module-id
 ///
 ///   wildcard-module-id:

clang/lib/Serialization/ASTReader.cpp

while (d != End) {
     Module *Mod = Reader.getSubmodule(GlobalSMID);
     FileManager &FileMgr = Reader.getFileManager();
     ModuleMap &ModMap =
         Reader.getPreprocessor().getHeaderSearchInfo().getModuleMap();
 
     std::string Filename = std::string(key.Filename);
     if (key.Imported)
       Reader.ResolveImportedPath(M, Filename);
-    // FIXME: This is not always the right filename-as-written, but we're not
-    // going to use this information to rebuild the module, so it doesn't make
-    // a lot of difference.
-    Module::Header H = {std::string(key.Filename), *FileMgr.getFile(Filename)};
+    // FIXME: NameAsWritten
+    Module::Header H = {std::string(key.Filename), "",
+                        *FileMgr.getFile(Filename)};
     ModMap.addHeader(Mod, H, HeaderRole, /*Imported*/true);
     HFI.isModuleHeader |= !(HeaderRole & ModuleMap::TextualHeader);
   }
 
   // This HeaderFileInfo was externally loaded.
   HFI.External = true;
   HFI.IsValid = true;
   return HFI;
case SUBMODULE_DEFINITION: {
       break;
     }
 
     case SUBMODULE_UMBRELLA_HEADER: {
       std::string Filename = std::string(Blob);
       ResolveImportedPath(F, Filename);
       if (auto Umbrella = PP.getFileManager().getFile(Filename)) {
         if (!CurrentModule->getUmbrellaHeader())
-          ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob);
+          // FIXME: NameAsWritten
+          ModMap.setUmbrellaHeader(CurrentModule, *Umbrella, Blob, "");
         else if (CurrentModule->getUmbrellaHeader().Entry != *Umbrella) {
           if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
             Error("mismatched umbrella headers in submodule");
           return OutOfDate;
         }
       }
       break;
     }
case SUBMODULE_TOPHEADER:
       CurrentModule->addTopHeaderFilename(Blob);
       break;
 
     case SUBMODULE_UMBRELLA_DIR: {
       std::string Dirname = std::string(Blob);
       ResolveImportedPath(F, Dirname);
       if (auto Umbrella = PP.getFileManager().getDirectory(Dirname)) {
         if (!CurrentModule->getUmbrellaDir())
-          ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob);
+          // FIXME: NameAsWritten
+          ModMap.setUmbrellaDir(CurrentModule, *Umbrella, Blob, "");
         else if (CurrentModule->getUmbrellaDir().Entry != *Umbrella) {
           if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
             Error("mismatched umbrella directories in submodule");
           return OutOfDate;
         }
       }
       break;
     }

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

makeInvocationForModuleBuildWithoutPaths(const ModuleDeps &Deps) {
   CI.getFrontendOpts().Inputs.clear();
   CI.getFrontendOpts().OutputFile.clear();
 
   CI.getFrontendOpts().ProgramAction = frontend::GenerateModule;
   CI.getLangOpts()->ModuleName = Deps.ID.ModuleName;
   CI.getFrontendOpts().IsSystemModule = Deps.IsSystem;
 
   CI.getLangOpts()->ImplicitModules = false;
-  CI.getHeaderSearchOpts().ImplicitModuleMaps = false;
 
   return CI;
 }
 
 static std::vector<std::string>
 serializeCompilerInvocation(CompilerInvocation &CI) {
   // Set up string allocator.
   llvm::BumpPtrAllocator Alloc;
void ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
   if (!ModI.first->second.ID.ModuleName.empty())
     return;
 
   ModuleDeps &MD = ModI.first->second;
 
   const FileEntry *ModuleMap = Instance.getPreprocessor()
                                    .getHeaderSearchInfo()
                                    .getModuleMap()
-                                   .getContainingModuleMapFile(M);
+                                   .getModuleMapFileForUniquing(M);
 
   MD.Invocation = Instance.getInvocationPtr();
   MD.ClangModuleMapFile = std::string(ModuleMap ? ModuleMap->getName() : "");
   MD.ID.ModuleName = M->getFullModuleName();
   MD.ImplicitModulePCMPath = std::string(M->getASTFile()->getName());
   MD.ID.ContextHash = MDC.ContextHash;
   MD.IsSystem = M->IsSystem;
   serialization::ModuleFile *MF =
       MDC.Instance.getASTReader()->getModuleManager().lookup(M->getASTFile());
   MDC.Instance.getASTReader()->visitInputFiles(
       *MF, true, true, [&](const serialization::InputFile &IF, bool isSystem) {
+        // __inferred_module.map is the result of the way in which an implicit
+        // module build handles inferred modules. It adds an overlay VFS with
+        // this file in the proper directory and relies on the rest of Clang to
+        // handle it like normal. With explicitly built modules we don't need
+        // to play VFS tricks, so replace it with the correct module map.
+        if (IF.getFile()->getName().endswith("__inferred_module.map")) {
+          MD.FileDeps.insert(ModuleMap->getName());
+          return;
+        }
         MD.FileDeps.insert(IF.getFile()->getName());
       });
 
   llvm::DenseSet<const Module *> AddedModules;
   addAllSubmoduleDeps(M, MD, AddedModules);
 }
 
 void ModuleDepCollectorPP::addAllSubmoduleDeps(

clang/test/ClangScanDeps/Inputs/frameworks/Inferred.framework/Headers/Inferred.h

+typedef int inferred;

clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Headers/System.h

+enum { bigger_than_int = 0x80000000 };

clang/test/ClangScanDeps/Inputs/frameworks/System.framework/Modules/module.modulemap

+framework module System [system] {
+  umbrella header "System.h"
+}

clang/test/ClangScanDeps/Inputs/frameworks/module.modulemap

+framework module * {}

clang/test/ClangScanDeps/Inputs/modules_inferred_cdb.json

+[
+{
+  "directory": "DIR",
+  "command": "clang -E DIR/modules_cdb_input.cpp -FFRAMEWORKS -fmodules -fmodules-cache-path=DIR/module-cache -fimplicit-modules -fimplicit-module-maps -pedantic -Werror",
+  "file": "DIR/modules_cdb_input.cpp"
+}
+]

clang/test/ClangScanDeps/module-deps-to-rsp.py

+#!/usr/bin/env python
+
+import argparse
+import json
+import sys
+
+class ModuleNotFoundError(Exception):
+  def __init__(self, module_name):
+    self.module_name = module_name
+
+class FullDeps:
+  def __init__(self):
+    self.modules = dict()
+    self.translation_units = str()
+    
+def getModulePathArgs(modules, full_deps):
+  cmd = []
+  for md in modules:
+    m = full_deps.modules[md['module-name'] + '-' + md['context-hash']]
+    cmd += [u'-fmodule-map-file=' + m['clang-modulemap-file']]
+    cmd += [u'-fmodule-file=' + md['module-name'] + '-' + md['context-hash'] + '.pcm']
+  return cmd
+
+def getCommandLineForModule(module_name, full_deps):
+  for m in full_deps.modules.values():
+    if m['name'] == module_name:
+      module = m
+      break
+  else:
+    raise ModuleNotFoundError(module_name)
+
+  cmd = m['command-line']
+  cmd += getModulePathArgs(m['clang-module-deps'], full_deps)
+  cmd += [u'-o', m['name'] + '-' + m['context-hash'] + '.pcm']
+  cmd += [m['clang-modulemap-file']]
+  
+  return cmd
+  
+def getCommandLineForTU(tu, full_deps):
+  cmd = tu['command-line']
+  cmd += getModulePathArgs(tu['clang-module-deps'], full_deps)
+  return cmd
+
+def parseFullDeps(json):
+  ret = FullDeps()
+  for m in json['modules']:
+    ret.modules[m['name'] + '-' + m['context-hash']] = m
+  ret.translation_units = json['translation-units']
+  return ret
+
+def main():
+  parser = argparse.ArgumentParser()
+  parser.add_argument("full_deps_file", help="Path to the full dependencies json file",
+                      type=str)
+  action = parser.add_mutually_exclusive_group(required=True)
+  action.add_argument("--module-name", help="The name of the module to get arguments for",
+                      type=str)
+  action.add_argument("--tu-index", help="The index of the translation unit to get arguments for",
+                      type=int)
+  args = parser.parse_args()
+  
+  full_deps = parseFullDeps(json.load(open(args.full_deps_file, 'r')))
+  
+  try:
+    if args.module_name:
+      print(" ".join(getCommandLineForModule(args.module_name, full_deps)))
+    
+    elif args.tu_index != None:
+      print(" ".join(getCommandLineForTU(full_deps.translation_units[args.tu_index], full_deps)))
+  except:
+    print("Unexpected error:", sys.exc_info()[0])
+    raise
+
+if __name__ == '__main__':
+  main()

clang/test/ClangScanDeps/modules-full.cpp

 // CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/Inputs/module.modulemap",
 // CHECK-NEXT:       "command-line": [
 // CHECK-NEXT:         "-cc1"
 // CHECK-NO-ABS-NOT:   "-fmodule-map-file={{.*}}"
 // CHECK-ABS:          "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
 // CHECK:              "-emit-module"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
 // CHECK-ABS:          "-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm"
-// CHECK-NOT:          "-fimplicit-module-maps"
 // CHECK:              "-fmodule-name=header1"
 // CHECK:              "-fno-implicit-modules"
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[CONTEXT_HASH_H1]]",
 // CHECK-NEXT:       "file-deps": [
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/header.h",
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "header1"
 // CHECK-NEXT:     },
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-module-deps": [],
 // CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/Inputs/module.modulemap",
 // CHECK-NEXT:       "command-line": [
 // CHECK-NEXT:         "-cc1",
 // CHECK:              "-emit-module",
-// CHECK-NOT:          "-fimplicit-module-maps",
 // CHECK:              "-fmodule-name=header1",
 // CHECK:              "-fno-implicit-modules",
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[CONTEXT_HASH_H2:[A-Z0-9]+]]",
 // CHECK-NEXT:       "file-deps": [
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/header.h",
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-NEXT:       ],
 // CHECK-NEXT:       "name": "header1"
 // CHECK-NEXT:     },
 // CHECK-NEXT:     {
 // CHECK-NEXT:       "clang-module-deps": [],
 // CHECK-NEXT:       "clang-modulemap-file": "[[PREFIX]]/Inputs/module.modulemap",
 // CHECK-NEXT:       "command-line": [
 // CHECK-NEXT:         "-cc1",
 // CHECK:              "-emit-module",
-// CHECK-NOT:          "-fimplicit-module-maps",
 // CHECK:              "-fmodule-name=header2",
 // CHECK:              "-fno-implicit-modules",
 // CHECK:            ],
 // CHECK-NEXT:       "context-hash": "[[CONTEXT_HASH_H1]]",
 // CHECK-NEXT:       "file-deps": [
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/header2.h",
 // CHECK-NEXT:         "[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-NEXT:       ],

clang/test/ClangScanDeps/modules-inferred-explicit-build.m

+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/modules_cdb_input.cpp
+// RUN: sed -e "s|DIR|%/t.dir|g" -e "s|FRAMEWORKS|%/S/Inputs/frameworks|g" -e "s|-E|-x objective-c -E|g" \
+// RUN:   %S/Inputs/modules_inferred_cdb.json > %t.cdb
+//
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -format experimental-full \
+// RUN:   -mode preprocess-minimized-sources > %t.db
+// RUN: %S/module-deps-to-rsp.py %t.db --module-name=Inferred > %t.inferred.rsp
+// RUN: %S/module-deps-to-rsp.py %t.db --module-name=System > %t.system.rsp
+// RUN: %S/module-deps-to-rsp.py %t.db --tu-index=0 > %t.tu.rsp
+// RUN: %clang @%t.inferred.rsp -pedantic -Werror
+// RUN: %clang @%t.system.rsp -pedantic -Werror
+// RUN: %clang -x objective-c -fsyntax-only %t.dir/modules_cdb_input.cpp \
+// RUN:   -F%S/Inputs/frameworks -fmodules -fimplicit-module-maps \
+// RUN:   -pedantic -Werror @%t.tu.rsp
+
+#include <Inferred/Inferred.h>
+#include <System/System.h>
+
+inferred a = bigger_than_int;

clang/test/ClangScanDeps/modules-inferred.m

+// RUN: rm -rf %t.dir
+// RUN: rm -rf %t.cdb
+// RUN: mkdir -p %t.dir
+// RUN: cp %s %t.dir/modules_cdb_input.cpp
+// RUN: sed -e "s|DIR|%/t.dir|g" -e "s|FRAMEWORKS|%S/Inputs/frameworks|g" \
+// RUN:   %S/Inputs/modules_inferred_cdb.json > %t.cdb
+//
+// RUN: echo -%t.dir > %t.result
+// RUN: echo -%S >> %t.result
+// RUN: clang-scan-deps -compilation-database %t.cdb -j 1 -format experimental-full \
+// RUN:   -generate-modules-path-args -mode preprocess-minimized-sources >> %t.result
+// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck --check-prefixes=CHECK %s
+
+#include <Inferred/Inferred.h>
+
+inferred a = 0;
+
+// CHECK: -[[PREFIX:.*]]
+// CHECK-NEXT: -[[SOURCEDIR:.*]]
+// CHECK-NEXT: {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-module-deps": [],
+// CHECK-NEXT:       "clang-modulemap-file": "[[SOURCEDIR]]/Inputs/frameworks/module.modulemap",
+// CHECK-NEXT:       "command-line": [
+// CHECK-NEXT:         "-cc1",
+// CHECK:              "-emit-module",
+// CHECK:              "-fmodule-name=Inferred",
+// CHECK:              "-fno-implicit-modules",
+// CHECK:            ],
+// CHECK-NEXT:       "context-hash": "[[CONTEXT_HASH_H1:[A-Z0-9]+]]",
+// CHECK-NEXT:       "file-deps": [
+// CHECK-NEXT:         "[[SOURCEDIR]]/Inputs/frameworks/Inferred.framework/Frameworks/Sub.framework/Headers/Sub.h",
+// CHECK-NEXT:         "[[SOURCEDIR]]/Inputs/frameworks/Inferred.framework/Headers/Inferred.h",
+// CHECK-NEXT:         "[[SOURCEDIR]]/Inputs/frameworks/module.modulemap"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "name": "Inferred"
+// CHECK-NEXT:     }
+// CHECK-NEXT:   ],
+// CHECK-NEXT:   "translation-units": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "clang-context-hash": "[[CONTEXT_HASH_H1]]",
+// CHECK-NEXT:       "clang-module-deps": [
+// CHECK-NEXT:         {
+// CHECK-NEXT:           "context-hash": "[[CONTEXT_HASH_H1]]",
+// CHECK-NEXT:           "module-name": "Inferred"
+// CHECK-NEXT:         }
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "command-line": [
+// CHECK-NEXT:         "-fno-implicit-modules",
+// CHECK-NEXT:         "-fno-implicit-module-maps",
+// CHECK-NEXT:         "-fmodule-file=[[PREFIX]]/module-cache/[[CONTEXT_HASH_H1]]/Inferred-{{[A-Z0-9]+}}.pcm",
+// CHECK-NEXT:         "-fmodule-map-file=[[SOURCEDIR]]/Inputs/frameworks/module.modulemap"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "file-deps": [
+// CHECK-NEXT:         "[[PREFIX]]/modules_cdb_input.cpp"
+// CHECK-NEXT:       ],
+// CHECK-NEXT:       "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
+// CHECK-NEXT:     }
+// CHECK-NEXT:   ]
+// CHECK-NEXT: }