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

[John McCall]

On Jun 20, 2013, at 6:15 AM, Reid Kleckner <rnk at google.com> wrote:
> On Tue, Jun 18, 2013 at 10:15 PM, John McCall <rjmccall at apple.com> wrote:
> +
> +    // In the MSVC++ ABI, make sure arguments have valid destructors.
> +    if (Context.getTargetInfo().getCXXABI().isTemporaryDestroyedByCallee())
> +      CheckByValParamsForDtors(NewFD);
> +
> 
> Please just do this in CheckParameter.  Check for C++ first, then the ABI flag,
> then a record type, then call FinalizeVarWithDestructor. :)
> 
> On further testing, I don't think this works because CheckParameter fires on function declarations, where the record might not be complete:
> struct A;
> void foo(A a);  // Can't look at A's dtor here.
> class A {
>   friend void bar(A);  // Can't look at A's dtor here.
> };
> 
> We don't actually know if the parameter decl is for a definition until ActOnFunctionDeclarator().

Good point, sorry.

Hmm.  It looks like ActOnStartOfFunctionDef, ActOnStartOfObjCMethodDef, ActOnBlockArguments, and addLambdaParameters all have basically the same loop in them, but doing slightly different checks for no particular reason.  And there's already a CheckParmsForFunctionDef method, but it's apparently not called for ObjC methods.

I think it makes sense to have a single method called in all of these cases which performs the basic checks, sets ownership, and pushes parameters into scope.  That method would be a natural place for your call to FinalizeVarWithDestructor.

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