RE: r324991 - Fix for PR32992. Static const classes not exported.

2018-02-20 Thread Ammarguellat, Zahira via cfe-commits
Hello,

Please see https://reviews.llvm.org/D42968
For the fix of the assertion belwo.
Thanks.

-Original Message-
From: hwennb...@google.com [mailto:hwennb...@google.com] On Behalf Of Hans 
Wennborg
Sent: Tuesday, February 20, 2018 4:04 AM
To: Ammarguellat, Zahira <zahira.ammarguel...@intel.com>
Cc: cfe-commits <cfe-commits@lists.llvm.org>
Subject: Re: r324991 - Fix for PR32992. Static const classes not exported.

The problem is that your patch caused the reproducer to trigger an assert, 
which it didn't do before, causing our builds to break.

Your patch seems like it does fix the PR, but it can't break currently working 
builds.

$ build.release/bin/clang -cc1 -triple i386-pc-windows-msvc19.11.0 -emit-pch 
-fms-extensions -fms-compatibility
-fms-compatibility-version=19.11 -std=c++14 -fdelayed-template-parsing -x 
c++-header /tmp/a.cc -o /dev/null
clang: 
/work/llvm.combined/llvm/tools/clang/lib/Serialization/ASTWriter.cpp:4723:
clang::ASTFileSignature clang::ASTWriter::WriteASTCore(clang::Sema&,
llvm::StringRef, const string&, clang::Module*): Assertion
`SemaRef.PendingLocalImplicitInstantiations.empty() && "There are local ones at 
end of translation unit!"' failed.
build.release/bin/clang(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x55c535576aaa]
build.release/bin/clang(_ZN4llvm3sys17RunSignalHandlersEv+0x3e)[0x55c53557492e]
build.release/bin/clang(+0x2424a7c)[0x55c535574a7c]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x110c0)[0x7f4aeeb9d0c0]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcf)[0x7f4aed732fcf]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x16a)[0x7f4aed7343fa]
/lib/x86_64-linux-gnu/libc.so.6(+0x2be37)[0x7f4aed72be37]
/lib/x86_64-linux-gnu/libc.so.6(+0x2bee2)[0x7f4aed72bee2]
build.release/bin/clang(_ZN5clang9ASTWriter12WriteASTCoreERNS_4SemaEN4llvm9StringRefERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPNS_6ModuleE+0x29fa)[0x55c536444cea]
build.release/bin/clang(_ZN5clang9ASTWriter8WriteASTERNS_4SemaERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPNS_6ModuleEN4llvm9StringRefEb+0xad)[0x55c536444dfd]
build.release/bin/clang(_ZN5clang12PCHGenerator21HandleTranslationUnitERNS_10ASTContextE+0x80)[0x55c536465fe0]
build.release/bin/clang(_ZN5clang17MultiplexConsumer21HandleTranslationUnitERNS_10ASTContextE+0x28)[0x55c535b53488]
build.release/bin/clang(_ZN5clang8ParseASTERNS_4SemaEbb+0x350)[0x55c5362c5d30]
build.release/bin/clang(_ZN5clang14FrontendAction7ExecuteEv+0x11e)[0x55c535b2958e]
build.release/bin/clang(_ZN5clang16CompilerInstance13ExecuteActionERNS_14FrontendActionE+0x176)[0x55c535af54d6]
build.release/bin/clang(_ZN5clang25ExecuteCompilerInvocationEPNS_16CompilerInstanceE+0x981)[0x55c535bc2561]
build.release/bin/clang(_Z8cc1_mainN4llvm8ArrayRefIPKcEES2_Pv+0xc50)[0x55c533e1a890]
build.release/bin/clang(main+0x1295)[0x55c533d9b585]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7f4aed7202b1]
build.release/bin/clang(_start+0x2a)[0x55c533e1684a]
Stack dump:
0.  Program arguments: build.release/bin/clang -cc1 -triple
i386-pc-windows-msvc19.11.0 -emit-pch -fms-extensions -fms-compatibility 
-fms-compatibility-version=19.11 -std=c++14 -fdelayed-template-parsing -x 
c++-header /tmp/a.cc -o /dev/null
1.   parser at end of file
Aborted

On Mon, Feb 19, 2018 at 10:18 PM, Ammarguellat, Zahira 
<zahira.ammarguel...@intel.com> wrote:
> Hans,
>
>
>
> The reproducer below generates wrong export symbols compared to MSVC 
> but I am not sure it is related to my change.
>
> Compiling this with MSVC generates these symbols:
>
>
>
> ksh-3.2$ dumpbin /directives t3.obj | grep EXPORT
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@$$QEAV0@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@AEBV01@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@$$QEAV01@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z
>
>
>
>
>
> Compiling with clang (my patches no included) generate these symbols:
>
> ksh-3.2$ dumpbin /directives t3.o | grep EXPORT
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@$$QEAV0@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@AEBV01@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@$$QEAV01@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z
>
>/EXPORT:?e@?$d@H@@0W4b@a@@B,DATA   ç This is not generated in MSVC.
>
>
>
> Although this is a bug that needs to be fixes, it is in my opinion 
> un-related to the patches I have proposed.
>
>
>
> Your thoughts?
>
>
>
> Thanks.
>
> -Zahira
>
>
>
>
>
> -Original Message-
> From: hwennb...@google.com [mailto:hwennb...@google.com] On Behalf Of 
> Hans Wennborg
> Sent: Monday, February 19, 2018 5:11 AM
> To: cfe-commits <cfe-commits@lists.llvm.org>; Am

Re: r324991 - Fix for PR32992. Static const classes not exported.

2018-02-20 Thread Hans Wennborg via cfe-commits
The problem is that your patch caused the reproducer to trigger an
assert, which it didn't do before, causing our builds to break.

Your patch seems like it does fix the PR, but it can't break currently
working builds.

$ build.release/bin/clang -cc1 -triple i386-pc-windows-msvc19.11.0
-emit-pch -fms-extensions -fms-compatibility
-fms-compatibility-version=19.11 -std=c++14 -fdelayed-template-parsing
-x c++-header /tmp/a.cc -o /dev/null
clang: 
/work/llvm.combined/llvm/tools/clang/lib/Serialization/ASTWriter.cpp:4723:
clang::ASTFileSignature clang::ASTWriter::WriteASTCore(clang::Sema&,
llvm::StringRef, const string&, clang::Module*): Assertion
`SemaRef.PendingLocalImplicitInstantiations.empty() && "There are
local ones at end of translation unit!"' failed.
build.release/bin/clang(_ZN4llvm3sys15PrintStackTraceERNS_11raw_ostreamE+0x1a)[0x55c535576aaa]
build.release/bin/clang(_ZN4llvm3sys17RunSignalHandlersEv+0x3e)[0x55c53557492e]
build.release/bin/clang(+0x2424a7c)[0x55c535574a7c]
/lib/x86_64-linux-gnu/libpthread.so.0(+0x110c0)[0x7f4aeeb9d0c0]
/lib/x86_64-linux-gnu/libc.so.6(gsignal+0xcf)[0x7f4aed732fcf]
/lib/x86_64-linux-gnu/libc.so.6(abort+0x16a)[0x7f4aed7343fa]
/lib/x86_64-linux-gnu/libc.so.6(+0x2be37)[0x7f4aed72be37]
/lib/x86_64-linux-gnu/libc.so.6(+0x2bee2)[0x7f4aed72bee2]
build.release/bin/clang(_ZN5clang9ASTWriter12WriteASTCoreERNS_4SemaEN4llvm9StringRefERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPNS_6ModuleE+0x29fa)[0x55c536444cea]
build.release/bin/clang(_ZN5clang9ASTWriter8WriteASTERNS_4SemaERKNSt7__cxx1112basic_stringIcSt11char_traitsIcESaIcEEEPNS_6ModuleEN4llvm9StringRefEb+0xad)[0x55c536444dfd]
build.release/bin/clang(_ZN5clang12PCHGenerator21HandleTranslationUnitERNS_10ASTContextE+0x80)[0x55c536465fe0]
build.release/bin/clang(_ZN5clang17MultiplexConsumer21HandleTranslationUnitERNS_10ASTContextE+0x28)[0x55c535b53488]
build.release/bin/clang(_ZN5clang8ParseASTERNS_4SemaEbb+0x350)[0x55c5362c5d30]
build.release/bin/clang(_ZN5clang14FrontendAction7ExecuteEv+0x11e)[0x55c535b2958e]
build.release/bin/clang(_ZN5clang16CompilerInstance13ExecuteActionERNS_14FrontendActionE+0x176)[0x55c535af54d6]
build.release/bin/clang(_ZN5clang25ExecuteCompilerInvocationEPNS_16CompilerInstanceE+0x981)[0x55c535bc2561]
build.release/bin/clang(_Z8cc1_mainN4llvm8ArrayRefIPKcEES2_Pv+0xc50)[0x55c533e1a890]
build.release/bin/clang(main+0x1295)[0x55c533d9b585]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0xf1)[0x7f4aed7202b1]
build.release/bin/clang(_start+0x2a)[0x55c533e1684a]
Stack dump:
0.  Program arguments: build.release/bin/clang -cc1 -triple
i386-pc-windows-msvc19.11.0 -emit-pch -fms-extensions
-fms-compatibility -fms-compatibility-version=19.11 -std=c++14
-fdelayed-template-parsing -x c++-header /tmp/a.cc -o /dev/null
1.   parser at end of file
Aborted

On Mon, Feb 19, 2018 at 10:18 PM, Ammarguellat, Zahira
<zahira.ammarguel...@intel.com> wrote:
> Hans,
>
>
>
> The reproducer below generates wrong export symbols compared to MSVC but I
> am not sure it is related to my change.
>
> Compiling this with MSVC generates these symbols:
>
>
>
> ksh-3.2$ dumpbin /directives t3.obj | grep EXPORT
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@$$QEAV0@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@AEBV01@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@$$QEAV01@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z
>
>
>
>
>
> Compiling with clang (my patches no included) generate these symbols:
>
> ksh-3.2$ dumpbin /directives t3.o | grep EXPORT
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4?$d@H@@QEAAAEAV0@$$QEAV0@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@AEBV01@@Z
>
>/EXPORT:??4?$h@H@f@@QEAAAEAV01@$$QEAV01@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@AEBV0@@Z
>
>/EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z
>
>/EXPORT:?e@?$d@H@@0W4b@a@@B,DATA   ç This is not generated in MSVC.
>
>
>
> Although this is a bug that needs to be fixes, it is in my opinion
> un-related to the patches I have proposed.
>
>
>
> Your thoughts?
>
>
>
> Thanks.
>
> -Zahira
>
>
>
>
>
> -Original Message-
> From: hwennb...@google.com [mailto:hwennb...@google.com] On Behalf Of Hans
> Wennborg
> Sent: Monday, February 19, 2018 5:11 AM
> To: cfe-commits <cfe-commits@lists.llvm.org>; Ammarguellat, Zahira
> <zahira.ammarguel...@intel.com>
> Subject: Re: r324991 - Fix for PR32992. Static const classes not exported.
>
>
>
> Reduced repro:
>
>
>
> $ clang -cc1 -triple i386-pc-windows-msvc19.11.0 -emit-pch -fms-extensions
> -fms-compatibility -fms-compatibility-version=19.11
>
> -std=c++14 -fdelayed-template-parsing -x c++-header a.ii -o /dev/null
>
>
>
> a.ii:
>
>

RE: r324991 - Fix for PR32992. Static const classes not exported.

2018-02-19 Thread Ammarguellat, Zahira via cfe-commits
Hans,



The reproducer below generates wrong export symbols compared to MSVC but I am 
not sure it is related to my change.

Compiling this with MSVC generates these symbols:



ksh-3.2$ dumpbin /directives t3.obj | grep EXPORT

   /EXPORT:??4?$d@H@@QEAAAEAV0@AEBV0@@Z

   /EXPORT:??4?$d@H@@QEAAAEAV0@$$QEAV0@@Z

   /EXPORT:??4?$h@H@f@@QEAAAEAV01@AEBV01@@Z

   /EXPORT:??4?$h@H@f@@QEAAAEAV01@$$QEAV01@@Z

   /EXPORT:??4i@@QEAAAEAV0@AEBV0@@Z

   /EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z





Compiling with clang (my patches no included) generate these symbols:

ksh-3.2$ dumpbin /directives t3.o | grep EXPORT

   /EXPORT:??4?$d@H@@QEAAAEAV0@AEBV0@@Z

   /EXPORT:??4?$d@H@@QEAAAEAV0@$$QEAV0@@Z

   /EXPORT:??4?$h@H@f@@QEAAAEAV01@AEBV01@@Z

   /EXPORT:??4?$h@H@f@@QEAAAEAV01@$$QEAV01@@Z

   /EXPORT:??4i@@QEAAAEAV0@AEBV0@@Z

   /EXPORT:??4i@@QEAAAEAV0@$$QEAV0@@Z

   /EXPORT:?e@?$d@H@@0W4b@a@@B,DATA   <== This is not generated in MSVC.



Although this is a bug that needs to be fixes, it is in my opinion un-related 
to the patches I have proposed.



Your thoughts?



Thanks.

-Zahira





-Original Message-
From: hwennb...@google.com [mailto:hwennb...@google.com] On Behalf Of Hans 
Wennborg
Sent: Monday, February 19, 2018 5:11 AM
To: cfe-commits <cfe-commits@lists.llvm.org>; Ammarguellat, Zahira 
<zahira.ammarguel...@intel.com>
Subject: Re: r324991 - Fix for PR32992. Static const classes not exported.



Reduced repro:



$ clang -cc1 -triple i386-pc-windows-msvc19.11.0 -emit-pch -fms-extensions 
-fms-compatibility -fms-compatibility-version=19.11

-std=c++14 -fdelayed-template-parsing -x c++-header a.ii -o /dev/null



a.ii:



namespace a {

enum b { c };

}

template  class d { static constexpr a::b e = a::c; }; namespace f { 
template  class h : d {}; } using f::h; class 
__declspec(dllexport) i : h<> {};



On Wed, Feb 14, 2018 at 4:22 PM, Hans Wennborg 
<h...@chromium.org<mailto:h...@chromium.org>> wrote:

> I ended up having to revert this in r325133 as it broke the Chromium

> build. Please see

> https://bugs.chromium.org/p/chromium/issues/detail?id=812231#c1 for a

> reproducer.

>

> On Tue, Feb 13, 2018 at 10:19 AM, Hans Wennborg via cfe-commits

> <cfe-commits@lists.llvm.org<mailto:cfe-commits@lists.llvm.org>> wrote:

>> Author: hans

>> Date: Tue Feb 13 01:19:43 2018

>> New Revision: 324991

>>

>> URL: http://llvm.org/viewvc/llvm-project?rev=324991=rev

>> Log:

>> Fix for PR32992. Static const classes not exported.

>>

>> Patch by zahiraam!

>>

>> Differential Revision: https://reviews.llvm.org/D42968

>>

>> Modified:

>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp

>> cfe/trunk/test/CodeGenCXX/dllexport.cpp

>>

>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp

>> URL:

>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cp

>> p?rev=324991=324990=324991=diff

>> =

>> =

>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)

>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Feb 13 01:19:43 2018

>> @@ -5476,7 +5476,7 @@ static void CheckAbstractClassUsage(Abst

>>}

>>  }

>>

>> -static void ReferenceDllExportedMethods(Sema , CXXRecordDecl

>> *Class) {

>> +static void ReferenceDllExportedMembers(Sema , CXXRecordDecl

>> +*Class) {

>>Attr *ClassAttr = getDLLAttr(Class);

>>if (!ClassAttr)

>>  return;

>> @@ -5491,6 +5491,16 @@ static void ReferenceDllExportedMethods(

>>  return;

>>

>>for (Decl *Member : Class->decls()) {

>> +// Defined static variables that are members of an exported base

>> +// class must be marked export too. Push them to implicit instantiation

>> +// queue.

>> +auto *VD = dyn_cast(Member);

>> +if (VD && Member->getAttr() &&

>> +VD->getStorageClass() == SC_Static &&

>> +TSK == TSK_ImplicitInstantiation)

>> +  S.PendingLocalImplicitInstantiations.push_back(

>> +  std::make_pair(VD, VD->getLocation()));

>> +

>>  auto *MD = dyn_cast(Member);

>>  if (!MD)

>>continue;

>> @@ -10902,12 +10912,12 @@ void Sema::ActOnFinishCXXNonNestedClass(

>>

>>  void Sema::referenceDLLExportedClassMethods() {

>>if (!DelayedDllExportClasses.empty()) {

>> -// Calling ReferenceDllExportedMethods might cause the current function 
>> to

>> +// Calling ReferenceDllExportedMembers might cause the current

>> + function to

>>  // be called again, so use a local copy of

Re: r324991 - Fix for PR32992. Static const classes not exported.

2018-02-19 Thread Hans Wennborg via cfe-commits
Reduced repro:

$ clang -cc1 -triple i386-pc-windows-msvc19.11.0 -emit-pch
-fms-extensions -fms-compatibility -fms-compatibility-version=19.11
-std=c++14 -fdelayed-template-parsing -x c++-header a.ii -o /dev/null

a.ii:

namespace a {
enum b { c };
}
template  class d { static constexpr a::b e = a::c; };
namespace f {
template  class h : d {};
}
using f::h;
class __declspec(dllexport) i : h<> {};

On Wed, Feb 14, 2018 at 4:22 PM, Hans Wennborg  wrote:
> I ended up having to revert this in r325133 as it broke the Chromium
> build. Please see
> https://bugs.chromium.org/p/chromium/issues/detail?id=812231#c1 for a
> reproducer.
>
> On Tue, Feb 13, 2018 at 10:19 AM, Hans Wennborg via cfe-commits
>  wrote:
>> Author: hans
>> Date: Tue Feb 13 01:19:43 2018
>> New Revision: 324991
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=324991=rev
>> Log:
>> Fix for PR32992. Static const classes not exported.
>>
>> Patch by zahiraam!
>>
>> Differential Revision: https://reviews.llvm.org/D42968
>>
>> Modified:
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/test/CodeGenCXX/dllexport.cpp
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=324991=324990=324991=diff
>> ==
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Feb 13 01:19:43 2018
>> @@ -5476,7 +5476,7 @@ static void CheckAbstractClassUsage(Abst
>>}
>>  }
>>
>> -static void ReferenceDllExportedMethods(Sema , CXXRecordDecl *Class) {
>> +static void ReferenceDllExportedMembers(Sema , CXXRecordDecl *Class) {
>>Attr *ClassAttr = getDLLAttr(Class);
>>if (!ClassAttr)
>>  return;
>> @@ -5491,6 +5491,16 @@ static void ReferenceDllExportedMethods(
>>  return;
>>
>>for (Decl *Member : Class->decls()) {
>> +// Defined static variables that are members of an exported base
>> +// class must be marked export too. Push them to implicit instantiation
>> +// queue.
>> +auto *VD = dyn_cast(Member);
>> +if (VD && Member->getAttr() &&
>> +VD->getStorageClass() == SC_Static &&
>> +TSK == TSK_ImplicitInstantiation)
>> +  S.PendingLocalImplicitInstantiations.push_back(
>> +  std::make_pair(VD, VD->getLocation()));
>> +
>>  auto *MD = dyn_cast(Member);
>>  if (!MD)
>>continue;
>> @@ -10902,12 +10912,12 @@ void Sema::ActOnFinishCXXNonNestedClass(
>>
>>  void Sema::referenceDLLExportedClassMethods() {
>>if (!DelayedDllExportClasses.empty()) {
>> -// Calling ReferenceDllExportedMethods might cause the current function 
>> to
>> +// Calling ReferenceDllExportedMembers might cause the current function 
>> to
>>  // be called again, so use a local copy of DelayedDllExportClasses.
>>  SmallVector WorkList;
>>  std::swap(DelayedDllExportClasses, WorkList);
>>  for (CXXRecordDecl *Class : WorkList)
>> -  ReferenceDllExportedMethods(*this, Class);
>> +  ReferenceDllExportedMembers(*this, Class);
>>}
>>  }
>>
>>
>> Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
>> URL: 
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=324991=324990=324991=diff
>> ==
>> --- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Tue Feb 13 01:19:43 2018
>> @@ -28,6 +28,7 @@ struct External { int v; };
>>
>>  // The vftable for struct W is comdat largest because we have RTTI.
>>  // M32-DAG: $"\01??_7W@@6B@" = comdat largest
>> +// M32-DAG: $"\01?smember@?$Base@H@PR32992@@0HA" = comdat any
>>
>>
>>  
>> //===--===//
>> @@ -977,3 +978,21 @@ class __declspec(dllexport) ACE_Service_
>>  // MSVC2015-DAG: define weak_odr dllexport 
>> {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
>>  // The declarations should not be exported.
>>  // MSVC2013-NOT: define weak_odr dllexport 
>> {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
>> +
>> +namespace PR32992 {
>> +// Static data members of a instantiated base class should be exported.
>> +template 
>> +class Base {
>> +  virtual void myfunc() {}
>> +  static int smember;
>> +};
>> +// MS-DAG:  @"\01?smember@?$Base@H@PR32992@@0HA" = weak_odr dllexport 
>> global i32 77, comdat, align 4
>> +template  int Base::smember = 77;
>> +template 
>> +class __declspec(dllexport) Derived2 : Base {
>> +  void myfunc() {}
>> +};
>> +class Derived : public Derived2 {
>> +  void myfunc() {}
>> +};
>> +}  // namespace PR32992
>>
>>
>> ___
>> cfe-commits mailing list
>> cfe-commits@lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org

Re: r324991 - Fix for PR32992. Static const classes not exported.

2018-02-14 Thread Hans Wennborg via cfe-commits
I ended up having to revert this in r325133 as it broke the Chromium
build. Please see
https://bugs.chromium.org/p/chromium/issues/detail?id=812231#c1 for a
reproducer.

On Tue, Feb 13, 2018 at 10:19 AM, Hans Wennborg via cfe-commits
 wrote:
> Author: hans
> Date: Tue Feb 13 01:19:43 2018
> New Revision: 324991
>
> URL: http://llvm.org/viewvc/llvm-project?rev=324991=rev
> Log:
> Fix for PR32992. Static const classes not exported.
>
> Patch by zahiraam!
>
> Differential Revision: https://reviews.llvm.org/D42968
>
> Modified:
> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> cfe/trunk/test/CodeGenCXX/dllexport.cpp
>
> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=324991=324990=324991=diff
> ==
> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Feb 13 01:19:43 2018
> @@ -5476,7 +5476,7 @@ static void CheckAbstractClassUsage(Abst
>}
>  }
>
> -static void ReferenceDllExportedMethods(Sema , CXXRecordDecl *Class) {
> +static void ReferenceDllExportedMembers(Sema , CXXRecordDecl *Class) {
>Attr *ClassAttr = getDLLAttr(Class);
>if (!ClassAttr)
>  return;
> @@ -5491,6 +5491,16 @@ static void ReferenceDllExportedMethods(
>  return;
>
>for (Decl *Member : Class->decls()) {
> +// Defined static variables that are members of an exported base
> +// class must be marked export too. Push them to implicit instantiation
> +// queue.
> +auto *VD = dyn_cast(Member);
> +if (VD && Member->getAttr() &&
> +VD->getStorageClass() == SC_Static &&
> +TSK == TSK_ImplicitInstantiation)
> +  S.PendingLocalImplicitInstantiations.push_back(
> +  std::make_pair(VD, VD->getLocation()));
> +
>  auto *MD = dyn_cast(Member);
>  if (!MD)
>continue;
> @@ -10902,12 +10912,12 @@ void Sema::ActOnFinishCXXNonNestedClass(
>
>  void Sema::referenceDLLExportedClassMethods() {
>if (!DelayedDllExportClasses.empty()) {
> -// Calling ReferenceDllExportedMethods might cause the current function 
> to
> +// Calling ReferenceDllExportedMembers might cause the current function 
> to
>  // be called again, so use a local copy of DelayedDllExportClasses.
>  SmallVector WorkList;
>  std::swap(DelayedDllExportClasses, WorkList);
>  for (CXXRecordDecl *Class : WorkList)
> -  ReferenceDllExportedMethods(*this, Class);
> +  ReferenceDllExportedMembers(*this, Class);
>}
>  }
>
>
> Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=324991=324990=324991=diff
> ==
> --- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Tue Feb 13 01:19:43 2018
> @@ -28,6 +28,7 @@ struct External { int v; };
>
>  // The vftable for struct W is comdat largest because we have RTTI.
>  // M32-DAG: $"\01??_7W@@6B@" = comdat largest
> +// M32-DAG: $"\01?smember@?$Base@H@PR32992@@0HA" = comdat any
>
>
>  
> //===--===//
> @@ -977,3 +978,21 @@ class __declspec(dllexport) ACE_Service_
>  // MSVC2015-DAG: define weak_odr dllexport 
> {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
>  // The declarations should not be exported.
>  // MSVC2013-NOT: define weak_odr dllexport 
> {{.+}}ACE_Service_Object@@Q{{.+}}@$$Q
> +
> +namespace PR32992 {
> +// Static data members of a instantiated base class should be exported.
> +template 
> +class Base {
> +  virtual void myfunc() {}
> +  static int smember;
> +};
> +// MS-DAG:  @"\01?smember@?$Base@H@PR32992@@0HA" = weak_odr dllexport global 
> i32 77, comdat, align 4
> +template  int Base::smember = 77;
> +template 
> +class __declspec(dllexport) Derived2 : Base {
> +  void myfunc() {}
> +};
> +class Derived : public Derived2 {
> +  void myfunc() {}
> +};
> +}  // namespace PR32992
>
>
> ___
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits