Allow adding dll attributes on certain redecls with a warning if the decl hasn't been used yet (PR20746)

[hans]

Summary

This shouldn't really be allowed, but it comes up in real code (see PR20746), and as long as the decl hasn't been used there's no technical difficulty in supporting it, so we can just downgrade the error to a warning.

[rnk]

lgtm

[hans]

Closed by commit rL216619 (authored by @hans).

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def err_declspec_thread_on_thread_variable : Error<
   "thread-local storage specifier">;
 def err_attribute_dll_not_extern : Error<
   "%q0 must have external linkage when declared %q1">;
 def warn_attribute_invalid_on_definition : Warning<
   "'%0' attribute cannot be specified on a definition">,
   InGroup<IgnoredAttributes>;
 def err_attribute_dll_redeclaration : Error<
   "redeclaration of %q0 cannot add %q1 attribute">;
+def warn_attribute_dll_redeclaration : Warning<
+  "redeclaration of %q0 should not add %q1 attribute">,
+  InGroup<DiagGroup<"dll-attribute-on-redeclaration">>;
 def err_attribute_dllimport_function_definition : Error<
   "dllimport cannot be applied to non-inline function definition">;
 def err_attribute_dll_deleted : Error<
   "attribute %q0 cannot be applied to a deleted function">;
 def err_attribute_dllimport_data_definition : Error<
   "definition of dllimport data">;
 def err_attribute_dllimport_static_field_definition : Error<
   "definition of dllimport static field not allowed">;

cfe/trunk/lib/Sema/SemaDecl.cpp

static void checkDLLAttributeRedeclaration(Sema &S, NamedDecl *OldDecl,
                                            NamedDecl *NewDecl,
                                            bool IsSpecialization) {
   if (TemplateDecl *OldTD = dyn_cast<TemplateDecl>(OldDecl))
     OldDecl = OldTD->getTemplatedDecl();
   if (TemplateDecl *NewTD = dyn_cast<TemplateDecl>(NewDecl))
     NewDecl = NewTD->getTemplatedDecl();
 
   if (!OldDecl || !NewDecl)
-      return;
 
   const DLLImportAttr *OldImportAttr = OldDecl->getAttr<DLLImportAttr>();
   const DLLExportAttr *OldExportAttr = OldDecl->getAttr<DLLExportAttr>();
   const DLLImportAttr *NewImportAttr = NewDecl->getAttr<DLLImportAttr>();
   const DLLExportAttr *NewExportAttr = NewDecl->getAttr<DLLExportAttr>();
 
   // dllimport and dllexport are inheritable attributes so we have to exclude
   // inherited attribute instances.
   bool HasNewAttr = (NewImportAttr && !NewImportAttr->isInherited()) ||
                     (NewExportAttr && !NewExportAttr->isInherited());
 
   // A redeclaration is not allowed to add a dllimport or dllexport attribute,
   // the only exception being explicit specializations.
   // Implicitly generated declarations are also excluded for now because there
   // is no other way to switch these to use dllimport or dllexport.
   bool AddsAttr = !(OldImportAttr || OldExportAttr) && HasNewAttr;
+
   if (AddsAttr && !IsSpecialization && !OldDecl->isImplicit()) {
-    S.Diag(NewDecl->getLocation(), diag::err_attribute_dll_redeclaration)
+    // If the declaration hasn't been used yet, allow with a warning for
+    // free functions and global variables.
+    bool JustWarn = false;
+    if (!OldDecl->isUsed() && OldDecl->getDeclContext()->isFileContext()) {
+      auto *VD = dyn_cast<VarDecl>(OldDecl);
+      if (VD && !VD->getDescribedVarTemplate())
+        JustWarn = true;
+      auto *FD = dyn_cast<FunctionDecl>(OldDecl);
+      if (FD && FD->getTemplatedKind() == FunctionDecl::TK_NonTemplate)
+        JustWarn = true;
+    }
+
+    unsigned DiagID = JustWarn ? diag::warn_attribute_dll_redeclaration
+                               : diag::err_attribute_dll_redeclaration;
+    S.Diag(NewDecl->getLocation(), DiagID)
         << NewDecl
         << (NewImportAttr ? (const Attr *)NewImportAttr : NewExportAttr);
     S.Diag(OldDecl->getLocation(), diag::note_previous_declaration);
+    if (!JustWarn) {
       NewDecl->setInvalidDecl();
       return;
     }
+  }
 
   // A redeclaration is not allowed to drop a dllimport attribute, the only
   // exceptions being inline function definitions, local extern declarations,
   // and qualified friend declarations.
   // NB: MSVC converts such a declaration to dllexport.
   bool IsInline = false, IsStaticDataMember = false, IsQualifiedFriend = false;
   if (const auto *VD = dyn_cast<VarDecl>(NewDecl))
     // Ignore static data because out-of-line definitions are diagnosed

cfe/trunk/test/Sema/dllexport.c

 // Redeclarations
 __declspec(dllexport) extern int GlobalRedecl1;
 __declspec(dllexport)        int GlobalRedecl1;
 
 __declspec(dllexport) extern int GlobalRedecl2;
                              int GlobalRedecl2;
 
                       extern int GlobalRedecl3; // expected-note{{previous declaration is here}}
+int useGlobalRedecl3() { return GlobalRedecl3; }
 __declspec(dllexport) extern int GlobalRedecl3; // expected-error{{redeclaration of 'GlobalRedecl3' cannot add 'dllexport' attribute}}
 
+                      extern int GlobalRedecl4; // expected-note{{previous declaration is here}}
+__declspec(dllexport) extern int GlobalRedecl4; // expected-warning{{redeclaration of 'GlobalRedecl4' should not add 'dllexport' attribute}}
+
+
 // External linkage is required.
 __declspec(dllexport) static int StaticGlobal; // expected-error{{'StaticGlobal' must have external linkage when declared 'dllexport'}}
 
 // Export in local scope.
 void functionScope() {
   __declspec(dllexport)        int LocalVarDecl; // expected-error{{'LocalVarDecl' must have external linkage when declared 'dllexport'}}
   __declspec(dllexport)        int LocalVarDef = 1; // expected-error{{'LocalVarDef' must have external linkage when declared 'dllexport'}}
   __declspec(dllexport) extern int ExternLocalVarDecl;

 
 __declspec(dllexport) void redecl2();
                       void redecl2();
 
 __declspec(dllexport) void redecl3();
                       void redecl3() {}
 
                       void redecl4(); // expected-note{{previous declaration is here}}
+void useRedecl4() { redecl4(); }
 __declspec(dllexport) void redecl4(); // expected-error{{redeclaration of 'redecl4' cannot add 'dllexport' attribute}}
 
                       void redecl5(); // expected-note{{previous declaration is here}}
+void useRedecl5() { redecl5(); }
 __declspec(dllexport) inline void redecl5() {} // expected-error{{redeclaration of 'redecl5' cannot add 'dllexport' attribute}}
 
+// Allow with a warning if the decl hasn't been used yet.
+                      void redecl6(); // expected-note{{previous declaration is here}}
+__declspec(dllexport) void redecl6(); // expected-warning{{redeclaration of 'redecl6' should not add 'dllexport' attribute}}
+
+
 // External linkage is required.
 __declspec(dllexport) static int staticFunc(); // expected-error{{'staticFunc' must have external linkage when declared 'dllexport'}}
 
 
 
 //===----------------------------------------------------------------------===//
 // Precedence
 //===----------------------------------------------------------------------===//

cfe/trunk/test/Sema/dllimport.c

 int GlobalRedecl2c __attribute__((dllimport));
 int GlobalRedecl2c __attribute__((dllimport));
 
 // NB: MSVC issues a warning and makes GlobalRedecl3 dllexport. We follow GCC
 // and drop the dllimport with a warning.
 __declspec(dllimport) extern int GlobalRedecl3; // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
                       extern int GlobalRedecl3; // expected-warning{{'GlobalRedecl3' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
 
+// Adding an attribute on redeclaration.
                       extern int GlobalRedecl4; // expected-note{{previous declaration is here}}
+int useGlobalRedecl4() { return GlobalRedecl4; }
 __declspec(dllimport) extern int GlobalRedecl4; // expected-error{{redeclaration of 'GlobalRedecl4' cannot add 'dllimport' attribute}}
 
+// Allow with a warning if the decl hasn't been used yet.
+                      extern int GlobalRedecl5; // expected-note{{previous declaration is here}}
+__declspec(dllimport) extern int GlobalRedecl5; // expected-warning{{redeclaration of 'GlobalRedecl5' should not add 'dllimport' attribute}}
+
+
 // External linkage is required.
 __declspec(dllimport) static int StaticGlobal; // expected-error{{'StaticGlobal' must have external linkage when declared 'dllimport'}}
 
 // Import in local scope.
 __declspec(dllimport) float LocalRedecl1; // expected-note{{previous definition is here}}
 __declspec(dllimport) float LocalRedecl2; // expected-note{{previous definition is here}}
 __declspec(dllimport) float LocalRedecl3; // expected-note{{previous definition is here}}
 __declspec(dllimport) float LocalRedecl4;

 // and drop the dllimport with a warning.
 __declspec(dllimport) void redecl2(); // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
                       void redecl2(); // expected-warning{{'redecl2' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
 
 __declspec(dllimport) void redecl3(); // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
                       void redecl3() {} // expected-warning{{'redecl3' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
 
                       void redecl4(); // expected-note{{previous declaration is here}}
+void useRedecl4() { redecl4(); }
 __declspec(dllimport) void redecl4(); // expected-error{{redeclaration of 'redecl4' cannot add 'dllimport' attribute}}
 
+// Allow with a warning if the decl hasn't been used yet.
+                      void redecl5(); // expected-note{{previous declaration is here}}
+__declspec(dllimport) void redecl5(); // expected-warning{{redeclaration of 'redecl5' should not add 'dllimport' attribute}}
+
+
 // Inline redeclarations are fine.
-__declspec(dllimport) void redecl5();
-                      inline void redecl5() {}
+__declspec(dllimport) void redecl6();
+                      inline void redecl6() {}
 
-                      void redecl6(); // expected-note{{previous declaration is here}}
-__declspec(dllimport) inline void redecl6() {} // expected-error{{redeclaration of 'redecl6' cannot add 'dllimport' attribute}}
+                      void redecl7(); // expected-note{{previous declaration is here}}
+__declspec(dllimport) inline void redecl7() {} // expected-warning{{redeclaration of 'redecl7' should not add 'dllimport' attribute}}
 
 // External linkage is required.
 __declspec(dllimport) static int staticFunc(); // expected-error{{'staticFunc' must have external linkage when declared 'dllimport'}}

cfe/trunk/test/SemaCXX/dllexport.cpp

 // Redeclarations
 __declspec(dllexport) extern int GlobalRedecl1;
 __declspec(dllexport)        int GlobalRedecl1;
 
 __declspec(dllexport) extern int GlobalRedecl2;
                              int GlobalRedecl2;
 
                       extern int GlobalRedecl3; // expected-note{{previous declaration is here}}
-__declspec(dllexport) extern int GlobalRedecl3; // expected-error{{redeclaration of 'GlobalRedecl3' cannot add 'dllexport' attribute}}
+__declspec(dllexport) extern int GlobalRedecl3; // expected-warning{{redeclaration of 'GlobalRedecl3' should not add 'dllexport' attribute}}
 
 // External linkage is required.
 __declspec(dllexport) static int StaticGlobal; // expected-error{{'StaticGlobal' must have external linkage when declared 'dllexport'}}
 __declspec(dllexport) Internal InternalTypeGlobal; // expected-error{{'InternalTypeGlobal' must have external linkage when declared 'dllexport'}}
 namespace    { __declspec(dllexport) int InternalGlobal; } // expected-error{{'(anonymous namespace)::InternalGlobal' must have external linkage when declared 'dllexport'}}
 namespace ns { __declspec(dllexport) int ExternalGlobal; }
 
 __declspec(dllexport) auto InternalAutoTypeGlobal = Internal(); // expected-error{{'InternalAutoTypeGlobal' must have external linkage when declared 'dllexport'}}

 // Redeclarations
 __declspec(dllexport) void redecl1();
 __declspec(dllexport) void redecl1() {}
 
 __declspec(dllexport) void redecl2();
                       void redecl2() {}
 
                       void redecl3(); // expected-note{{previous declaration is here}}
-__declspec(dllexport) void redecl3(); // expected-error{{redeclaration of 'redecl3' cannot add 'dllexport' attribute}}
+__declspec(dllexport) void redecl3(); // expected-warning{{redeclaration of 'redecl3' should not add 'dllexport' attribute}}
 
                       void redecl4(); // expected-note{{previous declaration is here}}
-__declspec(dllexport) inline void redecl4() {} // expected-error{{redeclaration of 'redecl4' cannot add 'dllexport' attribute}}
+__declspec(dllexport) inline void redecl4() {} // expected-warning{{redeclaration of 'redecl4' should not add 'dllexport' attribute}}
 
 // Friend functions
 struct FuncFriend {
   friend __declspec(dllexport) void friend1();
   friend __declspec(dllexport) void friend2();
   friend                       void friend3(); // expected-note{{previous declaration is here}}
   friend                       void friend4(); // expected-note{{previous declaration is here}}
 };
 __declspec(dllexport) void friend1() {}
                       void friend2() {}
-__declspec(dllexport) void friend3() {} // expected-error{{redeclaration of 'friend3' cannot add 'dllexport' attribute}}
-__declspec(dllexport) inline void friend4() {} // expected-error{{redeclaration of 'friend4' cannot add 'dllexport' attribute}}
+__declspec(dllexport) void friend3() {} // expected-warning{{redeclaration of 'friend3' should not add 'dllexport' attribute}}
+__declspec(dllexport) inline void friend4() {} // expected-warning{{redeclaration of 'friend4' should not add 'dllexport' attribute}}
 
 // Implicit declarations can be redeclared with dllexport.
 __declspec(dllexport) void* operator new(__SIZE_TYPE__ n);
 
 // External linkage is required.
 __declspec(dllexport) static int staticFunc(); // expected-error{{'staticFunc' must have external linkage when declared 'dllexport'}}
 __declspec(dllexport) Internal internalRetFunc(); // expected-error{{'internalRetFunc' must have external linkage when declared 'dllexport'}}
 namespace    { __declspec(dllexport) void internalFunc() {} } // expected-error{{'(anonymous namespace)::internalFunc' must have external linkage when declared 'dllexport'}}

cfe/trunk/test/SemaCXX/dllimport.cpp

 int GlobalRedecl2c __attribute__((dllimport));
 
 // NB: MSVC issues a warning and makes GlobalRedecl3 dllexport. We follow GCC
 // and drop the dllimport with a warning.
 __declspec(dllimport) extern int GlobalRedecl3; // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
                       extern int GlobalRedecl3; // expected-warning{{'GlobalRedecl3' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
 
                       extern int GlobalRedecl4; // expected-note{{previous declaration is here}}
-__declspec(dllimport) extern int GlobalRedecl4; // expected-error{{redeclaration of 'GlobalRedecl4' cannot add 'dllimport' attribute}}
+__declspec(dllimport) extern int GlobalRedecl4; // expected-warning{{redeclaration of 'GlobalRedecl4' should not add 'dllimport' attribute}}
 
 // External linkage is required.
 __declspec(dllimport) static int StaticGlobal; // expected-error{{'StaticGlobal' must have external linkage when declared 'dllimport'}}
 __declspec(dllimport) Internal InternalTypeGlobal; // expected-error{{'InternalTypeGlobal' must have external linkage when declared 'dllimport'}}
 namespace    { __declspec(dllimport) int InternalGlobal; } // expected-error{{'(anonymous namespace)::InternalGlobal' must have external linkage when declared 'dllimport'}}
 namespace ns { __declspec(dllimport) int ExternalGlobal; }
 
 __declspec(dllimport) auto InternalAutoTypeGlobal = Internal(); // expected-error{{'InternalAutoTypeGlobal' must have external linkage when declared 'dllimport'}}

 // and drop the dllimport with a warning.
 __declspec(dllimport) void redecl2(); // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
                       void redecl2(); // expected-warning{{'redecl2' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
 
 __declspec(dllimport) void redecl3(); // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
                       void redecl3() {} // expected-warning{{'redecl3' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
 
                       void redecl4(); // expected-note{{previous declaration is here}}
-__declspec(dllimport) void redecl4(); // expected-error{{redeclaration of 'redecl4' cannot add 'dllimport' attribute}}
+__declspec(dllimport) void redecl4(); // expected-warning{{redeclaration of 'redecl4' should not add 'dllimport' attribute}}
 
                       void redecl5(); // expected-note{{previous declaration is here}}
-__declspec(dllimport) inline void redecl5() {} // expected-error{{redeclaration of 'redecl5' cannot add 'dllimport' attribute}}
+__declspec(dllimport) inline void redecl5() {} // expected-warning{{redeclaration of 'redecl5' should not add 'dllimport' attribute}}
 
 // Friend functions
 struct FuncFriend {
   friend __declspec(dllimport) void friend1();
   friend __declspec(dllimport) void friend2(); // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
   friend __declspec(dllimport) void friend3(); // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
   friend                       void friend4(); // expected-note{{previous declaration is here}}
   friend                       void friend5(); // expected-note{{previous declaration is here}}
 };
 __declspec(dllimport) void friend1();
                       void friend2(); // expected-warning{{'friend2' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
                       void friend3() {} // expected-warning{{'friend3' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
-__declspec(dllimport) void friend4(); // expected-error{{redeclaration of 'friend4' cannot add 'dllimport' attribute}}
-__declspec(dllimport) inline void friend5() {} // expected-error{{redeclaration of 'friend5' cannot add 'dllimport' attribute}}
+__declspec(dllimport) void friend4(); // expected-warning{{redeclaration of 'friend4' should not add 'dllimport' attribute}}
+__declspec(dllimport) inline void friend5() {} // expected-warning{{redeclaration of 'friend5' should not add 'dllimport' attribute}}
 
 void __declspec(dllimport) friend6(); // expected-note{{previous declaration is here}} expected-note{{previous attribute is here}}
 void __declspec(dllimport) friend7();
 struct FuncFriend2 {
   friend void friend6(); // expected-warning{{'friend6' redeclared without 'dllimport' attribute: previous 'dllimport' ignored}}
   friend void ::friend7();
 };