[clang] [clang][Modules] Diagnosing Module Redefinition Across ModuleMaps (PR #190085)
Qiongsi Wu via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 7 15:50:35 PDT 2026
https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/190085
>From 1c48cbcdb2a8c8557c0ed207e62e30f410f49776 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Wed, 1 Apr 2026 16:13:42 -0700
Subject: [PATCH 1/3] Adding a diagnostics to check for one more specific case
of duplicating module declarations.
---
clang/lib/Lex/ModuleMap.cpp | 19 ++++-
clang/lib/Tooling/DependencyScanningTool.cpp | 6 ++
.../modules-byname-dup-module-decl-diag.c | 73 +++++++++++++++++++
.../Modules/implicit-module-redefinition.c | 56 ++++++++++++++
4 files changed, 152 insertions(+), 2 deletions(-)
create mode 100644 clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c
create mode 100644 clang/test/Modules/implicit-module-redefinition.c
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index 6c991430cb08b..ce8c8b576e614 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1756,9 +1756,24 @@ void ModuleMapLoader::handleModuleDecl(const modulemap::ModuleDecl &MD) {
if (Module *Existing = Map.lookupModuleQualified(ModuleName, ActiveModule)) {
// We might see a (re)definition of a module that we already have a
// definition for in four cases:
- // - If we loaded one definition from an AST file and we've just found a
- // corresponding definition in a module map file, or
+ // - If we loaded one definition from an AST file and we've just found the
+ // corresponding definition in the same module map file, or
bool LoadedFromASTFile = Existing->IsFromModuleFile;
+ if (LoadedFromASTFile) {
+ // Only suppress the redefinition error if the existing module was defined
+ // in the same module map file we are currently parsing. If they come from
+ // different module map files, this is a genuine conflict that should be
+ // diagnosed.
+ OptionalFileEntryRef ExistingModMapFile =
+ Map.getContainingModuleMapFile(Existing);
+ OptionalFileEntryRef CurrentModMapFile =
+ SourceMgr.getFileEntryRefForID(ModuleMapFID);
+ if (ExistingModMapFile && CurrentModMapFile &&
+ *ExistingModMapFile == *CurrentModMapFile)
+ LoadedFromASTFile = true;
+ else
+ LoadedFromASTFile = false;
+ }
// - If we previously inferred this module from different module map file.
bool Inferred = Existing->IsInferred;
// - If we're building a framework that vends a module map, we might've
diff --git a/clang/lib/Tooling/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanningTool.cpp
index adf0c10612468..19055158761ef 100644
--- a/clang/lib/Tooling/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanningTool.cpp
@@ -605,6 +605,12 @@ bool CompilerInstanceWithContext::computeDependencies(
if (!ModResult)
return false;
+ // Check for errors that occurred as side effects during module loading
+ // (e.g., modulemap parsing errors like duplicate module definitions).
+ // loadModule may return a valid module even when such errors occur.
+ if (CI.getDiagnostics().hasErrorOccurred())
+ return false;
+
MDC->applyDiscoveredDependencies(ModuleInvocation);
if (!Controller.finalize(CI, ModuleInvocation))
diff --git a/clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c b/clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c
new file mode 100644
index 0000000000000..a6deb71f1d10b
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c
@@ -0,0 +1,73 @@
+// Test duplicating module decls discovered during by-name module scanning with
+// a shared compiler instance.
+// This tests covers the case where the current modulemap we are loading
+// contains a module decl that satisfies two conditions:
+// 1. The compiler has seen the module decl during previous lookups.
+// 2. The previous decl comes from a different modulemap.
+// In this case, an error is produced with no dependency information returned.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// Scenario setup:
+// - "A" is a framework module whose modulemap also declares an empty "B".
+// - A separate include path has its own B/module.modulemap that declares "B"
+// with a header depending on module "Dep"
+// We scan B first, and during A's scan, the compiler should report an error.
+
+// RUN: not clang-scan-deps -compilation-database %t/cdb.json -format \
+// RUN: experimental-full -module-names=B,A > %t/result_ab.json 2> %t/err.txt
+// RUN: cat %t/result_ab.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
+// RUN: cat %t/err.txt | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t \
+// RUN: -check-prefix=ERROR %s
+
+// We should not produce scanning results for A, since there is a duplicating
+// module decl.
+// CHECK-NOT: "name": "A"
+
+// ERROR: A.framework/Modules/module.modulemap:8:8: error: redefinition of module 'B'
+// ERROR: include/B/module.modulemap:1:8: note: previously defined here
+
+//--- frameworks/A.framework/Modules/module.modulemap
+framework module A [system] {
+ umbrella header "A.h"
+ export *
+ module * { export * }
+}
+
+// This empty definition of B shadows the real one when cached.
+module B {
+ export *
+}
+
+//--- frameworks/A.framework/Headers/A.h
+// Framework A umbrella header
+void a_func(void);
+
+//--- include/B/module.modulemap
+module B {
+ header "B.h"
+ export *
+}
+
+//--- include/B/B.h
+#include "Dep.h"
+void b_func(void);
+
+//--- include/Dep/module.modulemap
+module Dep {
+ header "Dep.h"
+ export *
+}
+
+//--- include/Dep/Dep.h
+// Module Dep header
+void dep_func(void);
+
+//--- cdb.json.template
+[{
+ "file": "",
+ "directory": "DIR",
+ "command": "clang -fmodules -fmodules-cache-path=DIR/cache -I DIR/include/B -F DIR/frameworks -I DIR/include/Dep -x c"
+}]
diff --git a/clang/test/Modules/implicit-module-redefinition.c b/clang/test/Modules/implicit-module-redefinition.c
new file mode 100644
index 0000000000000..d54e79d309943
--- /dev/null
+++ b/clang/test/Modules/implicit-module-redefinition.c
@@ -0,0 +1,56 @@
+// Test that implicit module builds diagnose redefinition of a module when two
+// different modulemaps define the same module name.
+//
+// Module "b" is defined in both first/module.modulemap and
+// second/module.modulemap.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// We detect the error when we read a's modulemap in first
+// and discovers another b, which we have seen earlier in
+// second/module.modulemap.
+// RUN: not %clang_cc1 -x objective-c -fmodules -fimplicit-module-maps \
+// RUN: -I %t/second -I %t/first \
+// RUN: -fmodules-cache-path=%t/cache \
+// RUN: %t/sourcefile.c 2>&1 | FileCheck %s
+
+// CHECK: first{{[/\\]}}module.modulemap:5:8: error: redefinition of module 'b'
+// CHECK: second{{[/\\]}}module.modulemap:1:8: note: previously defined here
+
+// On the other hand, we do NOT detect error if we load a's modulemap first.
+// Checking duplicating module decls in general is too expensive, since
+// it requires loading all the modulemaps. The command below should succeed.
+// RUN: %clang_cc1 -x objective-c -fmodules -fimplicit-module-maps \
+// RUN: -I %t/first -I %t/second \
+// RUN: -fmodules-cache-path=%t/cache \
+// RUN: %t/sourcefile.c
+
+//--- first/module.modulemap
+module a {
+ header "a.h"
+}
+
+module b {
+ export *
+}
+
+//--- first/a.h
+// empty
+
+//--- second/module.modulemap
+module b {
+ header "b.h"
+}
+
+//--- second/b.h
+// empty
+
+
+//--- sourcefile.c
+ at import b;
+ at import a;
+
+int main() {
+ return 0;
+}
>From cc71d8515303f890b8dac81cf2715432f8ca6782 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Tue, 7 Apr 2026 15:11:46 -0700
Subject: [PATCH 2/3] Address code review comments.
---
clang/lib/Lex/ModuleMap.cpp | 8 ++------
clang/lib/Tooling/DependencyScanningTool.cpp | 3 ---
.../modules-byname-dup-module-decl-diag.c | 11 +++++------
3 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp
index ce8c8b576e614..099ceb992fb1f 100644
--- a/clang/lib/Lex/ModuleMap.cpp
+++ b/clang/lib/Lex/ModuleMap.cpp
@@ -1756,14 +1756,10 @@ void ModuleMapLoader::handleModuleDecl(const modulemap::ModuleDecl &MD) {
if (Module *Existing = Map.lookupModuleQualified(ModuleName, ActiveModule)) {
// We might see a (re)definition of a module that we already have a
// definition for in four cases:
- // - If we loaded one definition from an AST file and we've just found the
- // corresponding definition in the same module map file, or
+ // - If the module was loaded from an AST file and we've found its original
+ // definition again (same source module map)
bool LoadedFromASTFile = Existing->IsFromModuleFile;
if (LoadedFromASTFile) {
- // Only suppress the redefinition error if the existing module was defined
- // in the same module map file we are currently parsing. If they come from
- // different module map files, this is a genuine conflict that should be
- // diagnosed.
OptionalFileEntryRef ExistingModMapFile =
Map.getContainingModuleMapFile(Existing);
OptionalFileEntryRef CurrentModMapFile =
diff --git a/clang/lib/Tooling/DependencyScanningTool.cpp b/clang/lib/Tooling/DependencyScanningTool.cpp
index 19055158761ef..2bb74fb8b5a3a 100644
--- a/clang/lib/Tooling/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanningTool.cpp
@@ -605,9 +605,6 @@ bool CompilerInstanceWithContext::computeDependencies(
if (!ModResult)
return false;
- // Check for errors that occurred as side effects during module loading
- // (e.g., modulemap parsing errors like duplicate module definitions).
- // loadModule may return a valid module even when such errors occur.
if (CI.getDiagnostics().hasErrorOccurred())
return false;
diff --git a/clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c b/clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c
index a6deb71f1d10b..74aceea6f3550 100644
--- a/clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c
+++ b/clang/test/ClangScanDeps/modules-byname-dup-module-decl-diag.c
@@ -5,17 +5,16 @@
// 1. The compiler has seen the module decl during previous lookups.
// 2. The previous decl comes from a different modulemap.
// In this case, an error is produced with no dependency information returned.
-
-// RUN: rm -rf %t
-// RUN: split-file %s %t
-// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
-
-// Scenario setup:
+// Specifically, we have the following setup:
// - "A" is a framework module whose modulemap also declares an empty "B".
// - A separate include path has its own B/module.modulemap that declares "B"
// with a header depending on module "Dep"
// We scan B first, and during A's scan, the compiler should report an error.
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
// RUN: not clang-scan-deps -compilation-database %t/cdb.json -format \
// RUN: experimental-full -module-names=B,A > %t/result_ab.json 2> %t/err.txt
// RUN: cat %t/result_ab.json | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t %s
>From 7649bb15df3cd586b75b404ee2b9477b4859bcb0 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Tue, 7 Apr 2026 15:50:17 -0700
Subject: [PATCH 3/3] Adding two test cases against duplicating module
defintions coming from the same modulemap.
---
clang/test/ClangScanDeps/modules-mmap-redef.c | 40 +++++++++++++++++++
.../implicit-module-redefinition-same-file.c | 36 +++++++++++++++++
2 files changed, 76 insertions(+)
create mode 100644 clang/test/ClangScanDeps/modules-mmap-redef.c
create mode 100644 clang/test/Modules/implicit-module-redefinition-same-file.c
diff --git a/clang/test/ClangScanDeps/modules-mmap-redef.c b/clang/test/ClangScanDeps/modules-mmap-redef.c
new file mode 100644
index 0000000000000..589eaaf9045cc
--- /dev/null
+++ b/clang/test/ClangScanDeps/modules-mmap-redef.c
@@ -0,0 +1,40 @@
+// Test duplicating module definitions in the same modulemap.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: not clang-scan-deps -compilation-database %t/cdb.json -format \
+// RUN: experimental-full -module-names=A > %t/result.json 2> %t/err.txt
+// RUN: cat %t/err.txt | sed 's:\\\\\?:/:g' | FileCheck -DPREFIX=%/t \
+// RUN: -check-prefix=ERROR %s
+
+// ERROR: include/A/module.modulemap:9:8: error: redefinition of module 'A'
+// ERROR-NEXT: include/A/module.modulemap:1:8: note: previously defined here
+
+//--- include/A/module.modulemap
+module A {
+ header "A.h"
+}
+
+module A1 {
+ header "A1.h"
+}
+
+module A{
+ header "A.h"
+}
+
+//--- include/A/A.h
+
+//--- include/A/A1.h
+
+//--- include/A/A2.h
+
+
+//--- cdb.json.template
+[{
+ "file": "",
+ "directory": "DIR",
+ "command": "clang -fmodules -fmodules-cache-path=DIR/cache -I DIR/include/A -x c"
+}]
diff --git a/clang/test/Modules/implicit-module-redefinition-same-file.c b/clang/test/Modules/implicit-module-redefinition-same-file.c
new file mode 100644
index 0000000000000..b229507c681ac
--- /dev/null
+++ b/clang/test/Modules/implicit-module-redefinition-same-file.c
@@ -0,0 +1,36 @@
+// Test that implicit module builds diagnose redefinition of a module when the
+// same modulemap file contains duplicate module declarations.
+
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: not %clang_cc1 -x objective-c -fmodules -fimplicit-module-maps \
+// RUN: -I %t/include \
+// RUN: -fmodules-cache-path=%t/cache \
+// RUN: %t/test.m 2>&1 | FileCheck %s
+
+// CHECK: module.modulemap:9:8: error: redefinition of module 'A'
+// CHECK: module.modulemap:1:8: note: previously defined here
+// CHECK: fatal error: could not build module 'A'
+
+//--- include/module.modulemap
+module A {
+ header "A.h"
+}
+
+module A1 {
+ header "A1.h"
+}
+
+module A {
+ header "A.h"
+}
+
+//--- include/A.h
+// empty
+
+//--- include/A1.h
+// empty
+
+//--- test.m
+ at import A;
More information about the cfe-commits
mailing list