This is the mail archive of the
gcc-bugs@gcc.gnu.org
mailing list for the GCC project.
[Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
- From: "aoliva at redhat dot com" <gcc-bugzilla at gcc dot gnu dot org>
- To: gcc-bugs at gcc dot gnu dot org
- Date: 8 Mar 2005 23:23:50 -0000
- Subject: [Bug c++/19199] [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
- References: <20041230105911.19199.lars@trolltech.com>
- Reply-to: gcc-bugzilla at gcc dot gnu dot org
------- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-08 23:23 -------
Subject: Re: [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a reference to a temporary
On Mar 7, 2005, Roger Sayle <roger@eyesopen.com> wrote:
> For example, I believe that Alex's proposed solution to PR c++/19199
> isn't an appropriate fix. It's perfectly reasonable for fold to convert
> a C++ COND_EXPR into a MIN_EXPR or MAX_EXPR, as according to the C++
> front-end all three of these tree nodes are valid lvalues. Hence it's
> not this transformation in fold that's in error.
Bugzilla was kept out of the long discussion that ensued, so I'll try
to summarize. The problem is that the conversion is turning a
COND_EXPR such as:
((int)a < (int)b ? a : b)
into
(__typeof(a)) ((int)a <? (int)b)
which is not an lvalue. This is the transformation we have to avoid.
> Simply disabling the COND_EXPR -> MIN_EXPR/MAX_EXPR transformation is
> also likely to be a serious performance penalty, especially on targets
> that support efficient sequences for "min" and "max".
This was not what I intended to do with my patch, FWIW.
Unfortunately, I goofed in the added call to operand_equal_p, limiting
too much the situations in which the optimization could be applied.
The patch fixes this problem, and updates the patch such that it
applies cleanly again.
As for other languages whose COND_EXPRs can't be lvalues, we can
probably arrange quite easily for them to ensure at least one of their
result operands is not an lvalue, so as to enable the transformation
again.
Comments? Ok to install?
Index: gcc/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_ternary): Use it to avoid turning a COND_EXPR lvalue into
a MIN_EXPR rvalue.
Index: gcc/fold-const.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/fold-const.c,v
retrieving revision 1.535
diff -u -p -r1.535 fold-const.c
--- gcc/fold-const.c 7 Mar 2005 21:24:21 -0000 1.535
+++ gcc/fold-const.c 8 Mar 2005 22:07:52 -0000
@@ -2005,16 +2005,12 @@ fold_convert (tree type, tree arg)
}
}
-/* Return an expr equal to X but certainly not valid as an lvalue. */
+/* Return false if expr can be assumed to not be an lvalue, true
+ otherwise. */
-tree
-non_lvalue (tree x)
+static bool
+maybe_lvalue_p (tree x)
{
- /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
- us. */
- if (in_gimple_form)
- return x;
-
/* We only need to wrap lvalue tree codes. */
switch (TREE_CODE (x))
{
@@ -2054,8 +2050,24 @@ non_lvalue (tree x)
/* Assume the worst for front-end tree codes. */
if ((int)TREE_CODE (x) >= NUM_TREE_CODES)
break;
- return x;
+ return false;
}
+
+ return true;
+}
+
+/* Return an expr equal to X but certainly not valid as an lvalue. */
+
+tree
+non_lvalue (tree x)
+{
+ /* While we are in GIMPLE, NON_LVALUE_EXPR doesn't mean anything to
+ us. */
+ if (in_gimple_form)
+ return x;
+
+ if (! maybe_lvalue_p (x))
+ return x;
return build1 (NON_LVALUE_EXPR, TREE_TYPE (x), x);
}
@@ -9734,10 +9746,16 @@ fold_ternary (tree expr)
of B and C. Signed zeros prevent all of these transformations,
for reasons given above each one.
+ We don't want to use operand_equal_for_comparison_p here,
+ because it might turn an lvalue COND_EXPR into an rvalue one,
+ see PR c++/19199.
+
Also try swapping the arguments and inverting the conditional. */
if (COMPARISON_CLASS_P (arg0)
- && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
- arg1, TREE_OPERAND (arg0, 1))
+ && ((maybe_lvalue_p (op1) && maybe_lvalue_p (op2))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), op1, 0)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+ arg1, TREE_OPERAND (arg0, 1)))
&& !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (arg1))))
{
tem = fold_cond_expr_with_comparison (type, arg0, op1, op2);
@@ -9746,9 +9764,10 @@ fold_ternary (tree expr)
}
if (COMPARISON_CLASS_P (arg0)
- && operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
- op2,
- TREE_OPERAND (arg0, 1))
+ && ((maybe_lvalue_p (op1) && maybe_lvalue_p (op2))
+ ? operand_equal_p (TREE_OPERAND (arg0, 0), op2, 0)
+ : operand_equal_for_comparison_p (TREE_OPERAND (arg0, 0),
+ op2, TREE_OPERAND (arg0, 1)))
&& !HONOR_SIGNED_ZEROS (TYPE_MODE (TREE_TYPE (op2))))
{
tem = invert_truthvalue (arg0);
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR c++/19199
* g++.dg/expr/lval2.C: New.
Index: gcc/testsuite/g++.dg/expr/lval2.C
===================================================================
RCS file: gcc/testsuite/g++.dg/expr/lval2.C
diff -N gcc/testsuite/g++.dg/expr/lval2.C
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/expr/lval2.C 8 Mar 2005 22:08:07 -0000
@@ -0,0 +1,26 @@
+// PR c++/19199
+
+// { dg-do run }
+
+// We used to turn the COND_EXPR lvalue into a MIN_EXPR rvalue, and
+// then return a reference to a temporary in qMin.
+
+#include <assert.h>
+
+enum Foo { A, B };
+
+template<typename T> T &qMin(T &a, T &b)
+{
+ return a < b ? a : b;
+}
+
+int main (int, char **)
+{
+ Foo f = A;
+ Foo g = B;
+ Foo &h = qMin(f, g);
+ assert (&h == &f || &h == &g);
+ const Foo &i = qMin((const Foo&)f, (const Foo&)g);
+ assert (&i == &f || &i == &g);
+ return 0;
+}
Index: gcc/testsuite/g++.old-deja/g++.oliva/ChangeLog
from Alexandre Oliva <aoliva@redhat.com>
PR c++/19199
* expr2.C: Fixed.
Index: gcc/testsuite/g++.old-deja/g++.oliva/expr2.C
===================================================================
RCS file: /cvs/gcc/gcc/gcc/testsuite/g++.old-deja/g++.oliva/expr2.C,v
retrieving revision 1.5
diff -u -p -r1.5 expr2.C
--- gcc/testsuite/g++.old-deja/g++.oliva/expr2.C 1 May 2003 02:02:47 -0000 1.5
+++ gcc/testsuite/g++.old-deja/g++.oliva/expr2.C 8 Mar 2005 22:08:07 -0000
@@ -1,4 +1,4 @@
-// { dg-do run { xfail *-*-* } }
+// { dg-do run }
// Copyright (C) 2000 Free Software Foundation
--
Alexandre Oliva http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist oliva@{lsd.ic.unicamp.br, gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199