Sema: Static redeclaration after extern declarations is a Microsoft Extension

[majnemer]

Summary

CL permits static redeclarations to follow extern declarations. The
storage specifier on the latter declaration has no effect.

This fixes PR20034.

[rnk]

lgtm

test/Sema/private-extern.c

16

Good catch on improving these diagnostics!

[majnemer]

Closed by commit rL211238 (authored by @majnemer).

include/clang/Basic/DiagnosticSemaKinds.td

def ext_retained_language_linkage : Extension<
   InGroup<DiagGroup<"retained-language-linkage">>;
 def err_extern_c_global_conflict : Error<
   "declaration of %1 %select{with C language linkage|in global scope}0 "
   "conflicts with declaration %select{in global scope|with C language linkage}0">;
 def note_extern_c_global_conflict : Note<
   "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<
   "extern declaration of %0 follows non-extern declaration">;
 def err_non_extern_extern : Error<
   "non-extern declaration of %0 follows extern declaration">;
 def err_non_thread_thread : Error<
   "non-thread-local declaration of %0 follows thread-local declaration">;

lib/Sema/SemaDecl.cpp

if (const CXXConstructorDecl *Ctor = dyn_cast<CXXConstructorDecl>(MD)) {
     return Sema::CXXCopyAssignment;
   } else if (MD->isMoveAssignmentOperator()) {
     return Sema::CXXMoveAssignment;
   }
 
   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) {
+  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);
+}
+
 /// canRedefineFunction - checks if a function can be redefined. Currently,
 /// only extern inline functions can be redefined, and even then only in
 /// GNU89 mode.
 static bool canRedefineFunction(const FunctionDecl *FD,
                                 const LangOptions& LangOpts) {
   return ((FD->hasAttr<GNUInlineAttr>() || LangOpts.GNUInline) &&
           !LangOpts.CPlusPlus &&
           FD->isInlineSpecified() &&
if (UsingShadowDecl *Shadow = dyn_cast<UsingShadowDecl>(OldD)) {
       return true;
     }
   }
 
   // If the old declaration is invalid, just give up here.
   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);
 
   // Don't complain about this if we're in GNU89 mode and the old function
   // is an extern inline function.
   // Don't complain about specializations. They are not supposed to have
   // storage classes.
   if (!isa<CXXMethodDecl>(New) && !isa<CXXMethodDecl>(Old) &&
       New->getStorageClass() == SC_Static &&
       Old->hasExternalFormalLinkage() &&
       !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;
       Diag(OldLocation, PrevDiag);
       return true;
     }
   }
 
void Sema::MergeVarDecl(VarDecl *New, LookupResult &Previous) {
   }
 
   // Merge the types.
   MergeVarDeclTypes(New, Old, mergeTypeWithPrevious(*this, New, Old, Previous));
 
   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);
+    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();
     }
+  }
   // C99 6.2.2p4:
   //   For an identifier declared with the storage-class specifier
   //   extern in a scope in which a prior declaration of that
   //   identifier is visible,23) if the prior declaration specifies
   //   internal or external linkage, the linkage of the identifier at
   //   the later declaration is the same as the linkage specified at
   //   the prior declaration. If no prior declaration is visible, or
   //   if the prior declaration specifies no linkage, then the
   //   identifier has external linkage.
   if (New->hasExternalStorage() && Old->hasLinkage())
     /* Okay */;
   else if (New->getCanonicalDecl()->getStorageClass() != SC_Static &&
            !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();
   }
 
   // Variables with external linkage are analyzed in FinalizeDeclaratorGroup.
 
   // FIXME: The test for external storage here seems wrong? We still
   // need to check for mismatches.
   if (!New->hasExternalStorage() && !New->isFileVarDecl() &&
       // Don't complain about out-of-line definitions of static members.
       !(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);
     }
   }
 
   // C++ doesn't have tentative definitions, so go right ahead and check here.
   const VarDecl *Def;
   if (getLangOpts().CPlusPlus &&
       New->isThisDeclarationADefinition() == VarDecl::Definition &&
       (Def = Old->getDefinition())) {
     Diag(New->getLocation(), diag::err_redefinition) << New;
     Diag(Def->getLocation(), diag::note_previous_definition);
     New->setInvalidDecl();
     return;
   }
 
   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;
   }
 
   // Merge "used" flag.
   if (Old->getMostRecentDecl()->isUsed(false))
     New->setIsUsed();
 

test/Misc/warning-flags.c

(2) It prevents us adding new warnings to Clang that have no -W flag.  All
     new warnings should have -W flags.
 
     If you add a new warning without a flag, this test will fail.  To fix
     this test, simply add a warning group to that warning.
 
 
 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
 CHECK-NEXT:   ext_implicit_lib_function_decl
 CHECK-NEXT:   ext_many_braces_around_scalar_init
 CHECK-NEXT:   ext_missing_declspec
 CHECK-NEXT:   ext_missing_whitespace_after_macro_name
 CHECK-NEXT:   ext_new_paren_array_nonconst

 CHECK-NEXT:   warn_property_attribute
 CHECK-NEXT:   warn_property_getter_owning_mismatch
 CHECK-NEXT:   warn_property_types_are_incompatible
 CHECK-NEXT:   warn_readonly_property
 CHECK-NEXT:   warn_redeclaration_without_attribute_prev_attribute_ignored
 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
 CHECK-NEXT:   warn_typecheck_function_qualifiers
 CHECK-NEXT:   warn_undef_interface
 CHECK-NEXT:   warn_undef_interface_suggest
 CHECK-NEXT:   warn_undef_protocolref
 CHECK-NEXT:   warn_use_out_of_scope_declaration
 CHECK-NEXT:   warn_weak_identifier_undeclared
 CHECK-NEXT:   warn_weak_import
 
 The list of warnings in -Wpedantic should NEVER grow.
 
 CHECK: Number in -Wpedantic (not covered by other -W flags): 28

test/Sema/private-extern.c

 // RUN: %clang_cc1 -verify -fsyntax-only -Wno-private-extern %s
 // RUN: %clang_cc1 -verify -fsyntax-only -Wno-private-extern -fmodules %s
 
 static int g0; // expected-note{{previous definition}}
 int g0; // expected-error{{non-static declaration of 'g0' follows static declaration}}
 
 static int g1;
 extern int g1;
 
 static int g2; 
 __private_extern__ int g2;
 
 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() {
   int g6; // expected-note {{previous}}
   extern int g6; // expected-error {{extern declaration of 'g6' follows non-extern declaration}}
 }
 
 void f1() {
   int g7; // expected-note {{previous}}
   __private_extern__ int g7; // expected-error {{extern declaration of 'g7' follows non-extern declaration}}
 }
 
 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}}
 }
 
 void f4() {
   extern int g10;
   extern int g10;
 }
 

test/Sema/tentative-decls.c

 
 extern const int c[] = {1}; // expected-warning{{'extern' variable has an initializer}}
 const int c[];
 
 int i1 = 1; // expected-note {{previous definition is here}}
 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}}
 int i2 = 3; // expected-error{{non-static declaration of 'i2' follows static declaration}}
 
 static int i3 = 5;
 extern int i3;
 

 int (*pToArray)[];
 int (*pToArray)[8];
 
 int redef[10];
 int redef[];  // expected-note {{previous definition is here}}
 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}}
 }
 
 void func2(void)
 {
   extern double *p;
   extern double *p;
 }
 
 // <rdar://problem/6808352>
 static int a0[];
 static int b0;
 
 static int a0[] = { 4 };
 static int b0 = 5;

test/Sema/thread-specifier.c

#elif !defined(CXX11)
   __thread auto t12a = 0; // expected-error {{'_Thread_local' variables must have global storage}}
   auto __thread t12b = 0; // expected-error {{'_Thread_local' variables must have global storage}}
 #endif
   __thread register int t13a; // expected-error-re {{cannot combine with previous '{{__thread|_Thread_local|thread_local}}' declaration specifier}}
   register __thread int t13b; // expected-error {{cannot combine with previous 'register' declaration specifier}}
 }
 
 __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}}
 
 #ifdef CXX11
 extern thread_local int t17; // expected-note {{previous declaration is here}}
 _Thread_local int t17; // expected-error {{thread-local declaration of 't17' with static initialization follows declaration with dynamic initialization}}
 extern _Thread_local int t18; // expected-note {{previous declaration is here}}

test/Sema/var-redecl.c

 void g18(void) { // expected-note{{'g18' declared here}}
   extern int g19;
 }
 int *p=&g19; // expected-error{{use of undeclared identifier 'g19'}} \
              // expected-warning{{incompatible pointer types}}
 
 // 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}}

test/SemaCXX/MicrosoftExtensions.cpp

 void BB<T>::f(int g = 0) { } // expected-warning {{redefinition of default argument}}
 
 
 
 extern void static_func();
 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);
 
 void function_to_voidptr_conv() {
    void *a1 = function_prototype;
    void *a2 = &function_prototype;
    void *a3 = function_ptr;
 }