Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -4475,6 +4475,8 @@ "cannot add 'abi_tag' attribute in a redeclaration">; def err_new_abi_tag_on_redeclaration : Error< "'abi_tag' %0 missing in original declaration">; +def note_redefinition_same_file : + Note <"'%0' included multiple times, consider augmenting this header with #ifdef guards">; def note_deleted_dtor_no_operator_delete : Note< "virtual destructor requires an unambiguous, accessible 'operator delete'">; @@ -8701,6 +8703,11 @@ InGroup>; def note_equivalent_internal_linkage_decl : Note< "declared here%select{ in module '%1'|}0">; + +def note_redefinition_modules_same_file : Note< + "'%0' included multiple times, additional (likely non-modular) include site in module '%1'">; +def note_redefinition_modules_same_file_modulemap : Note< + "consider adding '%0' as part of '%1' definition in">; } let CategoryName = "Coroutines Issue" in { Index: include/clang/Sema/Sema.h =================================================================== --- include/clang/Sema/Sema.h +++ include/clang/Sema/Sema.h @@ -2282,6 +2282,7 @@ void MergeVarDeclTypes(VarDecl *New, VarDecl *Old, bool MergeTypeWithOld); void MergeVarDeclExceptionSpecs(VarDecl *New, VarDecl *Old); bool checkVarDeclRedefinition(VarDecl *OldDefn, VarDecl *NewDefn); + void diagnoseRedefinition(SourceLocation Old, SourceLocation New); bool MergeCXXFunctionDecl(FunctionDecl *New, FunctionDecl *Old, Scope *S); // AssignmentAction - This is used by all the assignment diagnostic functions Index: lib/Sema/SemaDecl.cpp =================================================================== --- lib/Sema/SemaDecl.cpp +++ lib/Sema/SemaDecl.cpp @@ -3728,6 +3728,49 @@ New->setImplicitlyInline(); } +void Sema::diagnoseRedefinition(SourceLocation Old, SourceLocation New) { + SourceManager &SrcMgr = getSourceManager(); + auto FNewDecLoc = SrcMgr.getDecomposedLoc(New); + auto FOldDecLoc = SrcMgr.getDecomposedLoc(Old); + auto *FNew = SrcMgr.getFileEntryForID(FNewDecLoc.first); + auto *FOld = SrcMgr.getFileEntryForID(FOldDecLoc.first); + // Is it the same file and same offset? If so, pointing to redefinition + // error to the same file doesn't add much. Provide more information and + // mention possible non-modular include. + if (FNew == FOld && FNewDecLoc.second == FOldDecLoc.second) { + SourceLocation IncludeLoc = SrcMgr.getIncludeLoc(FOldDecLoc.first); + if (getLangOpts().Modules) { + // Redefinition errors with modules are common with non modular mapped + // headers, example: a non-modular header H in module A that also gets + // included directly in a TU. Pointing twice to the same header/definition + // is confusing, try to get better diagnostics when modules is on. + auto OldModLoc = SrcMgr.getModuleImportLoc(Old); + if (!OldModLoc.first.isInvalid()) { + ModuleMap &Map = PP.getHeaderSearchInfo().getModuleMap(); + Module *Mod = Map.inferModuleFromLocation(FullSourceLoc(Old, SrcMgr)); + if (Mod && !IncludeLoc.isInvalid()) { + Diag(IncludeLoc, diag::note_redefinition_modules_same_file) + << SrcMgr.getFilename(SrcMgr.getSpellingLoc(Old)).str() + << Mod->getFullModuleName(); + if (!Mod->DefinitionLoc.isInvalid()) + Diag(Mod->DefinitionLoc, + diag::note_redefinition_modules_same_file_modulemap) + << SrcMgr.getFilename(SrcMgr.getSpellingLoc(Old)).str() + << Mod->getFullModuleName(); + return; + } + } + } else { // Redefinitions caused by the lack of header guards. + Diag(IncludeLoc, diag::note_redefinition_same_file) + << SrcMgr.getFilename(SrcMgr.getSpellingLoc(Old)).str(); + return; + } + } + + // Redefinition coming from different files + Diag(Old, diag::note_previous_definition); +} + /// We've just determined that \p Old and \p New both appear to be definitions /// of the same variable. Either diagnose or fix the problem. bool Sema::checkVarDeclRedefinition(VarDecl *Old, VarDecl *New) { @@ -3748,7 +3791,7 @@ return false; } else { Diag(New->getLocation(), diag::err_redefinition) << New; - Diag(Old->getLocation(), diag::note_previous_definition); + diagnoseRedefinition(Old->getLocation(), New->getLocation()); New->setInvalidDecl(); return true; } Index: test/Modules/Inputs/SameHeader/A.h =================================================================== --- /dev/null +++ test/Modules/Inputs/SameHeader/A.h @@ -0,0 +1,3 @@ +#ifndef __A_h__ +#define __A_h__ +#endif Index: test/Modules/Inputs/SameHeader/B.h =================================================================== --- /dev/null +++ test/Modules/Inputs/SameHeader/B.h @@ -0,0 +1,4 @@ +#ifndef __B_h__ +#define __B_h__ +#include "C.h" +#endif Index: test/Modules/Inputs/SameHeader/C.h =================================================================== --- /dev/null +++ test/Modules/Inputs/SameHeader/C.h @@ -0,0 +1,4 @@ +#ifndef __C_h__ +#define __C_h__ +int c = 1; +#endif Index: test/Modules/Inputs/SameHeader/module.modulemap =================================================================== --- /dev/null +++ test/Modules/Inputs/SameHeader/module.modulemap @@ -0,0 +1,11 @@ +module X { + module A { + header "A.h" + export * + } + module B { + header "B.h" + export * + } + export * +} Index: test/Modules/redefinition-same-header.m =================================================================== --- /dev/null +++ test/Modules/redefinition-same-header.m @@ -0,0 +1,10 @@ +// RUN: rm -rf %t.tmp +// RUN: %clang_cc1 -fsyntax-only -I %S/Inputs/SameHeader -fmodules \ +// RUN: -fimplicit-module-maps -fmodules-cache-path=%t.tmp %s -verify + +// expected-error@Inputs/SameHeader/C.h:3 {{redefinition of 'c'}} +// expected-note-re@Inputs/SameHeader/B.h:3 {{'{{.*}}/C.h' included multiple times, additional (likely non-modular) include site in module 'X.B'}} +// expected-note-re@Inputs/SameHeader/module.modulemap:6 {{consider adding '{{.*}}/C.h' as part of 'X.B' definition in}} + +#include "A.h" // maps to a modular +#include "C.h" // textual include Index: test/Sema/redefinition-same-header.c =================================================================== --- /dev/null +++ test/Sema/redefinition-same-header.c @@ -0,0 +1,12 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: echo 'int yyy = 42;' > %t/a.h +// RUN: %clang_cc1 -fsyntax-only %s -I%t -verify + +// expected-error@a.h:1 {{redefinition of 'yyy'}} +// expected-note-re@redefinition-same-header.c:9 {{'{{.*}}/a.h' included multiple times, consider augmenting this header with #ifdef guards}} + +#include "a.h" +#include "a.h" + +int foo() { return yyy; }