From 61195e12fc5cc128e5908cfb76a91ee6860659e5 Mon Sep 17 00:00:00 2001 From: Reid Kleckner Date: Thu, 5 Jan 2017 01:08:22 +0000 Subject: [PATCH] [MS] Instantiate default args during instantiation of exported default ctors Summary: Replace some old code that probably pre-dated the change to delay emission of dllexported code until after the closing brace of the outermost record type. Only uninstantiated default argument expressions need to be handled now. It is enough to instantiate default argument expressions when instantiating dllexported default ctors. This also fixes some double-diagnostic issues in this area. Fixes PR31500 Reviewers: rsmith Subscribers: cfe-commits Differential Revision: https://reviews.llvm.org/D28274 llvm-svn: 291045 --- clang/lib/Sema/SemaDeclCXX.cpp | 75 ++++++------------- .../lib/Sema/SemaTemplateInstantiateDecl.cpp | 35 ++++++++- clang/test/CodeGenCXX/dllexport.cpp | 12 +++ clang/test/SemaCXX/default-arg-closures.cpp | 9 +-- clang/test/SemaCXX/dllexport.cpp | 21 ++++++ 5 files changed, 89 insertions(+), 63 deletions(-) diff --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp index 084bd4c45eda..a650621b573a 100644 --- a/clang/lib/Sema/SemaDeclCXX.cpp +++ b/clang/lib/Sema/SemaDeclCXX.cpp @@ -5395,6 +5395,26 @@ static void ReferenceDllExportedMethods(Sema &S, CXXRecordDecl *Class) { } } +static void checkForMultipleExportedDefaultConstructors(Sema &S, CXXRecordDecl *Class) { + CXXConstructorDecl *LastExportedDefaultCtor = nullptr; + for (Decl *Member : Class->decls()) { + // Look for exported default constructors. + auto *CD = dyn_cast(Member); + if (!CD || !CD->isDefaultConstructor() || !CD->hasAttr()) + continue; + + if (LastExportedDefaultCtor) { + S.Diag(LastExportedDefaultCtor->getLocation(), + diag::err_attribute_dll_ambiguous_default_ctor) + << Class; + S.Diag(CD->getLocation(), diag::note_entity_declared_at) + << CD->getDeclName(); + return; + } + LastExportedDefaultCtor = CD; + } +} + /// \brief Check class-level dllimport/dllexport attribute. void Sema::checkClassLevelDLLAttribute(CXXRecordDecl *Class) { Attr *ClassAttr = getDLLAttr(Class); @@ -10362,64 +10382,11 @@ void Sema::ActOnFinishCXXMemberDecls() { DelayedExceptionSpecChecks.clear(); return; } - } -} - -static void checkDefaultArgExprsForConstructors(Sema &S, CXXRecordDecl *Class) { - // Don't do anything for template patterns. - if (Class->getDescribedClassTemplate()) - return; - - CallingConv ExpectedCallingConv = S.Context.getDefaultCallingConvention( - /*IsVariadic=*/false, /*IsCXXMethod=*/true); - - CXXConstructorDecl *LastExportedDefaultCtor = nullptr; - for (Decl *Member : Class->decls()) { - auto *CD = dyn_cast(Member); - if (!CD) { - // Recurse on nested classes. - if (auto *NestedRD = dyn_cast(Member)) - checkDefaultArgExprsForConstructors(S, NestedRD); - continue; - } else if (!CD->isDefaultConstructor() || !CD->hasAttr()) { - continue; - } - - CallingConv ActualCallingConv = - CD->getType()->getAs()->getCallConv(); - - // Skip default constructors with typical calling conventions and no default - // arguments. - unsigned NumParams = CD->getNumParams(); - if (ExpectedCallingConv == ActualCallingConv && NumParams == 0) - continue; - - if (LastExportedDefaultCtor) { - S.Diag(LastExportedDefaultCtor->getLocation(), - diag::err_attribute_dll_ambiguous_default_ctor) << Class; - S.Diag(CD->getLocation(), diag::note_entity_declared_at) - << CD->getDeclName(); - return; - } - LastExportedDefaultCtor = CD; - - for (unsigned I = 0; I != NumParams; ++I) { - (void)S.CheckCXXDefaultArgExpr(Class->getLocation(), CD, - CD->getParamDecl(I)); - S.DiscardCleanupsInEvaluationContext(); - } + checkForMultipleExportedDefaultConstructors(*this, Record); } } void Sema::ActOnFinishCXXNonNestedClass(Decl *D) { - auto *RD = dyn_cast(D); - - // Default constructors that are annotated with __declspec(dllexport) which - // have default arguments or don't use the standard calling convention are - // wrapped with a thunk called the default constructor closure. - if (RD && Context.getTargetInfo().getCXXABI().isMicrosoft()) - checkDefaultArgExprsForConstructors(*this, RD); - referenceDLLExportedClassMethods(); } diff --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp index 3cc938106d80..f4013b820641 100644 --- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp +++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp @@ -3619,6 +3619,27 @@ TemplateDeclInstantiator::InitMethodInstantiation(CXXMethodDecl *New, return false; } +/// In the MS ABI, we need to instantiate default arguments of dllexported +/// default constructors along with the constructor definition. This allows IR +/// gen to emit a constructor closure which calls the default constructor with +/// its default arguments. +static void InstantiateDefaultCtorDefaultArgs(Sema &S, + CXXConstructorDecl *Ctor) { + assert(S.Context.getTargetInfo().getCXXABI().isMicrosoft() && + Ctor->isDefaultConstructor()); + unsigned NumParams = Ctor->getNumParams(); + if (NumParams == 0) + return; + DLLExportAttr *Attr = Ctor->getAttr(); + if (!Attr) + return; + for (unsigned I = 0; I != NumParams; ++I) { + (void)S.CheckCXXDefaultArgExpr(Attr->getLocation(), Ctor, + Ctor->getParamDecl(I)); + S.DiscardCleanupsInEvaluationContext(); + } +} + /// \brief Instantiate the definition of the given function from its /// template. /// @@ -3796,11 +3817,17 @@ void Sema::InstantiateFunctionDefinition(SourceLocation PointOfInstantiation, TemplateArgs)) return; - // If this is a constructor, instantiate the member initializers. - if (const CXXConstructorDecl *Ctor = - dyn_cast(PatternDecl)) { - InstantiateMemInitializers(cast(Function), Ctor, + if (CXXConstructorDecl *Ctor = dyn_cast(Function)) { + // If this is a constructor, instantiate the member initializers. + InstantiateMemInitializers(Ctor, cast(PatternDecl), TemplateArgs); + + // If this is an MS ABI dllexport default constructor, instantiate any + // default arguments. + if (Context.getTargetInfo().getCXXABI().isMicrosoft() && + Ctor->isDefaultConstructor()) { + InstantiateDefaultCtorDefaultArgs(*this, Ctor); + } } // Instantiate the function body. diff --git a/clang/test/CodeGenCXX/dllexport.cpp b/clang/test/CodeGenCXX/dllexport.cpp index eb9ca79b7b40..116176e2cb92 100644 --- a/clang/test/CodeGenCXX/dllexport.cpp +++ b/clang/test/CodeGenCXX/dllexport.cpp @@ -515,6 +515,18 @@ struct __declspec(dllexport) ClassWithClosure { // M32-DAG: ret void }; +template struct TemplateWithClosure { + TemplateWithClosure(int x = sizeof(T)) {} +}; +extern template struct TemplateWithClosure; +template struct __declspec(dllexport) TemplateWithClosure; +extern template struct TemplateWithClosure; +template struct __declspec(dllexport) TemplateWithClosure; +// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01??_F?$TemplateWithClosure@D@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat +// M32-DAG: call {{.*}} @"\01??0?$TemplateWithClosure@D@@QAE@H@Z"({{.*}}, i32 1) +// M32-DAG: define weak_odr dllexport x86_thiscallcc void @"\01??_F?$TemplateWithClosure@H@@QAEXXZ"({{.*}}) {{#[0-9]+}} comdat +// M32-DAG: call {{.*}} @"\01??0?$TemplateWithClosure@H@@QAE@H@Z"({{.*}}, i32 4) + struct __declspec(dllexport) NestedOuter { DELETE_IMPLICIT_MEMBERS(NestedOuter); NestedOuter(void *p = 0) {} diff --git a/clang/test/SemaCXX/default-arg-closures.cpp b/clang/test/SemaCXX/default-arg-closures.cpp index e076cc05cd20..676bd486105f 100644 --- a/clang/test/SemaCXX/default-arg-closures.cpp +++ b/clang/test/SemaCXX/default-arg-closures.cpp @@ -4,16 +4,15 @@ // instantiating and checking the semantics of default arguments. Make sure we // do that right. -// FIXME: Don't diagnose this issue twice. template -struct DependentDefaultCtorArg { // expected-note {{in instantiation of default function argument}} - // expected-error@+1 2 {{type 'int' cannot be used prior to '::' because it has no members}} +struct DependentDefaultCtorArg { + // expected-error@+1 {{type 'int' cannot be used prior to '::' because it has no members}} DependentDefaultCtorArg(int n = T::error); }; struct __declspec(dllexport) // expected-note {{due to 'ExportDefaultCtorClosure' being dllexported}} -ExportDefaultCtorClosure // expected-note {{implicit default constructor for 'ExportDefaultCtorClosure' first required here}} -: DependentDefaultCtorArg // expected-note {{in instantiation of template class}} +ExportDefaultCtorClosure // expected-note {{in instantiation of default function argument expression for 'DependentDefaultCtorArg' required here}} expected-note {{implicit default constructor for 'ExportDefaultCtorClosure' first required here}} +: DependentDefaultCtorArg {}; template diff --git a/clang/test/SemaCXX/dllexport.cpp b/clang/test/SemaCXX/dllexport.cpp index b4850fc03d9b..a3fed70ec958 100644 --- a/clang/test/SemaCXX/dllexport.cpp +++ b/clang/test/SemaCXX/dllexport.cpp @@ -741,6 +741,27 @@ struct __declspec(dllexport) ClassWithMultipleDefaultCtors { ClassWithMultipleDefaultCtors(int = 40) {} // expected-error{{'__declspec(dllexport)' cannot be applied to more than one default constructor}} ClassWithMultipleDefaultCtors(int = 30, ...) {} // expected-note{{declared here}} }; +template +struct ClassTemplateWithMultipleDefaultCtors { + __declspec(dllexport) ClassTemplateWithMultipleDefaultCtors(int = 40) {} // expected-error{{'__declspec(dllexport)' cannot be applied to more than one default constructor}} + __declspec(dllexport) ClassTemplateWithMultipleDefaultCtors(int = 30, ...) {} // expected-note{{declared here}} +}; + +template struct HasDefaults { + HasDefaults(int x = sizeof(T)) {} // expected-error {{invalid application of 'sizeof'}} +}; +template struct __declspec(dllexport) HasDefaults; + +template struct +__declspec(dllexport) // expected-note {{in instantiation of default function argument expression for 'HasDefaults' required here}} +HasDefaults; // expected-note {{in instantiation of member function 'HasDefaults::HasDefaults' requested here}} + +template struct HasDefaults2 { + __declspec(dllexport) // expected-note {{in instantiation of default function argument expression for 'HasDefaults2' required here}} + HasDefaults2(int x = sizeof(T)) {} // expected-error {{invalid application of 'sizeof'}} +}; +template struct HasDefaults2; // expected-note {{in instantiation of member function 'HasDefaults2::HasDefaults2' requested here}} + #endif //===----------------------------------------------------------------------===//