------- Additional Comments From aoliva at gcc dot gnu dot org 2005-03-09
04:11 -------
Subject: Re: [3.3/3.4/4.0/4.1 Regression] Wrong warning about returning a
reference to a temporary
On Mar 8, 2005, Roger Sayle <[EMAIL PROTECTED]> wrote:
> On 8 Mar 2005, Alexandre Oliva wrote:
>>
>> * 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.
> This version is Ok for mainline, and currently open release branches.
Unfortunately, the problem in g++.oliva/expr2.C resurfaced, because,
as it turns out, the transformation (I != J ? I : J) => I yields an
lvalue as required, but not the *correct* lvalue in all cases. I
tried what you'd suggested on IRC (testing whether the thing is an
lvalue after the fact), but that obviously failed in this case as
well.
So I figured a better approach would be to perform lvalue tests to
fold_cond_expr_with_comparison(), where we can avoid specific
inappropriate transformations while still trying other alternatives.
While at that, I figured we could use pedantic_lvalues to avoid
refraining from applying optimizations just because it looks like the
cond-expr might be an lvalue, even though the language requires it not
to be.
This is currently bootstrapping on x86_64-linux-gnu. Ok to install if
bootstrap completes and regtesting passes?
Index: gcc/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
PR c++/19199
* fold-const.c (non_lvalue): Split tests into...
(maybe_lvalue_p): New function.
(fold_cond_expr_with_comparison): Preserve lvalue-ness.
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 9 Mar 2005 03:59:18 -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);
}
@@ -4162,7 +4174,15 @@ fold_cond_expr_with_comparison (tree typ
tree arg00 = TREE_OPERAND (arg0, 0);
tree arg01 = TREE_OPERAND (arg0, 1);
tree arg1_type = TREE_TYPE (arg1);
- tree tem;
+ tree tem = NULL;
+ /* If the COND_EXPR can possibly be an lvalue, we don't want to
+ perform transformations that return a simplified result that will
+ be recognized as lvalue, but that will not match the expected
+ result. We may still return other expressions that would be
+ incorrect, but those are going to be rvalues, and the caller is
+ supposed to discard them. */
+ bool lvalue = !pedantic_lvalues
+ && maybe_lvalue_p (arg1) && maybe_lvalue_p (arg2);
STRIP_NOPS (arg1);
STRIP_NOPS (arg2);
@@ -4196,10 +4216,12 @@ fold_cond_expr_with_comparison (tree typ
case EQ_EXPR:
case UNEQ_EXPR:
tem = fold_convert (arg1_type, arg1);
- return pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ tem = pedantic_non_lvalue (fold_convert (type, negate_expr (tem)));
+ break;
case NE_EXPR:
case LTGT_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
+ break;
case UNGE_EXPR:
case UNGT_EXPR:
if (flag_trapping_math)
@@ -4211,7 +4233,8 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold (build1 (ABS_EXPR, TREE_TYPE (arg1), arg1));
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem = pedantic_non_lvalue (fold_convert (type, tem));
+ break;
case UNLE_EXPR:
case UNLT_EXPR:
if (flag_trapping_math)
@@ -4222,12 +4245,18 @@ fold_cond_expr_with_comparison (tree typ
arg1 = fold_convert (lang_hooks.types.signed_type
(TREE_TYPE (arg1)), arg1);
tem = fold (build1 (ABS_EXPR, TREE_TYPE (arg1), arg1));
- return negate_expr (fold_convert (type, tem));
+ tem = negate_expr (fold_convert (type, tem));
+ break;
default:
gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
break;
}
+ if (tem && (! lvalue || maybe_lvalue_p (tem)))
+ return tem;
+ else
+ tem = NULL;
+
/* A != 0 ? A : 0 is simply A, unless A is -0. Likewise
A == 0 ? A : 0 is always 0 unless A is -0. Note that
both transformations are correct when A is NaN: A != 0
@@ -4236,11 +4265,16 @@ fold_cond_expr_with_comparison (tree typ
if (integer_zerop (arg01) && integer_zerop (arg2))
{
if (comp_code == NE_EXPR)
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
else if (comp_code == EQ_EXPR)
- return fold_convert (type, integer_zero_node);
+ tem = fold_convert (type, integer_zero_node);
}
+ if (tem && (! lvalue || maybe_lvalue_p (tem)))
+ return tem;
+ else
+ tem = NULL;
+
/* Try some transformations of A op B ? A : B.
A == B? A : B same as B
@@ -4284,9 +4318,15 @@ fold_cond_expr_with_comparison (tree typ
switch (comp_code)
{
case EQ_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg2));
+ if (lvalue)
+ break;
+ tem = pedantic_non_lvalue (fold_convert (type, arg2));
+ break;
case NE_EXPR:
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ if (lvalue)
+ break;
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
+ break;
case LE_EXPR:
case LT_EXPR:
case UNLE_EXPR:
@@ -4302,7 +4342,7 @@ fold_cond_expr_with_comparison (tree typ
tem = (comp_code == LE_EXPR || comp_code == UNLE_EXPR)
? fold (build2 (MIN_EXPR, comp_type, comp_op0, comp_op1))
: fold (build2 (MIN_EXPR, comp_type, comp_op1, comp_op0));
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem = pedantic_non_lvalue (fold_convert (type, tem));
}
break;
case GE_EXPR:
@@ -4316,16 +4356,16 @@ fold_cond_expr_with_comparison (tree typ
tem = (comp_code == GE_EXPR || comp_code == UNGE_EXPR)
? fold (build2 (MAX_EXPR, comp_type, comp_op0, comp_op1))
: fold (build2 (MAX_EXPR, comp_type, comp_op1, comp_op0));
- return pedantic_non_lvalue (fold_convert (type, tem));
+ tem = pedantic_non_lvalue (fold_convert (type, tem));
}
break;
case UNEQ_EXPR:
- if (!HONOR_NANS (TYPE_MODE (TREE_TYPE (arg1))))
- return pedantic_non_lvalue (fold_convert (type, arg2));
+ if (! lvalue && !HONOR_NANS (TYPE_MODE (TREE_TYPE (arg1))))
+ tem = pedantic_non_lvalue (fold_convert (type, arg2));
break;
case LTGT_EXPR:
- if (!HONOR_NANS (TYPE_MODE (TREE_TYPE (arg1))))
- return pedantic_non_lvalue (fold_convert (type, arg1));
+ if (! lvalue && !HONOR_NANS (TYPE_MODE (TREE_TYPE (arg1))))
+ tem = pedantic_non_lvalue (fold_convert (type, arg1));
break;
default:
gcc_assert (TREE_CODE_CLASS (comp_code) == tcc_comparison);
@@ -4333,6 +4373,11 @@ fold_cond_expr_with_comparison (tree typ
}
}
+ if (tem && (! lvalue || maybe_lvalue_p (tem)))
+ return tem;
+ else
+ tem = NULL;
+
/* If this is A op C1 ? A : C2 with C1 and C2 constant integers,
we might still be able to simplify this. For example,
if C1 is one less or one more than C2, this might have started
@@ -4347,7 +4392,7 @@ fold_cond_expr_with_comparison (tree typ
case EQ_EXPR:
/* We can replace A with C1 in this case. */
arg1 = fold_convert (type, arg01);
- return fold (build3 (COND_EXPR, type, arg0, arg1, arg2));
+ tem = fold (build3 (COND_EXPR, type, arg0, arg1, arg2));
case LT_EXPR:
/* If C1 is C2 + 1, this is min(A, C2). */
@@ -4357,8 +4402,8 @@ fold_cond_expr_with_comparison (tree typ
const_binop (PLUS_EXPR, arg2,
integer_one_node, 0),
OEP_ONLY_CONST))
- return pedantic_non_lvalue (fold (build2 (MIN_EXPR,
- type, arg1, arg2)));
+ tem = pedantic_non_lvalue (fold (build2 (MIN_EXPR,
+ type, arg1, arg2)));
break;
case LE_EXPR:
@@ -4369,8 +4414,8 @@ fold_cond_expr_with_comparison (tree typ
const_binop (MINUS_EXPR, arg2,
integer_one_node, 0),
OEP_ONLY_CONST))
- return pedantic_non_lvalue (fold (build2 (MIN_EXPR,
- type, arg1, arg2)));
+ tem = pedantic_non_lvalue (fold (build2 (MIN_EXPR,
+ type, arg1, arg2)));
break;
case GT_EXPR:
@@ -4381,8 +4426,8 @@ fold_cond_expr_with_comparison (tree typ
const_binop (MINUS_EXPR, arg2,
integer_one_node, 0),
OEP_ONLY_CONST))
- return pedantic_non_lvalue (fold (build2 (MAX_EXPR,
- type, arg1, arg2)));
+ tem = pedantic_non_lvalue (fold (build2 (MAX_EXPR,
+ type, arg1, arg2)));
break;
case GE_EXPR:
@@ -4393,8 +4438,8 @@ fold_cond_expr_with_comparison (tree typ
const_binop (PLUS_EXPR, arg2,
integer_one_node, 0),
OEP_ONLY_CONST))
- return pedantic_non_lvalue (fold (build2 (MAX_EXPR,
- type, arg1, arg2)));
+ tem = pedantic_non_lvalue (fold (build2 (MAX_EXPR,
+ type, arg1, arg2)));
break;
case NE_EXPR:
break;
@@ -4402,6 +4447,11 @@ fold_cond_expr_with_comparison (tree typ
gcc_unreachable ();
}
+ if (tem && (! lvalue || maybe_lvalue_p (tem)))
+ return tem;
+ else
+ tem = NULL;
+
return NULL_TREE;
}
Index: gcc/testsuite/ChangeLog
from Alexandre Oliva <[EMAIL PROTECTED]>
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 9 Mar 2005 03:59:32 -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 <[EMAIL PROTECTED]>
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 9 Mar 2005 03:59:32 -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 [EMAIL PROTECTED], gcc.gnu.org}
Free Software Evangelist [EMAIL PROTECTED], gnu.org}
--
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199
