[clang][deps] Report module map describing compiled module

[jansvoboda11]

Summary

This patch fixes compilation failure with explicit modules caused by scanner not reporting the module map describing the module whose implementation is being compiled.

Below is a breakdown of the attached test case. Note the VFS that makes frameworks "A" and "B" share the same header "shared/H.h".

In non-modular build, Clang skips the last import, since the "shared/H.h" header has already been included.

During scan (or implicit build), the compiler handles "tu.m" as follows:

• @import B imports module "B", as expected,
• #import <A/H.h> is resolved textually (due to -fmodule-name=A) to "shared/H.h" (due to the VFS remapping),
• #import <B/H.h> is resolved to import module "A_Private", since the header "shared/H.h" is already known to be part of that module, and the import is skipped.

In the end, the only modular dependency of the TU is "B".

In explicit modular build without -fmodule-name=A, TU does depend on module "A_Private" properly, not just textually. Clang therefore builds & loads its PCM, and knows to ignore the last import, since "shared/H.h" is known to be part of "A_Private".

But with current scanner behavior and -fmodule-name=A present, the last import fails during explicit build. Clang doesn't know about "A_Private" (it's included textually) and tries to import "B_Private" instead, which it doesn't know about either (the scanner correctly didn't report it as dependency). This is fixed by reporting the module map describing "A" and matching the semantics of implicit build.

[benlangmuir]

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

194

I think this should move inside the if below for needsModules.

We are checking the result of getModuleMapFileForUniquing for nullptr in ModuleDepCollector.cpp -- should we be doing the same here? I find it hard to guess when it might be null, but there's no asserts that it won't be anywhere that I can find.

[jansvoboda11]

Implement suggestions

[Bigcheese]

lgtm.

[w2yehia]

@jansvoboda11
FYI, this is breaking AIX buildbot:
https://lab.llvm.org/buildbot/#/builders/214/builds/3436/steps/6/logs/FAIL__Clang__modules-header-sharing_m

[jansvoboda11]

In D134222#3815793, @w2yehia wrote:

@jansvoboda11
FYI, this is breaking AIX buildbot:
https://lab.llvm.org/buildbot/#/builders/214/builds/3436/steps/6/logs/FAIL__Clang__modules-header-sharing_m

Thanks for reporting this and sorry for the breakage. Should be fixed in 8df1f3bc19d7.

[w2yehia]

In D134222#3815956, @jansvoboda11 wrote:

In D134222#3815793, @w2yehia wrote:

@jansvoboda11
FYI, this is breaking AIX buildbot:
https://lab.llvm.org/buildbot/#/builders/214/builds/3436/steps/6/logs/FAIL__Clang__modules-header-sharing_m

Thanks for reporting this and sorry for the breakage. Should be fixed in 8df1f3bc19d7.

thanks for the quick turnaround

clang/include/clang/Basic/LangOptions.h

bool isCompilingModule() const {
     return getCompilingModule() != CMK_None;
   }
 
   /// Are we compiling a standard c++ module interface?
   bool isCompilingModuleInterface() const {
     return getCompilingModule() == CMK_ModuleInterface;
   }
 
+  /// Are we compiling a module implementation?
+  bool isCompilingModuleImplementation() const {
+    return !isCompilingModule() && !ModuleName.empty();
+  }
+
   /// Do we need to track the owning module for a local declaration?
   bool trackLocalOwningModule() const {
     return isCompilingModule() || ModulesLocalVisibility;
   }
 
   bool isSignedOverflowDefined() const {
     return getSignedOverflowBehavior() == SOB_Defined;
   }

clang/include/clang/Lex/Preprocessor.h

void setTUFPEvalMethod(LangOptions::FPEvalMethodKind Val) {
     assert(Val != LangOptions::FEM_UnsetOnCommandLine &&
            "TUPEvalMethod should never be set to FEM_UnsetOnCommandLine");
     TUFPEvalMethod = Val;
   }
 
   /// Retrieves the module that we're currently building, if any.
   Module *getCurrentModule();
 
+  /// Retrieves the module whose implementation we're current compiling, if any.
+  Module *getCurrentModuleImplementation();
+
   /// Allocate a new MacroInfo object with the provided SourceLocation.
   MacroInfo *AllocateMacroInfo(SourceLocation L);
 
   /// Turn the specified lexer token into a fully checked and spelled
   /// filename, e.g. as an operand of \#include.
   ///
   /// The caller is expected to provide a buffer that is large enough to hold
   /// the spelling of the filename, but is also expected to handle the case

clang/lib/Lex/Preprocessor.cpp

 
 Module *Preprocessor::getCurrentModule() {
   if (!getLangOpts().isCompilingModule())
     return nullptr;
 
   return getHeaderSearchInfo().lookupModule(getLangOpts().CurrentModule);
 }
 
+Module *Preprocessor::getCurrentModuleImplementation() {
+  if (!getLangOpts().isCompilingModuleImplementation())
+    return nullptr;
+
+  return getHeaderSearchInfo().lookupModule(getLangOpts().ModuleName);
+}
+
 //===----------------------------------------------------------------------===//
 // Preprocessor Initialization Methods
 //===----------------------------------------------------------------------===//
 
 /// EnterMainSourceFile - Enter the specified FileID as the main source file,
 /// which implicitly adds the builtin defines etc.
 void Preprocessor::EnterMainSourceFile() {
   // We do not allow the preprocessor to reenter the main file.  Doing so will

clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

default:
     return true;
   }
 }
 
 void ModuleDepCollector::applyDiscoveredDependencies(CompilerInvocation &CI) {
   CI.clearImplicitModuleBuildOptions();
 
   if (llvm::any_of(CI.getFrontendOpts().Inputs, needsModules)) {
+    Preprocessor &PP = ScanInstance.getPreprocessor();
+    if (Module *CurrentModule = PP.getCurrentModuleImplementation())
+      if (const FileEntry *CurrentModuleMap =
+              PP.getHeaderSearchInfo()
+                  .getModuleMap()
+                  .getModuleMapFileForUniquing(CurrentModule))
+        CI.getFrontendOpts().ModuleMapFiles.emplace_back(
+            CurrentModuleMap->getName());
+
     SmallVector<ModuleID> DirectDeps;
     for (const auto &KV : ModularDeps)
       if (KV.second->ImportedByMainFile)
         DirectDeps.push_back(KV.second->ID);
 
     addModuleMapFiles(CI, DirectDeps);
     addModuleFiles(CI, DirectDeps);
 

clang/test/ClangScanDeps/modules-header-sharing.m

+// There are some edge-cases where Clang depends on knowing the module whose implementation it's currently building.
+// This test makes sure scanner always reports the corresponding module map.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- frameworks/A.framework/Modules/module.modulemap
+framework module A { umbrella header "A.h" }
+//--- frameworks/B.framework/Modules/module.modulemap
+framework module B { umbrella header "B.h" }
+//--- frameworks/A.framework/Headers/A.h
+//--- frameworks/B.framework/Headers/B.h
+//--- frameworks/A.framework/Modules/module.private.modulemap
+framework module A_Private { umbrella header "A_Private.h" }
+//--- frameworks/B.framework/Modules/module.private.modulemap
+framework module B_Private { umbrella header "B_Private.h" }
+//--- frameworks/A.framework/PrivateHeaders/A_Private.h
+#import <A/H.h>
+//--- frameworks/B.framework/PrivateHeaders/B_Private.h
+#import <B/H.h>
+
+//--- shared/H.h
+
+//--- overlay.json.template
+{
+  "case-sensitive": "false",
+  "version": 0,
+  "roots": [
+    {
+      "contents": [
+        {
+          "external-contents": "DIR/shared/H.h",
+          "name": "H.h",
+          "type": "file"
+        }
+      ],
+      "name": "DIR/frameworks/A.framework/PrivateHeaders",
+      "type": "directory"
+    },
+    {
+      "contents": [
+        {
+          "external-contents": "DIR/shared/H.h",
+          "name": "H.h",
+          "type": "file"
+        }
+      ],
+      "name": "DIR/frameworks/B.framework/PrivateHeaders",
+      "type": "directory"
+    }
+  ]
+}
+
+//--- cdb.json.template
+[{
+  "file": "DIR/tu.m",
+  "directory": "DIR",
+  "command": "clang -fmodules -fmodules-cache-path=DIR/cache -fmodule-name=A -ivfsoverlay DIR/overlay.json -F DIR/frameworks -c DIR/tu.m -o DIR/tu.o"
+}]
+
+//--- tu.m
+@import B;
+#import <A/H.h>
+#import <B/H.h>
+
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+// RUN: sed -e "s|DIR|%/t|g" %t/overlay.json.template > %t/overlay.json
+
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full > %t/result.json
+// RUN: cat %t/result.json | sed 's:\\\\\?:/:g' | FileCheck %s -DPREFIX=%/t
+// CHECK:      {
+// CHECK:        "translation-units": [
+// CHECK-NEXT:     {
+// CHECK-NEXT:       "commands": [
+// CHECK-NEXT:         {
+// CHECK:                "command-line": [
+// CHECK:                  "-fmodule-map-file=[[PREFIX]]/frameworks/A.framework/Modules/module.modulemap",
+// CHECK:                  "-fmodule-name=A",
+// CHECK:                ],
+// CHECK-NEXT:           "executable": "clang",
+// CHECK-NEXT:           "file-deps": [
+// CHECK-NEXT:             "[[PREFIX]]/tu.m",
+// CHECK-NEXT:             "[[PREFIX]]/shared/H.h"
+// CHECK-NEXT:           ],
+// CHECK-NEXT:           "input-file": "[[PREFIX]]/tu.m"
+// CHECK-NEXT:         }
+// CHECK-NEXT:       ]
+// CHECK-NEXT:     }
+// CHECK-NEXT:   ]
+// CHECK-NEXT: }
+
+// RUN: %deps-to-rsp %t/result.json --module-name=B > %t/B.cc1.rsp
+// RUN: %clang @%t/B.cc1.rsp
+
+// RUN: %deps-to-rsp %t/result.json --tu-index=0 > %t/tu.rsp
+// RUN: %clang @%t/tu.rsp

compiler-rt/test/orc/TestCases/Darwin/x86-64/Inputs/dlopen-dlclose-x2.s

-// Runs a sequence of dlopen, dlclose, dlopen, dlclose on a library "inits".
-// This is intended as a standard harness for testing constructor / destructor
-// behavior in the context of a full dlclose and then re-dlopen'ing of the
-// inits library.
-//
-// Compiled from:
-//
-// int main(int argc, char *argv[]) {
-//  printf("entering main\n");
-//  void *H = dlopen("inits", 0);
-//  if (!H) {
-//    printf("failed\n");
-//    return -1;
-//  }
-//  if (dlclose(H) == -1) {
-//    printf("failed\n");
-//    return -1;
-//  }
-//  H = dlopen("inits", 0);
-//  if (!H) {
-//    printf("failed\n");
-//    return -1;
-//  }
-//  if (dlclose(H) == -1) {
-//    printf("failed\n");
-//    return -1;
-//  }
-//  printf("leaving main\n");
-//  return 0;
-//}
-
-        .section  __TEXT,__text,regular,pure_instructions
-  .build_version macos, 13, 0  sdk_version 13, 0
-  .globl  _main
-  .p2align  4, 0x90
-_main:
-
-  pushq %r14
-  pushq %rbx
-  pushq %rax
-  leaq  L_str(%rip), %rdi
-  callq _puts
-  leaq  L_.str.1(%rip), %rdi
-  xorl  %esi, %esi
-  callq _dlopen
-  movl  $-1, %ebx
-  leaq  L_str.8(%rip), %r14
-  testq %rax, %rax
-  je  LBB0_4
-
-  movq  %rax, %rdi
-  callq _dlclose
-  cmpl  $-1, %eax
-  je  LBB0_4
-
-  leaq  L_.str.1(%rip), %rdi
-  xorl  %esi, %esi
-  callq _dlopen
-  testq %rax, %rax
-  je  LBB0_4
-
-  movq  %rax, %rdi
-  callq _dlclose
-  xorl  %ebx, %ebx
-  cmpl  $-1, %eax
-  sete  %bl
-  leaq  L_str.8(%rip), %rax
-  leaq  L_str.6(%rip), %r14
-  cmoveq  %rax, %r14
-  negl  %ebx
-LBB0_4:
-  movq  %r14, %rdi
-  callq _puts
-  movl  %ebx, %eax
-  addq  $8, %rsp
-  popq  %rbx
-  popq  %r14
-  retq
-
-  .section  __TEXT,__cstring,cstring_literals
-L_.str.1:
-  .asciz  "inits"
-L_str:
-  .asciz  "entering main"
-L_str.6:
-  .asciz  "leaving main"
-L_str.8:
-  .asciz  "failed"
-
-.subsections_via_symbols