[clang][Modules] Permit Link Declarations in Submodule Declarations in Module Maps

[qiongsiwu]

#148959 added two checks that restricts the uses of link declarations. It is later discovered that the Swift project uses link declarations in submodules extensively on Windows. This PR removes the check against link declarations in submodules.

This PR also fixes a logic bug where the incorrect link decl is returned if we turn off the warnings with the -Wno arguments.

rdar://159467837

[llvmbot]

@llvm/pr-subscribers-clang

Author: Qiongsi Wu (qiongsiwu)

Changes

#148959 added two checks that restricts the uses of link declarations. It is later discovered that the Swift project uses link declarations in submodules extensively on Windows. This PR softens the error into a warning that defaults to error, and can be disabled using -Wno-module-submodule-link-decl.

This PR also fixes a logic bug where the incorrect link decl is returned if we turn off the warnings with the -Wno arguments.

rdar://159467837

---

Full diff: https://github.com/llvm/llvm-project/pull/156377.diff

3 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticLexKinds.td (+4-2)
• (modified) clang/lib/Lex/ModuleMapFile.cpp (+1-5)
• (modified) clang/test/ClangScanDeps/link-libraries-diag-dup.c (+1-1)

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'

[cyndyishida]

For Wmodule-link-redeclaration check, what happens if two different modules have the same link declaration?

[cyndyishida]

It would be nice if there were some kind of information on how the user should fix this.
For example, could you add a comment that includes "expected top-level module" e.g. in warn_mmap_mismatched_private_submodule?

[qiongsiwu]

For Wmodule-link-redeclaration check, what happens if two different modules have the same link declaration?

The check is within a module. It does not do anything in the case when the same link decl shows up in different modules.