r232519 - MS ABI: Delay default constructor closure checking until the outermost class scope ends

Nico Weber thakis at chromium.org
Tue Mar 17 13:22:53 PDT 2015


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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150317/36ae2d55/attachment.html>


More information about the cfe-commits mailing list