[PATCH] Sema: Static redeclaration after extern declarations is a Microsoft Extension
Alp Toker
alp at nuanti.com
Sun Jun 15 17:11:57 PDT 2014
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
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
--
http://www.nuanti.com
the browser experts
More information about the cfe-commits
mailing list