On 15/06/2014 22:42, David Majnemer wrote:
Hi rsmith,
CL permits static redeclarations to follow extern declarations. The
storage specifier on the latter declaration has no effect.
This fixes PR20034.
http://reviews.llvm.org/D4149
Files:
include/clang/Basic/DiagnosticSemaKinds.td
lib/Sema/SemaDecl.cpp
test/Misc/warning-flags.c
test/Sema/private-extern.c
test/Sema/tentative-decls.c
test/Sema/thread-specifier.c
test/Sema/var-redecl.c
test/SemaCXX/MicrosoftExtensions.cpp
D4149.10428.patch
Index: include/clang/Basic/DiagnosticSemaKinds.td
===================================================================
--- include/clang/Basic/DiagnosticSemaKinds.td
+++ include/clang/Basic/DiagnosticSemaKinds.td
@@ -3809,8 +3809,8 @@
"declared %select{in global scope|with C language linkage}0 here">;
def warn_weak_import : Warning <
"an already-declared variable is made a weak_import declaration %0">;
-def warn_static_non_static : ExtWarn<
- "static declaration of %0 follows non-static declaration">;
+def ext_static_non_static : Extension<
+ "redeclaring non-static %0 as static is a Microsoft extension">,
InGroup<Microsoft>;
def err_non_static_static : Error<
"non-static declaration of %0 follows static declaration">;
def err_extern_non_extern : Error<
Index: lib/Sema/SemaDecl.cpp
===================================================================
--- lib/Sema/SemaDecl.cpp
+++ lib/Sema/SemaDecl.cpp
@@ -2254,6 +2254,24 @@
return Sema::CXXInvalid;
}
+// Determine whether the previous declaration was a definition, implicit
+// declaration, or a declaration.
+template <typename T>
+static std::pair<diag::kind, SourceLocation>
+getNoteDiagForInvalidRedeclaration(const T *Old, const T *New) {
Nice utility function. There's at least one place in SemaDeclCXX.cpp
that could use this too. Wonder if it's worth putting in a header?
+ diag::kind PrevDiag;
+ SourceLocation OldLocation = Old->getLocation();
+ if (Old->isThisDeclarationADefinition())
+ PrevDiag = diag::note_previous_definition;
+ else if (Old->isImplicit()) {
+ PrevDiag = diag::note_previous_implicit_declaration;
+ if (OldLocation.isInvalid())
+ OldLocation = New->getLocation();
+ } else
+ PrevDiag = diag::note_previous_declaration;
+ return std::make_pair(PrevDiag, OldLocation);
The use of a std::pair here isn't convincing to me. Would a couple of
reference parameters (&NoteID, &PrevLocation) be more descriptive?
+}
+
/// canRedefineFunction - checks if a function can be redefined. Currently,
/// only extern inline functions can be redefined, and even then only in
/// GNU89 mode.
@@ -2346,18 +2364,10 @@
if (Old->isInvalidDecl())
return true;
- // Determine whether the previous declaration was a definition,
- // implicit declaration, or a declaration.
diag::kind PrevDiag;
- SourceLocation OldLocation = Old->getLocation();
- if (Old->isThisDeclarationADefinition())
- PrevDiag = diag::note_previous_definition;
- else if (Old->isImplicit()) {
- PrevDiag = diag::note_previous_implicit_declaration;
- if (OldLocation.isInvalid())
- OldLocation = New->getLocation();
- } else
- PrevDiag = diag::note_previous_declaration;
+ SourceLocation OldLocation;
+ std::tie(PrevDiag, OldLocation) =
+ getNoteDiagForInvalidRedeclaration(Old, New);
PrevDiag usually refers to a previously emitted diagnostic so it feels
like the wrong name to use here. I know it was this way before your
patch but could you rename these to NoteID and PrevLocation?
// Don't complain about this if we're in GNU89 mode and the old function
// is an extern inline function.
@@ -2369,7 +2379,7 @@
!New->getTemplateSpecializationInfo() &&
!canRedefineFunction(Old, getLangOpts())) {
if (getLangOpts().MicrosoftExt) {
- Diag(New->getLocation(), diag::warn_static_non_static) << New;
+ Diag(New->getLocation(), diag::ext_static_non_static) << New;
Diag(OldLocation, PrevDiag);
} else {
Diag(New->getLocation(), diag::err_static_non_static) << New;
@@ -3070,13 +3080,25 @@
if (New->isInvalidDecl())
return;
+ diag::kind PrevDiag;
+ SourceLocation OldLocation;
+ std::tie(PrevDiag, OldLocation) =
+ getNoteDiagForInvalidRedeclaration(Old, New);
+
// [dcl.stc]p8: Check if we have a non-static decl followed by a static.
if (New->getStorageClass() == SC_Static &&
!New->isStaticDataMember() &&
Old->hasExternalFormalLinkage()) {
- Diag(New->getLocation(), diag::err_static_non_static) <<
New->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_definition);
- return New->setInvalidDecl();
+ if (getLangOpts().MicrosoftExt) {
+ Diag(New->getLocation(), diag::ext_static_non_static)
+ << New->getDeclName();
+ Diag(OldLocation, PrevDiag);
+ } else {
+ Diag(New->getLocation(), diag::err_static_non_static)
+ << New->getDeclName();
+ Diag(OldLocation, PrevDiag);
+ return New->setInvalidDecl();
+ }
}
This warn/ext/err pattern is unfortunate. Guess there's not much we can
do about it though.
Alp.
// C99 6.2.2p4:
// For an identifier declared with the storage-class specifier
@@ -3093,21 +3115,21 @@
!New->isStaticDataMember() &&
Old->getCanonicalDecl()->getStorageClass() == SC_Static) {
Diag(New->getLocation(), diag::err_non_static_static) <<
New->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_definition);
+ Diag(OldLocation, PrevDiag);
return New->setInvalidDecl();
}
// Check if extern is followed by non-extern and vice-versa.
if (New->hasExternalStorage() &&
!Old->hasLinkage() && Old->isLocalVarDecl()) {
Diag(New->getLocation(), diag::err_extern_non_extern) <<
New->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_definition);
+ Diag(OldLocation, PrevDiag);
return New->setInvalidDecl();
}
if (Old->hasLinkage() && New->isLocalVarDecl() &&
!New->hasExternalStorage()) {
Diag(New->getLocation(), diag::err_non_extern_extern) <<
New->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_definition);
+ Diag(OldLocation, PrevDiag);
return New->setInvalidDecl();
}
@@ -3120,25 +3142,25 @@
!(Old->getLexicalDeclContext()->isRecord() &&
!New->getLexicalDeclContext()->isRecord())) {
Diag(New->getLocation(), diag::err_redefinition) << New->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_definition);
+ Diag(OldLocation, PrevDiag);
return New->setInvalidDecl();
}
if (New->getTLSKind() != Old->getTLSKind()) {
if (!Old->getTLSKind()) {
Diag(New->getLocation(), diag::err_thread_non_thread) <<
New->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_declaration);
+ Diag(OldLocation, PrevDiag);
} else if (!New->getTLSKind()) {
Diag(New->getLocation(), diag::err_non_thread_thread) <<
New->getDeclName();
- Diag(Old->getLocation(), diag::note_previous_declaration);
+ Diag(OldLocation, PrevDiag);
} else {
// Do not allow redeclaration to change the variable between requiring
// static and dynamic initialization.
// FIXME: GCC allows this, but uses the TLS keyword on the first
// declaration to determine the kind. Do we need to be compatible here?
Diag(New->getLocation(), diag::err_thread_thread_different_kind)
<< New->getDeclName() << (New->getTLSKind() == VarDecl::TLS_Dynamic);
- Diag(Old->getLocation(), diag::note_previous_declaration);
+ Diag(OldLocation, PrevDiag);
}
}
@@ -3155,7 +3177,7 @@
if (haveIncompatibleLanguageLinkages(Old, New)) {
Diag(New->getLocation(), diag::err_different_language_linkage) << New;
- Diag(Old->getLocation(), diag::note_previous_definition);
+ Diag(OldLocation, PrevDiag);
New->setInvalidDecl();
return;
}
Index: test/Misc/warning-flags.c
===================================================================
--- test/Misc/warning-flags.c
+++ test/Misc/warning-flags.c
@@ -18,7 +18,7 @@
The list of warnings below should NEVER grow. It should gradually shrink to 0.
-CHECK: Warnings without flags (106):
+CHECK: Warnings without flags (105):
CHECK-NEXT: ext_delete_void_ptr_operand
CHECK-NEXT: ext_expected_semi_decl_list
CHECK-NEXT: ext_explicit_specialization_storage_class
@@ -114,7 +114,6 @@
CHECK-NEXT: warn_register_objc_catch_parm
CHECK-NEXT: warn_related_result_type_compatibility_class
CHECK-NEXT: warn_related_result_type_compatibility_protocol
-CHECK-NEXT: warn_static_non_static
CHECK-NEXT: warn_template_export_unsupported
CHECK-NEXT: warn_template_spec_extra_headers
CHECK-NEXT: warn_tentative_incomplete_array
Index: test/Sema/private-extern.c
===================================================================
--- test/Sema/private-extern.c
+++ test/Sema/private-extern.c
@@ -13,10 +13,10 @@
int g3; // expected-note{{previous definition}}
static int g3; // expected-error{{static declaration of 'g3' follows
non-static declaration}}
-extern int g4; // expected-note{{previous definition}}
+extern int g4; // expected-note{{previous declaration}}
static int g4; // expected-error{{static declaration of 'g4' follows
non-static declaration}}
-__private_extern__ int g5; // expected-note{{previous definition}}
+__private_extern__ int g5; // expected-note{{previous declaration}}
static int g5; // expected-error{{static declaration of 'g5' follows
non-static declaration}}
void f0() {
@@ -30,12 +30,12 @@
}
void f2() {
- extern int g8; // expected-note{{previous definition}}
+ extern int g8; // expected-note{{previous declaration}}
int g8; // expected-error {{non-extern declaration of 'g8' follows extern
declaration}}
}
void f3() {
- __private_extern__ int g9; // expected-note{{previous definition}}
+ __private_extern__ int g9; // expected-note{{previous declaration}}
int g9; // expected-error {{non-extern declaration of 'g9' follows extern
declaration}}
}
Index: test/Sema/tentative-decls.c
===================================================================
--- test/Sema/tentative-decls.c
+++ test/Sema/tentative-decls.c
@@ -23,7 +23,7 @@
int i1 = 2; // expected-error {{redefinition of 'i1'}}
int i1;
int i1;
-extern int i5; // expected-note {{previous definition is here}}
+extern int i5; // expected-note {{previous declaration is here}}
static int i5; // expected-error{{static declaration of 'i5' follows
non-static declaration}}
static int i2 = 5; // expected-note 1 {{previous definition is here}}
@@ -49,7 +49,7 @@
int redef[11]; // expected-error{{redefinition of 'redef'}}
void func() {
- extern int i6; // expected-note {{previous definition is here}}
+ extern int i6; // expected-note {{previous declaration is here}}
static int i6; // expected-error{{static declaration of 'i6' follows
non-static declaration}}
}
Index: test/Sema/thread-specifier.c
===================================================================
--- test/Sema/thread-specifier.c
+++ test/Sema/thread-specifier.c
@@ -58,7 +58,7 @@
}
__thread typedef int t14; // expected-error-re {{cannot combine with previous '{{__thread|_Thread_local|thread_local}}' declaration specifier}}
-__thread int t15; // expected-note {{previous declaration is here}}
+__thread int t15; // expected-note {{previous definition is here}}
extern int t15; // expected-error {{non-thread-local declaration of 't15'
follows thread-local declaration}}
extern int t16; // expected-note {{previous declaration is here}}
__thread int t16; // expected-error {{thread-local declaration of 't16'
follows non-thread-local declaration}}
Index: test/Sema/var-redecl.c
===================================================================
--- test/Sema/var-redecl.c
+++ test/Sema/var-redecl.c
@@ -58,5 +58,5 @@
// PR3645
static int a;
-extern int a; // expected-note {{previous definition is here}}
+extern int a; // expected-note {{previous declaration is here}}
int a; // expected-error {{non-static declaration of 'a' follows
static declaration}}
Index: test/SemaCXX/MicrosoftExtensions.cpp
===================================================================
--- test/SemaCXX/MicrosoftExtensions.cpp
+++ test/SemaCXX/MicrosoftExtensions.cpp
@@ -144,11 +144,14 @@
void static_func(); // expected-note {{previous declaration is here}}
-static void static_func() // expected-warning {{static declaration of 'static_func' follows non-static declaration}}
+static void static_func() // expected-warning {{redeclaring non-static
'static_func' as static is a Microsoft extension}}
{
}
+extern const int static_var; // expected-note {{previous declaration is here}}
+static const int static_var = 3; // expected-warning {{redeclaring non-static
'static_var' as static is a Microsoft extension}}
+
long function_prototype(int a);
long (*function_ptr)(int a);
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
