bruno created this revision.
Diagnostics related to redefinition errors that point to the same header file
do not provide much information that helps fixing the issue. In modules context
it usually happens because of a non modular include, in non-module context it
might happen because of the lack of header guardas. This patch tries to improve
this situation by enhancing diagnostics in this particular scenario. If the
approach seems reasonable, I can extend it to other relevant redefinition_error
diagnostic call sites.
Modules
-------
In file included from x.c:2:
Inputs4/C.h:3:5: error: redefinition of 'c'
int c = 1;
^
In module 'X' imported from x.c:1:
Inputs4/C.h:3:5: note: previous definition is here
int c = 1;
^
1 warning and 1 error generated.
After this patch
In file included from x.c:2:
Inputs4/C.h:3:5: error: redefinition of 'c'
int c = 1;
^
In module 'X' imported from x.c:1:
Inputs4/B.h:3:10: note: 'Inputs4/C.h' included multiple times, additional
(likely non-modular) include site in module 'X.B'
#include "C.h"
^
1 error generated.
Inputs4/module.modulemap:6:10: note: consider adding 'Inputs4/C.h' as part of
'X.B' definition in
module B {
^
1 error generated.
Without Modules
---------------
In file included from x.c:2:
./a.h:1:5: error: redefinition of 'yyy'
int yyy = 42;
^
./a.h:1:5: note: previous definition is here
int yyy = 42;
^
1 error generated.
After this patch
In file included from x.c:2:
./a.h:1:5: error: redefinition of 'yyy'
int yyy = 42;
^
x.c:1:10: note: './a.h' included multiple times, consider augmenting this
header with #ifdef guards
#include "a.h"
^
1 error generated.
https://reviews.llvm.org/D28832
Files:
include/clang/Basic/DiagnosticSemaKinds.td
include/clang/Sema/Sema.h
lib/Sema/SemaDecl.cpp
test/Modules/Inputs/SameHeader/A.h
test/Modules/Inputs/SameHeader/B.h
test/Modules/Inputs/SameHeader/C.h
test/Modules/Inputs/SameHeader/module.modulemap
test/Modules/redefinition-same-header.m
test/Sema/redefinition-same-header.c
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; }
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/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/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/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/A.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/SameHeader/A.h
@@ -0,0 +1,3 @@
+#ifndef __A_h__
+#define __A_h__
+#endif
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: 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: 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<DiagGroup<"modules-ambiguous-internal-linkage">>;
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 {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
