[cfe-commits] r155893 - in /cfe/trunk: lib/Sema/SemaTemplateInstantiate.cpp test/CodeGenCXX/destructors.cpp test/SemaCXX/conversion.cpp

[Andrew Trick]

On Apr 30, 2012, at 11:05 PM, David Blaikie <dblaikie at gmail.com> wrote:

> Author: dblaikie
> Date: Tue May  1 01:05:57 2012
> New Revision: 155893
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=155893&view=rev
> Log:
> PR12710 - broken default argument handling for templates.
> 
> I broke this in r155838 by not actually instantiating non-dependent default arg
> expressions. The motivation for that change was to avoid producing duplicate
> conversion warnings for such default args (we produce them once when we parse
> the template - there's no need to produce them at each instantiation) but
> without actually instantiating the default arg, things break in weird ways.
> 
> Technically, I think we could still get the right diagnostic experience without
> the bugs if we instantiated the non-dependent args (for non-dependent params
> only) immediately, rather than lazily. But I'm not sure if such a refactoring/
> change would be desirable so here's the conservative fix for now.
> 
> Modified:
>    cfe/trunk/lib/Sema/SemaTemplateInstantiate.cpp
>    cfe/trunk/test/CodeGenCXX/destructors.cpp
>    cfe/trunk/test/SemaCXX/conversion.cpp


David,

Would you mind reverting the corresponding change in SemaExpr? This change results in miscompilation of 483.xalancbmk. The miscompilation depends on the host compiler used to build clang, and I haven't determined the reason for it, but I do need to get tree in a healthy state ASAP. If you're sure this change is safe, then I can keep a bug open.

-Andy

--- a/lib/Sema/SemaExpr.cpp
+++ b/lib/Sema/SemaExpr.cpp
@@ -3300,11 +3300,9 @@ ExprResult Sema::BuildCXXDefaultArgExpr(SourceLocation CallLoc,
     if (Result.isInvalid())
       return ExprError();

-    Expr *Arg = Result.takeAs<Expr>();
-    CheckImplicitConversions(Arg, Arg->getExprLoc());
-    Param->setDefaultArg(Arg);
     // Build the default argument expression.
-    return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param, Arg));
+    return Owned(CXXDefaultArgExpr::Create(Context, CallLoc, Param,
+                                           Result.takeAs<Expr>()));
   }

   // If the default expression creates temporaries, we need to