[PATCH] [ms-cxxabi] Destroy temporary record arguments in the callee

[John McCall]

On Jun 19, 2013, at 3:04 PM, Reid Kleckner <rnk at google.com> wrote:
> On Wed, Jun 19, 2013 at 5:17 PM, John McCall <rjmccall at apple.com> wrote:
> On Jun 19, 2013, at 1:44 PM, Reid Kleckner <rnk at google.com> wrote:
> >    - Add conditional tests
> >    - Deactivate cleanups in reverse
> >    - Other comments.
> 
> +  /// \brief Are temporary objects passed by value to a call destroyed by the
> +  /// callee?
> +  bool isTemporaryDestroyedByCallee() const {
> 
> Please embellish this comment to discuss the ways in which this is
> actually a major language difference.
> 
> Done.
>  
> +void Sema::CheckByValParamsForDtors(FunctionDecl *FD) {
> +  assert(Context.getTargetInfo().getCXXABI().isTemporaryDestroyedByCallee());
> +  for (unsigned p = 0, NumParams = FD->getNumParams(); p < NumParams; ++p) {
> +    ParmVarDecl *Param = FD->getParamDecl(p);
> +    if (CXXRecordDecl *RD = Param->getType()->getAsCXXRecordDecl()) {
> +      if (CXXDestructorDecl *Dtor = LookupDestructor(RD)) {
> +        // Don't call CheckDestructorAccess() or we'll reject valid TUs.
> +        MarkFunctionReferenced(FD->getLocation(), Dtor);
> +        DiagnoseUseOfDecl(Dtor, FD->getLocation());
> +      }
> +    }
> +  }
> +}
> +
> 
> I think this code is dead now?
> 
> Oops, deleted.
>  
> You should add my test case, and then you'll need to go do some
> spelunking to make it pass, because you'll have to disable the check
> for a destructor when initializing a parameter temporary.
> 
> I'd rather err on the side of rejecting because MSVC rejects this for example:
> 
> struct A {
>   A(const A &a);  // Comment out to accept.
>  private:
>   ~A();
>   friend void bar(A);
> };
> void bar(A a) { }
> void foo(A *a) { bar(*a); }  // error C2248: 'A::~A' : cannot access private member declared in class 'A'
> 
> They seem to have some heuristics in their equivalent of Sema to guess whether or not they'll need to call ~A.  I don't know if we want to mirror that or not.

This difference might be about checks for formal temporaries that are in practice elided; I'm not sure.

Anyhow, if you can't find a reasonable model for what MSVC actually does, I am fine with conservatively requiring the destructor on both sides.  You should capture this in that comment you're embellishing, though.

John.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130619/f04f2985/attachment.html>