r232519 - MS ABI: Delay default constructor closure checking until the outermost class scope ends
Reid Kleckner
rnk at google.com
Tue Mar 17 14:07:57 PDT 2015
I've reduced it to this:
template <typename T>
struct SomeTemplate {
SomeTemplate(T o = T()) : o(o) {}
T o;
};
struct __declspec(dllexport) InheritFromTemplate : SomeTemplate<int> {};
This asserts due to isa on a null pointer, but it asserted in the same way
before my change.
On Tue, Mar 17, 2015 at 1:22 PM, Nico Weber <thakis at chromium.org> wrote:
>
> http://build.chromium.org/p/chromium.fyi/builders/CrWinClang%28dbg%29/builds/0/steps/compile/logs/stdio
> has many `Assertion failed: Val && "isa<> used on a null pointer"` that
> used to not be there, and this change added an isa<>() call. Maybe it's
> related?
>
> On Tue, Mar 17, 2015 at 3:00 PM, Reid Kleckner <reid at kleckner.net> wrote:
>
>> Author: rnk
>> Date: Tue Mar 17 14:00:50 2015
>> New Revision: 232519
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=232519&view=rev
>> Log:
>> MS ABI: Delay default constructor closure checking until the outermost
>> class scope ends
>>
>> Previously, we would error out on this code because the default argument
>> wasn't parsed until the end of Outer:
>>
>> struct __declspec(dllexport) Outer {
>> struct __declspec(dllexport) Inner {
>> Inner(void *p = 0);
>> };
>> };
>>
>> Now we do the checking on the closing brace of Outer instead of Inner.
>>
>> Modified:
>> cfe/trunk/include/clang/Sema/Sema.h
>> cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>> cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> cfe/trunk/lib/Sema/SemaDecl.cpp
>> cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> cfe/trunk/test/CodeGenCXX/dllexport.cpp
>>
>> Modified: cfe/trunk/include/clang/Sema/Sema.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=232519&r1=232518&r2=232519&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Sema/Sema.h (original)
>> +++ cfe/trunk/include/clang/Sema/Sema.h Tue Mar 17 14:00:50 2015
>> @@ -5012,6 +5012,7 @@ public:
>> SourceLocation RBrac,
>> AttributeList *AttrList);
>> void ActOnFinishCXXMemberDecls();
>> + void ActOnFinishCXXMethodDefs(Decl *D);
>>
>> void ActOnReenterCXXMethodParameter(Scope *S, ParmVarDecl *Param);
>> unsigned ActOnReenterTemplateScope(Scope *S, Decl *Template);
>>
>> Modified: cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp?rev=232519&r1=232518&r2=232519&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/MicrosoftCXXABI.cpp Tue Mar 17 14:00:50 2015
>> @@ -3309,6 +3309,8 @@ MicrosoftCXXABI::getAddrOfCXXCtorClosure
>> QualType RecordTy = getContext().getRecordType(RD);
>> llvm::Function *ThunkFn = llvm::Function::Create(
>> ThunkTy, getLinkageForRTTI(RecordTy), ThunkName.str(),
>> &CGM.getModule());
>> + ThunkFn->setCallingConv(static_cast<llvm::CallingConv::ID>(
>> + FnInfo.getEffectiveCallingConvention()));
>> bool IsCopy = CT == Ctor_CopyingClosure;
>>
>> // Start codegen.
>>
>> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=232519&r1=232518&r2=232519&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Tue Mar 17 14:00:50 2015
>> @@ -2915,6 +2915,10 @@ void Parser::ParseCXXMemberSpecification
>> ParseLexedMemberInitializers(getCurrentClass());
>> ParseLexedMethodDefs(getCurrentClass());
>> PrevTokLocation = SavedPrevTokLocation;
>> +
>> + // We've finished parsing everything, including default argument
>> + // initializers.
>> + Actions.ActOnFinishCXXMethodDefs(TagDecl);
>> }
>>
>> if (TagDecl)
>>
>> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=232519&r1=232518&r2=232519&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Mar 17 14:00:50 2015
>> @@ -12067,24 +12067,6 @@ void Sema::ActOnStartCXXMemberDeclaratio
>> "Broken injected-class-name");
>> }
>>
>> -static void getDefaultArgExprsForConstructors(Sema &S, CXXRecordDecl
>> *Class) {
>> - for (Decl *Member : Class->decls()) {
>> - auto *CD = dyn_cast<CXXConstructorDecl>(Member);
>> - if (!CD || !CD->isDefaultConstructor() ||
>> !CD->hasAttr<DLLExportAttr>())
>> - continue;
>> -
>> - for (unsigned I = 0, E = CD->getNumParams(); I != E; ++I) {
>> - // Skip any default arguments that we've already instantiated.
>> - if (S.Context.getDefaultArgExprForConstructor(CD, I))
>> - continue;
>> -
>> - Expr *DefaultArg = S.BuildCXXDefaultArgExpr(Class->getLocation(),
>> CD,
>> -
>> CD->getParamDecl(I)).get();
>> - S.Context.addDefaultArgExprForConstructor(CD, I, DefaultArg);
>> - }
>> - }
>> -}
>> -
>> void Sema::ActOnTagFinishDefinition(Scope *S, Decl *TagD,
>> SourceLocation RBraceLoc) {
>> AdjustDeclIfTemplate(TagD);
>> @@ -12098,17 +12080,9 @@ void Sema::ActOnTagFinishDefinition(Scop
>> RD->completeDefinition();
>> }
>>
>> - if (auto *Class = dyn_cast<CXXRecordDecl>(Tag)) {
>> + if (isa<CXXRecordDecl>(Tag))
>> FieldCollector->FinishClass();
>>
>> - // 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 (!Class->getDescribedClassTemplate() &&
>> - Context.getTargetInfo().getCXXABI().isMicrosoft())
>> - getDefaultArgExprsForConstructors(*this, Class);
>> - }
>> -
>> // Exit this scope of this tag's definition.
>> PopDeclContext();
>>
>>
>> Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=232519&r1=232518&r2=232519&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
>> +++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Tue Mar 17 14:00:50 2015
>> @@ -9421,6 +9421,44 @@ void Sema::ActOnFinishCXXMemberDecls() {
>> }
>> }
>>
>> +static void getDefaultArgExprsForConstructors(Sema &S, CXXRecordDecl
>> *Class) {
>> + // Don't do anything for template patterns.
>> + if (Class->getDescribedClassTemplate())
>> + return;
>> +
>> + for (Decl *Member : Class->decls()) {
>> + auto *CD = dyn_cast<CXXConstructorDecl>(Member);
>> + if (!CD) {
>> + // Recurse on nested classes.
>> + if (auto *NestedRD = dyn_cast<CXXRecordDecl>(Member))
>> + getDefaultArgExprsForConstructors(S, NestedRD);
>> + continue;
>> + } else if (!CD->isDefaultConstructor() ||
>> !CD->hasAttr<DLLExportAttr>()) {
>> + continue;
>> + }
>> +
>> + for (unsigned I = 0, E = CD->getNumParams(); I != E; ++I) {
>> + // Skip any default arguments that we've already instantiated.
>> + if (S.Context.getDefaultArgExprForConstructor(CD, I))
>> + continue;
>> +
>> + Expr *DefaultArg = S.BuildCXXDefaultArgExpr(Class->getLocation(),
>> CD,
>> +
>> CD->getParamDecl(I)).get();
>> + S.Context.addDefaultArgExprForConstructor(CD, I, DefaultArg);
>> + }
>> + }
>> +}
>> +
>> +void Sema::ActOnFinishCXXMethodDefs(Decl *D) {
>> + auto *RD = dyn_cast<CXXRecordDecl>(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())
>> + getDefaultArgExprsForConstructors(*this, RD);
>> +}
>> +
>> void Sema::AdjustDestructorExceptionSpec(CXXRecordDecl *ClassDecl,
>> CXXDestructorDecl *Destructor) {
>> assert(getLangOpts().CPlusPlus11 &&
>>
>> Modified: cfe/trunk/test/CodeGenCXX/dllexport.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/dllexport.cpp?rev=232519&r1=232518&r2=232519&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/CodeGenCXX/dllexport.cpp (original)
>> +++ cfe/trunk/test/CodeGenCXX/dllexport.cpp Tue Mar 17 14:00:50 2015
>> @@ -486,7 +486,7 @@ struct S {
>>
>> struct CtorWithClosure {
>> __declspec(dllexport) CtorWithClosure(...) {}
>> -// M32-DAG: define weak_odr dllexport void @"\01??_FCtorWithClosure@
>> @QAEXXZ"
>> +// M32-DAG: define weak_odr dllexport x86_thiscallcc void
>> @"\01??_FCtorWithClosure@@QAEXXZ"
>> // M32-DAG: %[[this_addr:.*]] = alloca %struct.CtorWithClosure*, align
>> 4
>> // M32-DAG: store %struct.CtorWithClosure* %this,
>> %struct.CtorWithClosure** %[[this_addr]], align 4
>> // M32-DAG: %[[this:.*]] = load %struct.CtorWithClosure*,
>> %struct.CtorWithClosure** %[[this_addr]]
>> @@ -494,12 +494,16 @@ struct CtorWithClosure {
>> // M32-DAG: ret void
>> };
>>
>> +#define DELETE_IMPLICIT_MEMBERS(ClassName) \
>> + ClassName(ClassName &&) = delete; \
>> + ClassName(ClassName &) = delete; \
>> + ~ClassName() = delete; \
>> + ClassName &operator=(ClassName &) = delete
>> +
>> struct __declspec(dllexport) ClassWithClosure {
>> - ClassWithClosure(ClassWithClosure &&) = delete;
>> - ClassWithClosure(ClassWithClosure &) = delete;
>> - ~ClassWithClosure() = delete;
>> + DELETE_IMPLICIT_MEMBERS(ClassWithClosure);
>> ClassWithClosure(...) {}
>> -// M32-DAG: define weak_odr dllexport void @"\01??_FClassWithClosure@
>> @QAEXXZ"
>> +// M32-DAG: define weak_odr dllexport x86_thiscallcc void
>> @"\01??_FClassWithClosure@@QAEXXZ"
>> // M32-DAG: %[[this_addr:.*]] = alloca %struct.ClassWithClosure*,
>> align 4
>> // M32-DAG: store %struct.ClassWithClosure* %this,
>> %struct.ClassWithClosure** %[[this_addr]], align 4
>> // M32-DAG: %[[this:.*]] = load %struct.ClassWithClosure*,
>> %struct.ClassWithClosure** %[[this_addr]]
>> @@ -507,6 +511,18 @@ struct __declspec(dllexport) ClassWithCl
>> // M32-DAG: ret void
>> };
>>
>> +struct __declspec(dllexport) NestedOuter {
>> + DELETE_IMPLICIT_MEMBERS(NestedOuter);
>> + NestedOuter(void *p = 0) {}
>> + struct __declspec(dllexport) NestedInner {
>> + DELETE_IMPLICIT_MEMBERS(NestedInner);
>> + NestedInner(void *p = 0) {}
>> + };
>> +};
>> +
>> +// M32-DAG: define weak_odr dllexport x86_thiscallcc void
>> @"\01??_FNestedOuter@@QAEXXZ"
>> +// M32-DAG: define weak_odr dllexport x86_thiscallcc void
>> @"\01??_FNestedInner at NestedOuter@@QAEXXZ"
>> +
>> struct __declspec(dllexport) T {
>> // Copy assignment operator:
>> // M32-DAG: define weak_odr dllexport x86_thiscallcc
>> dereferenceable({{[0-9]+}}) %struct.T* @"\01??4T@@QAEAAU0 at ABU0@@Z"
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150317/9fda9eb9/attachment.html>
More information about the cfe-commits
mailing list