diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 137e7cc3bd76f..f9b032597fa67 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -916,6 +916,11 @@ def err_mmap_nested_submodule_id : Error< "qualified module name can only be used to define modules at the top level">; def err_mmap_expected_feature : Error<"expected a feature name">; 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_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 00337b830ad4c..1c5823b410421 100644 --- a/clang/lib/Lex/ModuleMapFile.cpp +++ b/clang/lib/Lex/ModuleMapFile.cpp @@ -118,7 +118,8 @@ struct ModuleMapFileParser { std::optional parseExcludeDecl(clang::SourceLocation LeadingLoc); std::optional parseUmbrellaDirDecl(SourceLocation UmbrellaLoc); - std::optional parseLinkDecl(); + std::optional + parseLinkDecl(llvm::StringMap &SeenLinkDecl, bool Allowed); SourceLocation consumeToken(); void skipUntil(MMToken::TokenKind K); @@ -325,6 +326,7 @@ std::optional ModuleMapFileParser::parseModuleDecl(bool TopLevel) { SourceLocation LBraceLoc = consumeToken(); bool Done = false; + llvm::StringMap SeenLinkDecl; do { std::optional SubDecl; switch (Tok.Kind) { @@ -405,7 +407,9 @@ std::optional ModuleMapFileParser::parseModuleDecl(bool TopLevel) { break; case MMToken::LinkKeyword: - SubDecl = parseLinkDecl(); + // Link decls are only allowed in top level modules or explicit + // submodules. + SubDecl = parseLinkDecl(SeenLinkDecl, TopLevel || MDecl.Explicit); break; default: @@ -822,7 +826,8 @@ ModuleMapFileParser::parseUmbrellaDirDecl(clang::SourceLocation UmbrellaLoc) { /// /// module-declaration: /// 'link' 'framework'[opt] string-literal -std::optional ModuleMapFileParser::parseLinkDecl() { +std::optional ModuleMapFileParser::parseLinkDecl( + llvm::StringMap &SeenLinkDecl, bool Allowed) { assert(Tok.is(MMToken::LinkKeyword)); LinkDecl LD; LD.Location = consumeToken(); @@ -838,12 +843,33 @@ std::optional ModuleMapFileParser::parseLinkDecl() { if (!Tok.is(MMToken::StringLiteral)) { Diags.Report(Tok.getLocation(), diag::err_mmap_expected_library_name) << LD.Framework << SourceRange(LD.Location); + consumeToken(); HadError = true; return std::nullopt; } - LD.Library = Tok.getString(); + StringRef Library = Tok.getString(); + + LD.Library = Library; consumeToken(); + + // 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; + } + + auto [It, Inserted] = + SeenLinkDecl.insert(std::make_pair(Library, LD.Location)); + 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 new file mode 100644 index 0000000000000..e6612ca7bd216 --- /dev/null +++ b/clang/test/ClangScanDeps/link-libraries-diag-dup.c @@ -0,0 +1,57 @@ +// RUN: rm -rf %t +// RUN: mkdir %t +// RUN: split-file %s %t + +//--- module.modulemap +module A { + umbrella header "A.h" + + module B { + header "B.h" + link "libraryB" + } + + explicit module D { + header "D.h" + link "libraryD" + } + + link "libraryA" + link "libraryA" +} + +module C { + header "C.h" + link "libraryA" +} + +//--- A.h +#include "B.h" +//--- B.h +// empty +//--- C.h +// empty +//--- D.h +// empty +//--- TU.c +#include "A.h" +#include "C.h" +#include "D.h" + +//--- cdb.json.template +[{ + "file": "DIR/TU.c", + "directory": "DIR", + "command": "clang -fmodules -fmodules-cache-path=DIR/cache -I DIR -c DIR/TU.c" +}] + +// 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 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 +// 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'