[clang] [clang][Modules] Diagnosing Module Redefinition Across ModuleMaps (PR #190085)
Qiongsi Wu via cfe-commits
cfe-commits at lists.llvm.org
Thu Apr 2 08:44:12 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] 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;
+}
More information about the cfe-commits
mailing list