C++ PATCHes relating to c++/48834, c++/40975 (array new)

C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 801 bytes --]

So, the problem in 48834 was that we had specified a particular target 
for the vec initialization, but it was getting clobbered by ending up on 
the rhs of an INIT_EXPR.  So 48834.patch avoids that.

But Diego doesn't think there was any real reason to abort on trying to 
copy a STATEMENT_LIST, so it seems to me that we could revert my earlier 
patch for 40975 and just add support for copying STATEMENT_LIST.  So 
40975-2.patch adds that support.  I haven't attached a patch to revert 
my earlier 40975 patch.

Some of the incidental changes from my earlier patch seem independently 
useful, so I'm reapplying those in vec-sfinae.patch.

Tested x86_64-pc-linux-gnu, applying to trunk.  Also applying 
40975-2.patch to 4.4, 4.5 and 4.6; it doesn't fix the bug in 4.3 so I'm 
not applying it there.

[-- Attachment #2: 48834.patch --]
[-- Type: text/plain, Size: 1557 bytes --]

commit 41d391e1683e6ed7e28ad31f41732b3f4b691baa
Author: Jason Merrill <jason@redhat.com>
Date:   Mon May 2 01:36:01 2011 -0400

    	PR c++/48834
    	* tree.c (build_vec_init_expr): Set TREE_SIDE_EFFECTS.
    	Protect an explicit target.

diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index 2eaa1db..fff3fbf 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -564,6 +564,7 @@ build_vec_init_expr (tree target, tree init, tree nelts,
     elt_init = build_vec_init_elt (type, init, complain);
 
   init = build3 (VEC_INIT_EXPR, type, slot, init, nelts);
+  TREE_SIDE_EFFECTS (init) = true;
   SET_EXPR_LOCATION (init, input_location);
 
   if (cxx_dialect >= cxx0x
@@ -571,7 +572,11 @@ build_vec_init_expr (tree target, tree init, tree nelts,
     VEC_INIT_EXPR_IS_CONSTEXPR (init) = true;
   VEC_INIT_EXPR_VALUE_INIT (init) = value_init;
 
-  if (slot != target)
+  if (slot == target)
+    /* If we specified what array we're initializing, make sure
+       we don't override that in cp_gimplify_init_expr.  */
+    init = cp_build_compound_expr (init, slot, complain);
+  else
     {
       init = build_target_expr (slot, init, complain);
       TARGET_EXPR_IMPLICIT_P (init) = 1;
diff --git a/gcc/testsuite/g++.dg/init/new31.C b/gcc/testsuite/g++.dg/init/new31.C
new file mode 100644
index 0000000..33c94aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/init/new31.C
@@ -0,0 +1,18 @@
+// PR c++/48834
+// { dg-options -Wuninitialized }
+// { dg-do run }
+
+struct S
+{
+  S ():i (0)
+  {
+  }
+  int i;
+};
+
+int
+main ()
+{
+  S *s = new S[2];
+  return 0;
+}

[-- Attachment #3: 40975-2.patch --]
[-- Type: text/plain, Size: 1279 bytes --]

commit 98011319607130da27331a5f1b763ba8ce741734
Author: Jason Merrill <jason@redhat.com>
Date:   Mon May 2 16:02:29 2011 -0400

    	PR c++/40975
    	* tree-inline.c (copy_tree_r): Handle STATEMENT_LIST.

diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 5da4a12..3777675 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4271,14 +4271,26 @@ copy_tree_r (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
 					 CONSTRUCTOR_ELTS (*tp));
       *tp = new_tree;
     }
+  else if (code == STATEMENT_LIST)
+    {
+      /* We used to just abort on STATEMENT_LIST, but we can run into them
+         with statement-expressions (c++/40975).  */
+      tree new_list = alloc_stmt_list ();
+      tree_stmt_iterator i = tsi_start (*tp);
+      tree_stmt_iterator j = tsi_last (new_list);
+      for (; !tsi_end_p (i); tsi_next (&i))
+	{
+	  tree stmt = tsi_stmt (i);
+	  tsi_link_after (&j, stmt, TSI_CONTINUE_LINKING);
+	}
+      *tp = new_list;
+    }
   else if (TREE_CODE_CLASS (code) == tcc_type)
     *walk_subtrees = 0;
   else if (TREE_CODE_CLASS (code) == tcc_declaration)
     *walk_subtrees = 0;
   else if (TREE_CODE_CLASS (code) == tcc_constant)
     *walk_subtrees = 0;
-  else
-    gcc_assert (code != STATEMENT_LIST);
   return NULL_TREE;
 }
 

[-- Attachment #4: vec-sfinae.patch --]
[-- Type: text/plain, Size: 4822 bytes --]

commit 6f60af2d7324b81f4b524c34c321280e4874c2ee
Author: Jason Merrill <jason@redhat.com>
Date:   Mon May 2 17:02:10 2011 -0400

    	* tree.c (build_vec_init_expr): Take complain parm.
    	(build_vec_init_elt): Likewise.  Free arg vector.
    	(diagnose_non_constexpr_vec_init, build_array_copy): Adjust.
    	* cp-tree.h (VEC_INIT_EXPR_SLOT): Use VEC_INIT_EXPR_CHECK.
    	(VEC_INIT_EXPR_INIT): Likewise.
    	Adjust build_vec_init_expr declaration.
    	* init.c (perform_member_init): Adjust.

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 9bad404..961581e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -2896,8 +2896,8 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter)
        (arg) = next_aggr_init_expr_arg (&(iter)))
 
 /* VEC_INIT_EXPR accessors.  */
-#define VEC_INIT_EXPR_SLOT(NODE) TREE_OPERAND (NODE, 0)
-#define VEC_INIT_EXPR_INIT(NODE) TREE_OPERAND (NODE, 1)
+#define VEC_INIT_EXPR_SLOT(NODE) TREE_OPERAND (VEC_INIT_EXPR_CHECK (NODE), 0)
+#define VEC_INIT_EXPR_INIT(NODE) TREE_OPERAND (VEC_INIT_EXPR_CHECK (NODE), 1)
 
 /* Indicates that a VEC_INIT_EXPR is a potential constant expression.
    Only set when the current function is constexpr.  */
@@ -5420,7 +5420,7 @@ extern tree get_target_expr_sfinae		(tree, tsubst_flags_t);
 extern tree build_cplus_array_type		(tree, tree);
 extern tree build_array_of_n_type		(tree, int);
 extern tree build_array_copy			(tree);
-extern tree build_vec_init_expr			(tree, tree);
+extern tree build_vec_init_expr			(tree, tree, tsubst_flags_t);
 extern void diagnose_non_constexpr_vec_init	(tree);
 extern tree hash_tree_cons			(tree, tree, tree);
 extern tree hash_tree_chain			(tree, tree);
diff --git a/gcc/cp/init.c b/gcc/cp/init.c
index 50dbcc9..7a7379e 100644
--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -506,7 +506,7 @@ perform_member_init (tree member, tree init)
       /* mem() means value-initialization.  */
       if (TREE_CODE (type) == ARRAY_TYPE)
 	{
-	  init = build_vec_init_expr (type, init);
+	  init = build_vec_init_expr (type, init, tf_warning_or_error);
 	  init = build2 (INIT_EXPR, type, decl, init);
 	  finish_expr_stmt (init);
 	}
@@ -543,7 +543,7 @@ perform_member_init (tree member, tree init)
 	      || same_type_ignoring_top_level_qualifiers_p (type,
 							    TREE_TYPE (init)))
 	    {
-	      init = build_vec_init_expr (type, init);
+	      init = build_vec_init_expr (type, init, tf_warning_or_error);
 	      init = build2 (INIT_EXPR, type, decl, init);
 	      finish_expr_stmt (init);
 	    }
diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c
index dfd11ad..0f2f86c 100644
--- a/gcc/cp/tree.c
+++ b/gcc/cp/tree.c
@@ -475,7 +475,7 @@ build_cplus_new (tree type, tree init, tsubst_flags_t complain)
    another array to copy.  */
 
 static tree
-build_vec_init_elt (tree type, tree init)
+build_vec_init_elt (tree type, tree init, tsubst_flags_t complain)
 {
   tree inner_type = strip_array_types (type);
   VEC(tree,gc) *argvec;
@@ -485,7 +485,7 @@ build_vec_init_elt (tree type, tree init)
     /* No interesting initialization to do.  */
     return integer_zero_node;
   else if (init == void_type_node)
-    return build_value_init (inner_type, tf_warning_or_error);
+    return build_value_init (inner_type, complain);
 
   gcc_assert (init == NULL_TREE
 	      || (same_type_ignoring_top_level_qualifiers_p
@@ -499,9 +499,12 @@ build_vec_init_elt (tree type, tree init)
 	dummy = move (dummy);
       VEC_quick_push (tree, argvec, dummy);
     }
-  return build_special_member_call (NULL_TREE, complete_ctor_identifier,
+  init = build_special_member_call (NULL_TREE, complete_ctor_identifier,
 				    &argvec, inner_type, LOOKUP_NORMAL,
-				    tf_warning_or_error);
+				    complain);
+  release_tree_vector (argvec);
+
+  return init;
 }
 
 /* Return a TARGET_EXPR which expresses the initialization of an array to
@@ -509,11 +512,11 @@ build_vec_init_elt (tree type, tree init)
    from another array of the same type.  */
 
 tree
-build_vec_init_expr (tree type, tree init)
+build_vec_init_expr (tree type, tree init, tsubst_flags_t complain)
 {
   tree slot;
   bool value_init = false;
-  tree elt_init = build_vec_init_elt (type, init);
+  tree elt_init = build_vec_init_elt (type, init, complain);
 
   if (init == void_type_node)
     {
@@ -550,14 +553,14 @@ diagnose_non_constexpr_vec_init (tree expr)
   else
     init = VEC_INIT_EXPR_INIT (expr);
 
-  elt_init = build_vec_init_elt (type, init);
+  elt_init = build_vec_init_elt (type, init, tf_warning_or_error);
   require_potential_constant_expression (elt_init);
 }
 
 tree
 build_array_copy (tree init)
 {
-  return build_vec_init_expr (TREE_TYPE (init), init);
+  return build_vec_init_expr (TREE_TYPE (init), init, tf_warning_or_error);
 }
 
 /* Build a TARGET_EXPR using INIT to initialize a new temporary of the

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Eric Botcazou]

> But Diego doesn't think there was any real reason to abort on trying to
> copy a STATEMENT_LIST, so it seems to me that we could revert my earlier
> patch for 40975 and just add support for copying STATEMENT_LIST.  So
> 40975-2.patch adds that support.

FWIW this assertion caught an impressive number of bugs in Ada over the years 
related to COND_EXPR: when they are incorrectly shared, gimplifying them on 
one side creates a STATEMENT_LIST and stopped the copying on the other side.

I'm not sure you can copy statements if they have any side-effects; this looks 
quite dangerous to me.  Instead statement-expressions should be wrapped up in 
a SAVE_EXPR/TARGET_EXPR to protect them and prevent copying.

-- 
Eric Botcazou

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Jason Merrill]

On 05/02/2011 06:23 PM, Eric Botcazou wrote:
> I'm not sure you can copy statements if they have any side-effects; this looks
> quite dangerous to me.  Instead statement-expressions should be wrapped up in
> a SAVE_EXPR/TARGET_EXPR to protect them and prevent copying.

It sounds like Ada and C++ are using copy_tree_r in very different ways.

The use in C++ has to do with default arguments: we parse the expression 
at the point of the function declaration, but whenever we want to use 
the expression in a function call we need to make a deep copy of the 
expression.  In this case, we want to copy everything.

How is it used in Ada?

Jason

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Eric Botcazou]

> How is it used in Ada?

The front-end doesn't use it directly, it's only used through the gimplifier 
by the unsharing phase (unshare_body).  We also have statement expressions.

-- 
Eric Botcazou

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Jason Merrill]

On 05/03/2011 03:00 AM, Eric Botcazou wrote:
>> How is it used in Ada?
>
> The front-end doesn't use it directly, it's only used through the gimplifier
> by the unsharing phase (unshare_body).  We also have statement expressions.

In that case you wouldn't be affected by this patch; unshare_body uses 
mostly_copy_tree_r, which has its own special case for STATEMENT_LIST.

Jason

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Eric Botcazou]

> In that case you wouldn't be affected by this patch; unshare_body uses
> mostly_copy_tree_r, which has its own special case for STATEMENT_LIST.

Right, I added it precisely to support statement expressions in Ada (instead 
of changing copy_tree_r) since we never want to copy them in the unsharing 
phase.  That's why I find the change to copy_tree_r questionable.

-- 
Eric Botcazou

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Jason Merrill]

On 05/03/2011 11:52 AM, Eric Botcazou wrote:
>> In that case you wouldn't be affected by this patch; unshare_body uses
>> mostly_copy_tree_r, which has its own special case for STATEMENT_LIST.
>
> Right, I added it precisely to support statement expressions in Ada (instead
> of changing copy_tree_r) since we never want to copy them in the unsharing
> phase.  That's why I find the change to copy_tree_r questionable.

Well, let's look at the users of copy_tree_r.

cp/tree.c (bot_manip): The case I want to fix.

gimplify.c (mostly_copy_tree_r): Handles STATEMENT_LIST itself.

stor-layout.c (copy_self_referential_tree_r): Affected by the change. 
Would you like me to add a gcc_unreachable() here?

tree-inline.c (copy_tree_body_r): already copies STATEMENT_LIST itself 
(with a copy_statement_list function which I should use instead of 
open-coding it again).
(copy_bind_expr): subroutine of copy_tree_body_r.
(unsave_r, remap_gimple_op_r): Handle STATEMENT_LIST themselves.

So, looks like one place that could have an undesired change in 
behavior.  I'll go ahead and add that assert.

Jason

RFA Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 587 bytes --]

On 05/03/2011 01:28 PM, Jason Merrill wrote:
> stor-layout.c (copy_self_referential_tree_r): Affected by the change.
> Would you like me to add a gcc_unreachable() here?
>
> tree-inline.c (copy_tree_body_r): already copies STATEMENT_LIST itself
> (with a copy_statement_list function which I should use instead of
> open-coding it again).

Thus.  Tested x86_64-pc-linux-gnu with c,ada,c++,fortran,java,lto,objc. 
OK?  I also removed the recursion from copy_statement_list because it 
would just extra garbage STATEMENT_LISTs since they're already copied by 
the normal walk_tree.

Jason

[-- Attachment #2: 40975-3.patch --]
[-- Type: text/plain, Size: 2102 bytes --]

commit 85aad99f6848bfaebbe5c794bf0a95c80e0f49cd
Author: Jason Merrill <jason@redhat.com>
Date:   Tue May 3 13:19:10 2011 -0400

    	PR c++/40975
    	* tree-inline.c (copy_tree_r): Use copy_statement_list.
    	(copy_statement_list): Don't recurse.
    	* stor-layout.c (copy_self_referential_tree_r): Don't allow
    	STATEMENT_LIST.

diff --git a/gcc/stor-layout.c b/gcc/stor-layout.c
index ea0d44d..ecd1450 100644
--- a/gcc/stor-layout.c
+++ b/gcc/stor-layout.c
@@ -245,6 +245,8 @@ copy_self_referential_tree_r (tree *tp, int *walk_subtrees, void *data)
      cannot always guarantee in practice.  So punt in this case.  */
   else if (code == SAVE_EXPR)
     return error_mark_node;
+  else if (code == STATEMENT_LIST)
+    gcc_unreachable ();
 
   return copy_tree_r (tp, walk_subtrees, data);
 }
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index 3777675..ea7b7ab 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -662,8 +662,6 @@ copy_statement_list (tree *tp)
   for (; !tsi_end_p (oi); tsi_next (&oi))
     {
       tree stmt = tsi_stmt (oi);
-      if (TREE_CODE (stmt) == STATEMENT_LIST)
-	copy_statement_list (&stmt);
       tsi_link_after (&ni, stmt, TSI_CONTINUE_LINKING);
     }
 }
@@ -4272,19 +4270,9 @@ copy_tree_r (tree *tp, int *walk_subtrees, void *data ATTRIBUTE_UNUSED)
       *tp = new_tree;
     }
   else if (code == STATEMENT_LIST)
-    {
-      /* We used to just abort on STATEMENT_LIST, but we can run into them
-         with statement-expressions (c++/40975).  */
-      tree new_list = alloc_stmt_list ();
-      tree_stmt_iterator i = tsi_start (*tp);
-      tree_stmt_iterator j = tsi_last (new_list);
-      for (; !tsi_end_p (i); tsi_next (&i))
-	{
-	  tree stmt = tsi_stmt (i);
-	  tsi_link_after (&j, stmt, TSI_CONTINUE_LINKING);
-	}
-      *tp = new_list;
-    }
+    /* We used to just abort on STATEMENT_LIST, but we can run into them
+       with statement-expressions (c++/40975).  */
+    copy_statement_list (tp);
   else if (TREE_CODE_CLASS (code) == tcc_type)
     *walk_subtrees = 0;
   else if (TREE_CODE_CLASS (code) == tcc_declaration)

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Eric Botcazou]

> Well, let's look at the users of copy_tree_r.
>
> cp/tree.c (bot_manip): The case I want to fix.

Then I'd put the fix there.  The old behaviour of copy_tree_r is IMO the most 
sensible one and its callers should cope, as almost all already do it seems.

-- 
Eric Botcazou

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Jason Merrill]

On 05/04/2011 04:12 AM, Eric Botcazou wrote:
>> Well, let's look at the users of copy_tree_r.
>>
>> cp/tree.c (bot_manip): The case I want to fix.
>
> Then I'd put the fix there.  The old behaviour of copy_tree_r is IMO the most
> sensible one and its callers should cope, as almost all already do it seems.

Well, I disagree.  STATEMENT_LISTs are just another kind of thing you 
encounter in an expression; if a caller wants special handling, they can 
arrange for it.

This is how things used to work before, but it broke when the tree-ssa 
merge switched from using TREE_CHAIN on statements to a separate 
STATEMENT_LIST.

Jason

Re: C++ PATCHes relating to c++/48834, c++/40975 (array new)

[Eric Botcazou]

> Well, I disagree.  STATEMENT_LISTs are just another kind of thing you
> encounter in an expression; if a caller wants special handling, they can
> arrange for it.

But you're unilaterally choosing one special handling (copying) among several 
ones (copying, not copying, aborting) just because of one caller, for no good 
reason IMO.

> This is how things used to work before, but it broke when the tree-ssa
> merge switched from using TREE_CHAIN on statements to a separate
> STATEMENT_LIST.

Well, the assertion certainly wasn't put there by accident.

-- 
Eric Botcazou

copy_tree_r and STATEMENT_LIST (was Re: C++ PATCHes relating to c++/48834, c++/40975 (array new))

[Jason Merrill]

On 05/04/2011 06:57 PM, Eric Botcazou wrote:
> But you're unilaterally choosing one special handling (copying) among several
> ones (copying, not copying, aborting) just because of one caller, for no good
> reason IMO.

It seems pretty straightforward to me that a function named copy_tree_r 
should copy everything that isn't always shared (like decls).  It 
already copies SAVE_EXPR, after all; how is copying STATEMENT_LIST going 
to cause trouble in a context where copying SAVE_EXPR isn't?

>> This is how things used to work before, but it broke when the tree-ssa
>> merge switched from using TREE_CHAIN on statements to a separate
>> STATEMENT_LIST.
>
> Well, the assertion certainly wasn't put there by accident.

No, but the rationale isn't documented.  It was added by the patch that 
introduced STATEMENT_LIST,

   http://gcc.gnu.org/ml/gcc-patches/2003-11/msg00879.html

but doesn't appear in the ChangeLog entry.  I notice that in that patch 
unsave_r copied STATEMENT_LIST, but now it doesn't.  rth, do you happen 
to remember why you put it there?

Jason

Re: copy_tree_r and STATEMENT_LIST (was Re: C++ PATCHes relating to c++/48834, c++/40975 (array new))

[Richard Henderson]

On 05/05/2011 07:09 AM, Jason Merrill wrote:
> No, but the rationale isn't documented.  It was added by the patch
> that introduced STATEMENT_LIST,
> 
>   http://gcc.gnu.org/ml/gcc-patches/2003-11/msg00879.html
> 
> but doesn't appear in the ChangeLog entry. I notice that in that
> patch unsave_r copied STATEMENT_LIST, but now it doesn't. rth, do you
> happen to remember why you put it there?

No, I don't recall.  I suspect that I put that in there while testing
in order to determine if I needed to support copying statement lists,
and it turned out that I didn't.


r~

Re: copy_tree_r and STATEMENT_LIST (was Re: C++ PATCHes relating to c++/48834, c++/40975 (array new))

[Eric Botcazou]

> It seems pretty straightforward to me that a function named copy_tree_r
> should copy everything that isn't always shared (like decls).  It
> already copies SAVE_EXPR, after all; how is copying STATEMENT_LIST going
> to cause trouble in a context where copying SAVE_EXPR isn't?

OK, this can make sense, callers should handle special nodes like SAVE_EXPR, 
TARGET_EXPR, STATEMENT_LIST, etc themselves.  In light of this, they need to 
be audited and adjusted, as you did already a few days ago.  So I think I can 
live with your 40975-3.patch in the end.

Thanks for your patience.

-- 
Eric Botcazou

PATCH to copy_statement_list for c++/47999 (ICE in testsuite on darwin)

[Jason Merrill]

[-- Attachment #1: Type: text/plain, Size: 380 bytes --]

On 05/03/2011 02:18 PM, Jason Merrill wrote:
> I also removed the recursion from copy_statement_list because it
> would just extra garbage STATEMENT_LISTs since they're already copied by
> the normal walk_tree.

I was wrong about this, the recursion is necessary because 
tsi_link_after destroys STATEMENT_LISTs.

Applying as obvious since it just restores code I removed before.

[-- Attachment #2: 47999.patch --]
[-- Type: text/x-patch, Size: 1674 bytes --]

commit 79330512f8ade4ecff47ab8cb8c050440d9648e0
Author: Jason Merrill <jason@redhat.com>
Date:   Tue Mar 8 08:46:41 2011 -0500

    	PR c++/47999
    	* semantics.c (finish_call_expr): Preserve reference semantics
    	in templates.

diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 53497f3..ce24d46 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -2150,11 +2150,17 @@ finish_call_expr (tree fn, VEC(tree,gc) **args, bool disallow_virtual,
     /* A call where the function is unknown.  */
     result = cp_build_function_call_vec (fn, args, complain);
 
-  if (processing_template_decl)
+  if (processing_template_decl && result != error_mark_node)
     {
+      if (TREE_CODE (result) == INDIRECT_REF)
+	result = TREE_OPERAND (result, 0);
+      gcc_assert (TREE_CODE (result) == CALL_EXPR
+		  || TREE_CODE (fn) == PSEUDO_DTOR_EXPR
+		  || errorcount);
       result = build_call_vec (TREE_TYPE (result), orig_fn, orig_args);
       KOENIG_LOOKUP_P (result) = koenig_p;
       release_tree_vector (orig_args);
+      result = convert_from_reference (result);
     }
 
   return result;
diff --git a/gcc/testsuite/g++.dg/cpp0x/auto22.C b/gcc/testsuite/g++.dg/cpp0x/auto22.C
new file mode 100644
index 0000000..66630e5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/auto22.C
@@ -0,0 +1,21 @@
+// PR c++/47999
+// { dg-options -std=c++0x }
+
+int& identity(int& i)
+{
+  return i;
+}
+
+// In a function template, auto type deduction works incorrectly.
+template <typename = void>
+void f()
+{
+  int i = 0;
+  auto&& x = identity(i); // Type of x should be `int&`, but it is `int&&`.
+}
+
+int main (int argc, char* argv[])
+{
+  f();
+  return 0;
+}