r256996 - Properly bind up any cleanups in an ExprWithCleanups after

[John McCall via cfe-commits]

> On Jan 6, 2016, at 3:54 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> On Wed, Jan 6, 2016 at 3:34 PM, John McCall via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
> Author: rjmccall
> Date: Wed Jan  6 17:34:20 2016
> New Revision: 256996
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=256996&view=rev <http://llvm.org/viewvc/llvm-project?rev=256996&view=rev>
> Log:
> Properly bind up any cleanups in an ExprWithCleanups after
> instantiating a default argument expression.
> 
> This was previously just working implicitly by reinstantiating
> in the current context, but caching means that we weren't
> registering cleanups in subsequent uses.
> 
> Isn't the code you changed below only reached the first time anyway, now that we only instantiate the default argument once?

Yes.  The difference is that Param->getInit() will now be wrapped in an
ExprWithCleanups (assuming it does contain cleanups).  This means that on
subsequent calls, the standard logic beneath this will now flag the existence
of those cleanups, ultimately causing the containing full-expression to also
get wrapped.  That’s what wasn’t happening before, causing cleanups to
escape into the surrounding context, which apparently was only checked
by some OpenMP tests.

(CXXDefaultArgExpr::getExpr() looks through the ExprWithCleanups, and
that’s what’s used by CodeGen and other clients.)

John.


>  
> Modified:
>     cfe/trunk/lib/Sema/SemaExpr.cpp
> 
> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=256996&r1=256995&r2=256996&view=diff <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=256996&r1=256995&r2=256996&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Wed Jan  6 17:34:20 2016
> @@ -4313,17 +4313,16 @@ ExprResult Sema::BuildCXXDefaultArgExpr(
>      if (Result.isInvalid())
>        return ExprError();
> 
> -    Expr *Arg = Result.getAs<Expr>();
> -    CheckCompletedExpr(Arg, Param->getOuterLocStart());
> +    Result = ActOnFinishFullExpr(Result.getAs<Expr>(),
> +                                 Param->getOuterLocStart());
> +    if (Result.isInvalid())
> +      return ExprError();
> 
>      // Remember the instantiated default argument.
> -    Param->setDefaultArg(Arg);
> +    Param->setDefaultArg(Result.getAs<Expr>());
>      if (ASTMutationListener *L = getASTMutationListener()) {
>        L->DefaultArgumentInstantiated(Param);
>      }
> -
> -    // Build the default argument expression.
> -    return CXXDefaultArgExpr::Create(Context, CallLoc, Param);
>    }
> 
>    // If the default expression creates temporaries, we need to
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160106/28e03e17/attachment-0001.html>