From 0cb749e2ec11cf51d2c97e935727e55aeaf65097 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Mon, 1 Sep 2025 16:09:21 -0700 Subject: [PATCH 1/3] 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>, 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>, + DefaultError; def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">, InGroup; 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 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 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 Date: Tue, 2 Sep 2025 16:51:44 -0700 Subject: [PATCH 2/3] 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>, 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>, - DefaultError; def warn_mmap_unknown_attribute : Warning<"unknown attribute '%0'">, InGroup; 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 parseUmbrellaDirDecl(SourceLocation UmbrellaLoc); std::optional - parseLinkDecl(llvm::StringMap &SeenLinkDecl, bool Allowed); + parseLinkDecl(llvm::StringMap &SeenLinkDecl); SourceLocation consumeToken(); void skipUntil(MMToken::TokenKind K); @@ -409,7 +409,7 @@ std::optional 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 ModuleMapFileParser::parseLinkDecl( - llvm::StringMap &SeenLinkDecl, bool Allowed) { + llvm::StringMap &SeenLinkDecl) { assert(Tok.is(MMToken::LinkKeyword)); LinkDecl LD; LD.Location = consumeToken(); @@ -853,12 +853,8 @@ std::optional 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' From 7a3297724fb1f2aea406a8118b67b2045331fea8 Mon Sep 17 00:00:00 2001 From: Qiongsi Wu Date: Wed, 3 Sep 2025 14:37:11 -0700 Subject: [PATCH 3/3] Remove incorrect comment. --- clang/lib/Lex/ModuleMapFile.cpp | 2 -- 1 file changed, 2 deletions(-) diff --git a/clang/lib/Lex/ModuleMapFile.cpp b/clang/lib/Lex/ModuleMapFile.cpp index 727079deec483..8de06dac6828b 100644 --- a/clang/lib/Lex/ModuleMapFile.cpp +++ b/clang/lib/Lex/ModuleMapFile.cpp @@ -407,8 +407,6 @@ std::optional ModuleMapFileParser::parseModuleDecl(bool TopLevel) { break; case MMToken::LinkKeyword: - // Link decls are only allowed in top level modules or explicit - // submodules. SubDecl = parseLinkDecl(SeenLinkDecl); break;