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

Richard Smith richard at metafoo.co.uk
Mon Sep 29 12:10:59 PDT 2014 

On Thu, Sep 25, 2014 at 10:27 AM, Hans Wennborg <hans at chromium.org> wrote:

> On Fri, Sep 19, 2014 at 7:06 AM, Tom Stellard <tom at stellard.net> wrote:
> > On Thu, Sep 18, 2014 at 10:11:51AM -0700, Hans Wennborg wrote:
> >> On Thu, Sep 4, 2014 at 10:36 AM, Hans Wennborg <hans at chromium.org>
> wrote:
> >> > On Thu, Sep 4, 2014 at 10:33 AM, Reid Kleckner <rnk at google.com>
> wrote:
> >> >> Can we merge this to 3.5, or are we done merging patches?
> >> >
> >> > 3.5-final was tagged a few days ago. I'd be fine with merging this for
> >> > 3.5.1, though.
> >>
> >> Tom, may I merge this to the branch?
> >>
> >
> > Yes, as long as Richard approves.
>
> Richard: Ping?


Yes, seems fine.


> >> This was discussed on the bug too:
> >> http://llvm.org/bugs/show_bug.cgi?id=20746#c12
> >>
> >> Thanks,
> >> Hans
> >>
> >>
> >> >> On Wed, Aug 27, 2014 at 2:27 PM, Hans Wennborg <hans at hanshq.net>
> wrote:
> >> >>>
> >> >>> Author: hans
> >> >>> Date: Wed Aug 27 16:27:40 2014
> >> >>> New Revision: 216619
> >> >>>
> >> >>> URL: http://llvm.org/viewvc/llvm-project?rev=216619&view=rev
> >> >>> Log:
> >> >>> Allow adding dll attributes on certain redecls with a warning if
> the decl
> >> >>> hasn't been used yet (PR20746)
> >> >>>
> >> >>> This shouldn't really be allowed, but it comes up in real code (see
> PR).
> >> >>> As
> >> >>> long as the decl hasn't been used there's no technical difficulty in
> >> >>> supporting
> >> >>> it, so downgrade the error to a warning.
> >> >>>
> >> >>> Differential Revision: http://reviews.llvm.org/D5087
> >> >>>
> >> >>> Modified:
> >> >>>     cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >> >>>     cfe/trunk/lib/Sema/SemaDecl.cpp
> >> >>>     cfe/trunk/test/Sema/dllexport.c
> >> >>>     cfe/trunk/test/Sema/dllimport.c
> >> >>>     cfe/trunk/test/SemaCXX/dllexport.cpp
> >> >>>     cfe/trunk/test/SemaCXX/dllimport.cpp
> >> >>>
> >> >>> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> >> >>> URL:
> >> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=216619&r1=216618&r2=216619&view=diff
> >> >>>
> >> >>>
> ==============================================================================
> >> >>> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> >> >>> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Aug 27
> >> >>> 16:27:40 2014
> >> >>> @@ -2133,6 +2133,9 @@ def warn_attribute_invalid_on_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<
> >> >>>
> >> >>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> >> >>> URL:
> >> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=216619&r1=216618&r2=216619&view=diff
> >> >>>
> >> >>>
> ==============================================================================
> >> >>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> >> >>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Wed Aug 27 16:27:40 2014
> >> >>> @@ -5075,7 +5075,7 @@ static void checkDLLAttributeRedeclarati
> >> >>>      NewDecl = NewTD->getTemplatedDecl();
> >> >>>
> >> >>>    if (!OldDecl || !NewDecl)
> >> >>> -      return;
> >> >>> +    return;
> >> >>>
> >> >>>    const DLLImportAttr *OldImportAttr =
> OldDecl->getAttr<DLLImportAttr>();
> >> >>>    const DLLExportAttr *OldExportAttr =
> OldDecl->getAttr<DLLExportAttr>();
> >> >>> @@ -5092,13 +5092,30 @@ static void checkDLLAttributeRedeclarati
> >> >>>    // 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)
> >> >>> -      << NewDecl
> >> >>> -      << (NewImportAttr ? (const Attr *)NewImportAttr :
> NewExportAttr);
> >> >>> +    // 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);
> >> >>> -    NewDecl->setInvalidDecl();
> >> >>> -    return;
> >> >>> +    if (!JustWarn) {
> >> >>> +      NewDecl->setInvalidDecl();
> >> >>> +      return;
> >> >>> +    }
> >> >>>    }
> >> >>>
> >> >>>    // A redeclaration is not allowed to drop a dllimport attribute,
> the
> >> >>> only
> >> >>>
> >> >>> Modified: cfe/trunk/test/Sema/dllexport.c
> >> >>> URL:
> >> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/dllexport.c?rev=216619&r1=216618&r2=216619&view=diff
> >> >>>
> >> >>>
> ==============================================================================
> >> >>> --- cfe/trunk/test/Sema/dllexport.c (original)
> >> >>> +++ cfe/trunk/test/Sema/dllexport.c Wed Aug 27 16:27:40 2014
> >> >>> @@ -39,8 +39,13 @@ __declspec(dllexport) extern int GlobalR
> >> >>>                               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'}}
> >> >>>
> >> >>> @@ -86,11 +91,18 @@ __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'}}
> >> >>>
> >> >>>
> >> >>> Modified: cfe/trunk/test/Sema/dllimport.c
> >> >>> URL:
> >> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/dllimport.c?rev=216619&r1=216618&r2=216619&view=diff
> >> >>>
> >> >>>
> ==============================================================================
> >> >>> --- cfe/trunk/test/Sema/dllimport.c (original)
> >> >>> +++ cfe/trunk/test/Sema/dllimport.c Wed Aug 27 16:27:40 2014
> >> >>> @@ -64,9 +64,16 @@ int GlobalRedecl2c __attribute__((dllimp
> >> >>>  __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'}}
> >> >>>
> >> >>> @@ -128,14 +135,20 @@ __declspec(dllimport) void redecl3(); //
> >> >>>                        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'}}
> >> >>>
> >> >>> Modified: cfe/trunk/test/SemaCXX/dllexport.cpp
> >> >>> URL:
> >> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/dllexport.cpp?rev=216619&r1=216618&r2=216619&view=diff
> >> >>>
> >> >>>
> ==============================================================================
> >> >>> --- cfe/trunk/test/SemaCXX/dllexport.cpp (original)
> >> >>> +++ cfe/trunk/test/SemaCXX/dllexport.cpp Wed Aug 27 16:27:40 2014
> >> >>> @@ -53,7 +53,7 @@ __declspec(dllexport) extern int GlobalR
> >> >>>                               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'}}
> >> >>> @@ -186,10 +186,10 @@ __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 {
> >> >>> @@ -200,8 +200,8 @@ struct FuncFriend {
> >> >>>  };
> >> >>>  __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);
> >> >>>
> >> >>> Modified: cfe/trunk/test/SemaCXX/dllimport.cpp
> >> >>> URL:
> >> >>>
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/dllimport.cpp?rev=216619&r1=216618&r2=216619&view=diff
> >> >>>
> >> >>>
> ==============================================================================
> >> >>> --- cfe/trunk/test/SemaCXX/dllimport.cpp (original)
> >> >>> +++ cfe/trunk/test/SemaCXX/dllimport.cpp Wed Aug 27 16:27:40 2014
> >> >>> @@ -75,7 +75,7 @@ __declspec(dllimport) extern int GlobalR
> >> >>>                        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'}}
> >> >>> @@ -224,10 +224,10 @@ __declspec(dllimport) void redecl3(); //
> >> >>>                        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 {
> >> >>> @@ -240,8 +240,8 @@ struct FuncFriend {
> >> >>>  __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();
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140929/78da4bec/attachment.html>

More information about the cfe-commits mailing list