[clang][modules][deps] Serialize inputs into PCMs using the "as requested" name

[jansvoboda11]

Summary

This patch changes the PCM serialization logic to refer to input files by their "requested" name. This fixes a bug where the dependency scanner reports the "final" file paths, which can result in failed explicit compiles due to the module.modulemap file not being surrounded by the expected framework directory structure.

Depends on D135634.

[Bigcheese]

lgtm

[jansvoboda11]

Rebase, update test description

[benlangmuir]

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

457

Was this canonicalization necessary? If this is needed can we do it in the implicit module build so that it is serialized with the correct path? That seems better for both stability of the implicit module build and avoids re-canonicalizing the path every time it is scanned.

[jansvoboda11]

Rebase, remove unnecessary canonicalization

[jansvoboda11]

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

457

Good question. Canonicalization is not necessary for the test to pass, so I removed it. We can do it during serialization if we discover it's necessary.

clang/lib/Serialization/ASTWriter.cpp

// And whether this file was overridden.
           INPUT_FILE,
           InputFileOffsets.size(),
           (uint64_t)Entry.File.getSize(),
           (uint64_t)getTimestampForOutput(Entry.File),
           Entry.BufferOverridden,
           Entry.IsTransient,
           Entry.IsTopLevelModuleMap};
 
-      EmitRecordWithPath(IFAbbrevCode, Record, Entry.File.getName());
+      EmitRecordWithPath(IFAbbrevCode, Record, Entry.File.getNameAsRequested());
     }
 
     // Emit content hash for this file.
     {
       RecordData::value_type Record[] = {INPUT_FILE_HASH, Entry.ContentHash[0],
                                          Entry.ContentHash[1]};
       Stream.EmitRecordWithAbbrev(IFHAbbrevCode, Record);
     }

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

ModuleID ModuleDepCollectorPP::handleTopLevelModule(const Module *M) {
 
   llvm::DenseSet<const Module *> SeenDeps;
   addAllSubmodulePrebuiltDeps(M, MD, SeenDeps);
   addAllSubmoduleDeps(M, MD, SeenDeps);
   addAllAffectingClangModules(M, MD, SeenDeps);
 
   MDC.ScanInstance.getASTReader()->visitTopLevelModuleMaps(
       *MF, [&](FileEntryRef FE) {
-        if (FE.getName().endswith("__inferred_module.map"))
+        if (FE.getNameAsRequested().endswith("__inferred_module.map"))
           return;
-        MD.ModuleMapFileDeps.emplace_back(FE.getName());
+        MD.ModuleMapFileDeps.emplace_back(FE.getNameAsRequested());
       });
 
   CompilerInvocation CI = MDC.makeInvocationForModuleBuildWithoutOutputs(
       MD, [&](CompilerInvocation &BuildInvocation) {
         if (MDC.OptimizeArgs)
           optimizeHeaderSearchOpts(BuildInvocation.getHeaderSearchOpts(),
                                    *MDC.ScanInstance.getASTReader(), *MF);
       });
 

clang/test/ClangScanDeps/modules-file-name-as-requested.m

+// This test checks that the module map paths we're reporting are the as-requested
+// paths (as opposed to the paths files resolve to after going through VFS overlays).
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- real/module.modulemap
+framework module FW { header "Header.h" }
+//--- real/Header.h
+//--- overlay.json.template
+{
+  "case-sensitive": "false",
+  "version": "0",
+  "roots": [
+    {
+      "contents": [
+        {
+          "external-contents" : "DIR/real/Header.h",
+          "name" : "Header.h",
+          "type" : "file"
+        }
+      ],
+      "name": "DIR/frameworks/FW.framework/Headers",
+      "type": "directory"
+    },
+    {
+      "contents": [
+        {
+          "external-contents": "DIR/real/module.modulemap",
+          "name": "module.modulemap",
+          "type": "file"
+        }
+      ],
+      "name": "DIR/frameworks/FW.framework/Modules",
+      "type": "directory"
+    }
+  ]
+}
+
+//--- modules/module.modulemap
+module Importer { header "header.h" }
+//--- modules/header.h
+#include <FW/Header.h>
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.m",
+  "directory": "DIR",
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -Werror=non-modular-include-in-module -ivfsoverlay DIR/overlay.json -F DIR/frameworks -I DIR/modules -c DIR/tu.m -o DIR/tu.o"
+}]
+
+//--- tu.m
+@import Importer;
+
+// RUN: sed -e "s|DIR|%/t|g" %t/overlay.json.template > %t/overlay.json
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+
+// RUN: %deps-to-rsp %t/result.json --module-name=FW > %t/FW.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --module-name=Importer > %t/Importer.cc1.rsp
+// RUN: %deps-to-rsp %t/result.json --tu-index=0 > %t/tu.rsp
+// RUN: %clang @%t/FW.cc1.rsp
+// RUN: %clang @%t/Importer.cc1.rsp
+// RUN: %clang @%t/tu.rsp