[clang] [clang][Modules] Permit Link Declarations in Submodule Declarations in Module Maps (PR #156377)
Qiongsi Wu via cfe-commits
cfe-commits at lists.llvm.org
Wed Sep 3 09:01:12 PDT 2025
https://github.com/qiongsiwu updated https://github.com/llvm/llvm-project/pull/156377
>From 0cb749e2ec11cf51d2c97e935727e55aeaf65097 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Mon, 1 Sep 2025 16:09:21 -0700
Subject: [PATCH 1/2] Permit link decls in submodule declarations.
---
clang/include/clang/Basic/DiagnosticLexKinds.td | 6 ++++--
clang/lib/Lex/ModuleMapFile.cpp | 6 +-----
clang/test/ClangScanDeps/link-libraries-diag-dup.c | 2 +-
3 files changed, 6 insertions(+), 8 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index c03c4033cd3a6..eec19e462b84f 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -915,8 +915,10 @@ def err_mmap_expected_attribute : Error<"expected an attribute name">;
def warn_mmap_link_redeclaration : Warning<"redeclaration of link library '%0'">,
InGroup<DiagGroup<"module-link-redeclaration">>, DefaultError;
def note_mmap_prev_link_declaration : Note<"previously declared here">;
-def err_mmap_submodule_link_decl
- : Error<"link declaration is not allowed in submodules">;
+def warn_mmap_submodule_link_decl
+ : Warning<"link declaration is not allowed in submodules">,
+ InGroup<DiagGroup<"module-submodule-link-decl">>,
+ DefaultError;
def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">,
InGroup<IgnoredAttributes>;
def warn_mmap_mismatched_private_submodule : Warning<
diff --git a/clang/lib/Lex/ModuleMapFile.cpp b/clang/lib/Lex/ModuleMapFile.cpp
index f0cd9d2bee82a..7ceda7a0486d8 100644
--- a/clang/lib/Lex/ModuleMapFile.cpp
+++ b/clang/lib/Lex/ModuleMapFile.cpp
@@ -856,9 +856,7 @@ std::optional<LinkDecl> ModuleMapFileParser::parseLinkDecl(
// Make sure we eat all the tokens when we report the errors so parsing
// can continue.
if (!Allowed) {
- Diags.Report(LD.Location, diag::err_mmap_submodule_link_decl);
- HadError = true;
- return std::nullopt;
+ Diags.Report(LD.Location, diag::warn_mmap_submodule_link_decl);
}
auto [It, Inserted] =
@@ -866,8 +864,6 @@ std::optional<LinkDecl> ModuleMapFileParser::parseLinkDecl(
if (!Inserted) {
Diags.Report(LD.Location, diag::warn_mmap_link_redeclaration) << Library;
Diags.Report(It->second, diag::note_mmap_prev_link_declaration);
- HadError = true;
- return std::nullopt;
}
return std::move(LD);
diff --git a/clang/test/ClangScanDeps/link-libraries-diag-dup.c b/clang/test/ClangScanDeps/link-libraries-diag-dup.c
index e6612ca7bd216..ffb29bd15a1c8 100644
--- a/clang/test/ClangScanDeps/link-libraries-diag-dup.c
+++ b/clang/test/ClangScanDeps/link-libraries-diag-dup.c
@@ -51,7 +51,7 @@ module C {
// Note that module D does not report an error because it is explicit.
// Therefore we can use CHECK-NEXT for the redeclaration error on line 15.
-// CHECK: module.modulemap:6:5: error: link declaration is not allowed in submodules
+// CHECK: module.modulemap:6:5: error: link declaration is not allowed in submodules [-Wmodule-submodule-link-decl]
// CHECK-NEXT: module.modulemap:15:3: error: redeclaration of link library 'libraryA' [-Wmodule-link-redeclaration]
// CHECK-NEXT: module.modulemap:14:3: note: previously declared here
// CHECK-NOT: module.modulemap:20:3: error: redeclaration of link library 'libraryA'
>From 4b36ae5bbcf5e0647d54687badf216f6a7763711 Mon Sep 17 00:00:00 2001
From: Qiongsi Wu <qiongsi_wu at apple.com>
Date: Tue, 2 Sep 2025 16:51:44 -0700
Subject: [PATCH 2/2] Remove the check against link decls in submodules.
---
clang/include/clang/Basic/DiagnosticLexKinds.td | 4 ----
clang/lib/Lex/ModuleMapFile.cpp | 12 ++++--------
.../test/ClangScanDeps/link-libraries-diag-dup.c | 15 +++++++++------
3 files changed, 13 insertions(+), 18 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index eec19e462b84f..73213f02df643 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -915,10 +915,6 @@ def err_mmap_expected_attribute : Error<"expected an attribute name">;
def warn_mmap_link_redeclaration : Warning<"redeclaration of link library '%0'">,
InGroup<DiagGroup<"module-link-redeclaration">>, DefaultError;
def note_mmap_prev_link_declaration : Note<"previously declared here">;
-def warn_mmap_submodule_link_decl
- : Warning<"link declaration is not allowed in submodules">,
- InGroup<DiagGroup<"module-submodule-link-decl">>,
- DefaultError;
def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">,
InGroup<IgnoredAttributes>;
def warn_mmap_mismatched_private_submodule : Warning<
diff --git a/clang/lib/Lex/ModuleMapFile.cpp b/clang/lib/Lex/ModuleMapFile.cpp
index 7ceda7a0486d8..727079deec483 100644
--- a/clang/lib/Lex/ModuleMapFile.cpp
+++ b/clang/lib/Lex/ModuleMapFile.cpp
@@ -119,7 +119,7 @@ struct ModuleMapFileParser {
std::optional<UmbrellaDirDecl>
parseUmbrellaDirDecl(SourceLocation UmbrellaLoc);
std::optional<LinkDecl>
- parseLinkDecl(llvm::StringMap<SourceLocation> &SeenLinkDecl, bool Allowed);
+ parseLinkDecl(llvm::StringMap<SourceLocation> &SeenLinkDecl);
SourceLocation consumeToken();
void skipUntil(MMToken::TokenKind K);
@@ -409,7 +409,7 @@ std::optional<ModuleDecl> ModuleMapFileParser::parseModuleDecl(bool TopLevel) {
case MMToken::LinkKeyword:
// Link decls are only allowed in top level modules or explicit
// submodules.
- SubDecl = parseLinkDecl(SeenLinkDecl, TopLevel || MDecl.Explicit);
+ SubDecl = parseLinkDecl(SeenLinkDecl);
break;
default:
@@ -827,7 +827,7 @@ ModuleMapFileParser::parseUmbrellaDirDecl(clang::SourceLocation UmbrellaLoc) {
/// module-declaration:
/// 'link' 'framework'[opt] string-literal
std::optional<LinkDecl> ModuleMapFileParser::parseLinkDecl(
- llvm::StringMap<SourceLocation> &SeenLinkDecl, bool Allowed) {
+ llvm::StringMap<SourceLocation> &SeenLinkDecl) {
assert(Tok.is(MMToken::LinkKeyword));
LinkDecl LD;
LD.Location = consumeToken();
@@ -853,12 +853,8 @@ std::optional<LinkDecl> ModuleMapFileParser::parseLinkDecl(
LD.Library = Library;
consumeToken();
- // Make sure we eat all the tokens when we report the errors so parsing
+ // Make sure we eat all the token when we report the errors so parsing
// can continue.
- if (!Allowed) {
- Diags.Report(LD.Location, diag::warn_mmap_submodule_link_decl);
- }
-
auto [It, Inserted] =
SeenLinkDecl.insert(std::make_pair(Library, LD.Location));
if (!Inserted) {
diff --git a/clang/test/ClangScanDeps/link-libraries-diag-dup.c b/clang/test/ClangScanDeps/link-libraries-diag-dup.c
index ffb29bd15a1c8..79087924d37a4 100644
--- a/clang/test/ClangScanDeps/link-libraries-diag-dup.c
+++ b/clang/test/ClangScanDeps/link-libraries-diag-dup.c
@@ -14,8 +14,10 @@ module A {
explicit module D {
header "D.h"
link "libraryD"
+ link "libraryD"
}
+ link "libraryB"
link "libraryA"
link "libraryA"
}
@@ -49,9 +51,10 @@ module C {
// RUN: not clang-scan-deps -compilation-database %t/cdb.json -format \
// RUN: experimental-full 2>&1 | FileCheck %s
-// Note that module D does not report an error because it is explicit.
-// Therefore we can use CHECK-NEXT for the redeclaration error on line 15.
-// CHECK: module.modulemap:6:5: error: link declaration is not allowed in submodules [-Wmodule-submodule-link-decl]
-// CHECK-NEXT: module.modulemap:15:3: error: redeclaration of link library 'libraryA' [-Wmodule-link-redeclaration]
-// CHECK-NEXT: module.modulemap:14:3: note: previously declared here
-// CHECK-NOT: module.modulemap:20:3: error: redeclaration of link library 'libraryA'
+// Note that the `link "libraryB"` in the top level module A does not
+// cause an issue because we only check within a module.
+// CHECK: 12:5: error: redeclaration of link library 'libraryD' [-Wmodule-link-redeclaration]
+// CHECK-NEXT: 11:5: note: previously declared here
+// CHECK-NEXT: 17:3: error: redeclaration of link library 'libraryA' [-Wmodule-link-redeclaration]
+// CHECK-NEXT: 16:3: note: previously declared here
+// CHECK-NOT: error: redeclaration of link library 'libraryB'
More information about the cfe-commits
mailing list