This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
C++ PATCH: Bug in ?: operator
- To: egcs-patches at egcs dot cygnus dot com
- Subject: C++ PATCH: Bug in ?: operator
- From: Nathan Sidwell <nathan at acm dot org>
- Date: Thu, 15 Apr 1999 09:39:31 +0100
- CC: Gabriel Dos Reis <Gabriel dot Dos-Reis at cmla dot ens-cachan dot fr>
- Organization: University of Bristol
- Reply-To: nathan at compsci dot bristol dot ac dot uk
Hi,
here's a patch for http://egcs.cygnus.com/ml/egcs-bugs/1999-03/msg00883.html
from Gabriel Dos Reis. Normally the second and third operands of a conditional
promote to some common type, when one is void, different things happen,
(5.16/2)
-- if one is a throw, the other's type is used
-- if both are void, the result is void
-- otherwise we've violated a `shall' constraint
In addition, g++ attempts to offer the following extensions/warnings on enum
types
-- if both are the same enum, the result is the enum type (not the promoted
integral type)
-- if one is an enum and the other is not an integral type we warn
-- if both are different enums, we error
However, the behaviour of add_builtin_candidate (call.c), is such that only the
first of these cases occur, the latter two are stymied by add_builtin_candidate
not adding candidates of the form operator?(bool, enum, int) and
operator?(bool, enum A, enum B). As it is not possible to overload ? operator
(13.6/24), I believe it safe for add_builtin_candidate to simply add these
prototypes and let build_conditional_expr sort it out.
With this change to add_builtin_candidate, build_conditional_expr gets the
unpromoted enum operands and it can apply the following extensions/warnings
-- if both are the same enum, the result is that type.
-- if one is an enum and the other non-integral integral, warn and promote
-- if both are different enums, warn and promote to the common (integral) type.
The warnings are enabled by the extra_warnings flag.
When one type is void, we see if it is a THROW_EXPR. If it is, use the other
type. This brings us into conformance with 5.16/2, first option. Otherwise, if
one is void and one non-void, we ped warn and select void. This conforms to the
`shall' violation, but provides an extension -- if this is unacceptable, I'll
alter the patch.
Then to prevent the error Gabriel had, of converting non-void to void, in the
promotion of the operands, if it is promoted to void, we do it ourselves. I
don't use cp_convert, because this is an implicit void cast so we should point
out weirdness. I can't leave the operands unpromoted, because, if the condition
is a constant, we'll fold it out immediately (and lose its voidness). Perhaps
this should be abstracted out to an implicit cast to void function.
I adjusted build_modify_expr to cope with such COND_EXPRs in the LHS of an
assignment. It needs to convert things like `(i ? throw : j) = e'
into `(i ? throw : j = e)'
That sorts out the C++ frontend, but the changes tickle an ICE in the backend's
expand_expr (expr.c). In `i = c ? throw : j;', expand_expr attempts to
store_expr on both arms of the cond. Of course, the throw can't do that. So I
changed expand_expr to not call store_expr on conditional operands who's type
is void. Of course, at the moment, if one side is void, so is the other, and
such a tree never reach store_expr.
Attached is a patch and a test case derived from Gabriel's bug report. No new
regressions on the 19990412 snapshot.
The C++ standard only specifies that the throw expression is at the top level
of the conditional, not hidden inside a comma expression, so I don't trog down
inside the second or third operands looking for a trailing throw. This means
(flag ? throw x : 1)
is ok, and of type int, but
(flag ? (i++, throw x) : 1)
is bad (the patch pedwarns and makes it type void). You may or may not be
surprised by this.
Enjoy,
nathan
--
Dr Nathan Sidwell :: Computer Science Department :: Bristol University
You can up the bandwidth, but you can't up the speed of light
nathan@acm.org http://www.cs.bris.ac.uk/~nathan/ nathan@cs.bris.ac.uk
egcs/gcc/ChangeLog:
Wed Apr 14 16:52:56 BST 1999 Nathan Sidwell <nathan@acm.org>
* expr.c (expand_expr): Cope with COND_EXPRs with one
non-returning branch.
egcs/gcc/cp/ChangeLog:
Wed Apr 14 16:52:56 BST 1999 Nathan Sidwell <nathan@acm.org>
* call.c (add_builtin_candidate): Allow unpromoted enumeral
choices in COND_EXPRs.
* typeck.c (build_conditional_expr): Use same_type_p. Allow
mismatched enumerals to promote. Deal with throw in one branch.
As extension permit one side to be void. Don't die on converting
to void.
(build_modify_expr): Adjust for such COND_EXPRs on lhs.
Index: egcs/gcc/expr.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/expr.c,v
retrieving revision 1.137
diff -c -3 -p -r1.137 expr.c
*** expr.c 1999/04/09 01:35:36 1.137
--- expr.c 1999/04/14 15:52:27
*************** expand_expr (exp, target, tmode, modifie
*** 7664,7670 ****
jumpifnot (TREE_OPERAND (exp, 0), op0);
start_cleanup_deferral ();
! if (temp != 0)
store_expr (TREE_OPERAND (exp, 1), temp, 0);
else
expand_expr (TREE_OPERAND (exp, 1),
--- 7664,7674 ----
jumpifnot (TREE_OPERAND (exp, 0), op0);
start_cleanup_deferral ();
!
! /* One branch of the cond can be void, if it never returns. For
! example A ? throw : E */
! if (temp != 0
! && TREE_TYPE (TREE_OPERAND (exp, 1)) != void_type_node)
store_expr (TREE_OPERAND (exp, 1), temp, 0);
else
expand_expr (TREE_OPERAND (exp, 1),
*************** expand_expr (exp, target, tmode, modifie
*** 7675,7681 ****
emit_barrier ();
emit_label (op0);
start_cleanup_deferral ();
! if (temp != 0)
store_expr (TREE_OPERAND (exp, 2), temp, 0);
else
expand_expr (TREE_OPERAND (exp, 2),
--- 7679,7686 ----
emit_barrier ();
emit_label (op0);
start_cleanup_deferral ();
! if (temp != 0
! && TREE_TYPE (TREE_OPERAND (exp, 2)) != void_type_node)
store_expr (TREE_OPERAND (exp, 2), temp, 0);
else
expand_expr (TREE_OPERAND (exp, 2),
Index: egcs/gcc/cp/call.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/cp/call.c,v
retrieving revision 1.141
diff -c -3 -p -r1.141 call.c
*** call.c 1999/04/13 00:20:42 1.141
--- call.c 1999/04/14 15:52:29
*************** add_builtin_candidate (candidates, code,
*** 1677,1688 ****
flags |= LOOKUP_NO_TEMP_BIND;
/* Extension: Support ?: of enumeral type. Hopefully this will not
! be an extension for long. */
! if (TREE_CODE (type1) == ENUMERAL_TYPE && type1 == type2)
! break;
! else if (TREE_CODE (type1) == ENUMERAL_TYPE
! || TREE_CODE (type2) == ENUMERAL_TYPE)
! return candidates;
if (ARITHMETIC_TYPE_P (type1) && ARITHMETIC_TYPE_P (type2))
break;
if (TREE_CODE (type1) == TREE_CODE (type2)
--- 1677,1684 ----
flags |= LOOKUP_NO_TEMP_BIND;
/* Extension: Support ?: of enumeral type. Hopefully this will not
! be an extension for long. Let enumeral types through, so we can warn
! about mismatches later. */
if (ARITHMETIC_TYPE_P (type1) && ARITHMETIC_TYPE_P (type2))
break;
if (TREE_CODE (type1) == TREE_CODE (type2)
Index: egcs/gcc/cp/typeck.c
===================================================================
RCS file: /egcs/carton/cvsfiles/egcs/gcc/cp/typeck.c,v
retrieving revision 1.151
diff -c -3 -p -r1.151 typeck.c
*** typeck.c 1999/04/13 00:20:39 1.151
--- typeck.c 1999/04/14 15:52:34
*************** build_conditional_expr (ifexp, op1, op2)
*** 5077,5083 ****
the same way. Return the unpromoted type and let the combined
value get promoted if necessary. */
! if (TYPE_MAIN_VARIANT (type1) == TYPE_MAIN_VARIANT (type2)
&& code2 != ARRAY_TYPE
&& code2 != FUNCTION_TYPE
&& code2 != METHOD_TYPE)
--- 5077,5083 ----
the same way. Return the unpromoted type and let the combined
value get promoted if necessary. */
! if (same_type_p (TYPE_MAIN_VARIANT (type1), TYPE_MAIN_VARIANT (type2))
&& code2 != ARRAY_TYPE
&& code2 != FUNCTION_TYPE
&& code2 != METHOD_TYPE)
*************** build_conditional_expr (ifexp, op1, op2)
*** 5118,5141 ****
return result;
}
! /* They don't match; promote them both and then try to reconcile them.
! But don't permit mismatching enum types. */
! if (code1 == ENUMERAL_TYPE)
{
! if (code2 == ENUMERAL_TYPE)
{
! cp_error ("enumeral mismatch in conditional expression: `%T' vs `%T'",
type1, type2);
- return error_mark_node;
}
! else if (extra_warnings && ! IS_AGGR_TYPE_CODE (code2)
! && type2 != type_promotes_to (type1))
! warning ("enumeral and non-enumeral type in conditional expression");
! }
! else if (extra_warnings
! && code2 == ENUMERAL_TYPE && ! IS_AGGR_TYPE_CODE (code1)
! && type1 != type_promotes_to (type2))
! warning ("enumeral and non-enumeral type in conditional expression");
if (code1 != VOID_TYPE)
{
--- 5118,5148 ----
return result;
}
! /* They don't match; promote them both and then try to reconcile them. */
!
! if (extra_warnings && (code1 == ENUMERAL_TYPE || code2 == ENUMERAL_TYPE))
{
! /* Warn about mismatching enumerals, but let them through [over.built]
! 13.6/24. */
! if (code1 != ENUMERAL_TYPE)
! {
! if (! IS_AGGR_TYPE_CODE (code1) && type1 != type_promotes_to (type2))
! cp_warning ("`%#T' and non-enumeral type `%T' in conditional expression",
! type2, type1);
! }
! else if (code2 != ENUMERAL_TYPE)
! {
! if (! IS_AGGR_TYPE_CODE (code2) && type2 != type_promotes_to (type1))
! cp_warning ("`%#T' and non-enumeral type `%T' in conditional expression",
! type1, type2);
! }
! else
{
! /* They are both enumerals, but must be different. */
! cp_warning ("`%#T' and `%#T' enumeral mismatch in conditional expression",
type1, type2);
}
! }
if (code1 != VOID_TYPE)
{
*************** build_conditional_expr (ifexp, op1, op2)
*** 5185,5193 ****
}
else if (code1 == VOID_TYPE || code2 == VOID_TYPE)
{
! if (pedantic && (code1 != VOID_TYPE || code2 != VOID_TYPE))
! pedwarn ("ANSI C++ forbids conditional expr with only one void side");
! result_type = void_type_node;
}
else if (code1 == POINTER_TYPE && null_ptr_cst_p (op2))
result_type = qualify_type (type1, type2);
--- 5192,5211 ----
}
else if (code1 == VOID_TYPE || code2 == VOID_TYPE)
{
! /* If one side is void, it could be a throw, [expr.cond]5.16/2. */
! if (code1 == VOID_TYPE && code2 == VOID_TYPE)
! result_type = void_type_node;
! else if (code1 == VOID_TYPE && TREE_CODE (op1) == THROW_EXPR)
! result_type = type2;
! else if (code2 == VOID_TYPE && TREE_CODE (op2) == THROW_EXPR)
! result_type = type1;
! else
! {
! /* Otherwise it's wrong, but convert them both to void. */
! if (pedantic)
! pedwarn ("ANSI C++ forbids conditional expr with only one void side");
! result_type = void_type_node;
! }
}
else if (code1 == POINTER_TYPE && null_ptr_cst_p (op2))
result_type = qualify_type (type1, type2);
*************** build_conditional_expr (ifexp, op1, op2)
*** 5333,5343 ****
result_type = build_ptrmemfunc_type (result_type);
if (result_type != TREE_TYPE (op1))
! op1 = convert_for_initialization
! (NULL_TREE, result_type, op1, LOOKUP_NORMAL, "converting", NULL_TREE, 0);
if (result_type != TREE_TYPE (op2))
! op2 = convert_for_initialization
! (NULL_TREE, result_type, op2, LOOKUP_NORMAL, "converting", NULL_TREE, 0);
if (TREE_CODE (ifexp) == INTEGER_CST)
return integer_zerop (ifexp) ? op2 : op1;
--- 5351,5389 ----
result_type = build_ptrmemfunc_type (result_type);
if (result_type != TREE_TYPE (op1))
! {
! if (result_type == void_type_node)
! {
! /* We've already pedwarned, should that have been necessary. */
! op1 = require_complete_type_in_void (op1);
! if (op1 != error_mark_node)
! {
! if ((extra_warnings || warn_unused) && ! TREE_SIDE_EFFECTS (op1))
! warning ("second operand of void conditional has no effect");
! op1 = build1 (CONVERT_EXPR, result_type, op1);
! }
! }
! else if (code1 != VOID_TYPE)
! op1 = convert_for_initialization
! (NULL_TREE, result_type, op1, LOOKUP_NORMAL, "converting", NULL_TREE, 0);
! }
if (result_type != TREE_TYPE (op2))
! {
! if (result_type == void_type_node)
! {
! /* We've already pedwarned, should that have been necessary. */
! op2 = require_complete_type_in_void (op2);
! if (op2 != error_mark_node)
! {
! if ((extra_warnings || warn_unused) && ! TREE_SIDE_EFFECTS (op2))
! warning ("third operand of void conditional has no effect");
! op2 = build1 (CONVERT_EXPR, result_type, op2);
! }
! }
! else if (code2 != VOID_TYPE)
! op2 = convert_for_initialization
! (NULL_TREE, result_type, op2, LOOKUP_NORMAL, "converting", NULL_TREE, 0);
! }
if (TREE_CODE (ifexp) == INTEGER_CST)
return integer_zerop (ifexp) ? op2 : op1;
*************** build_modify_expr (lhs, modifycode, rhs)
*** 5913,5925 ****
{
/* Produce (a ? (b = rhs) : (c = rhs))
except that the RHS goes through a save-expr
! so the code to compute it is only emitted once. */
! tree cond
! = build_conditional_expr (TREE_OPERAND (lhs, 0),
! build_modify_expr (cp_convert (TREE_TYPE (lhs), TREE_OPERAND (lhs, 1)),
! modifycode, rhs),
! build_modify_expr (cp_convert (TREE_TYPE (lhs), TREE_OPERAND (lhs, 2)),
! modifycode, rhs));
if (cond == error_mark_node)
return cond;
/* Make sure the code to compute the rhs comes out
--- 5959,5982 ----
{
/* Produce (a ? (b = rhs) : (c = rhs))
except that the RHS goes through a save-expr
! so the code to compute it is only emitted once.
! It is possible that one of the cond arms is a THROW_EXPR,
! and the other not [expr.cond]/2. So only modify the non-void
! parts. */
! tree first = TREE_OPERAND (lhs, 1);
! tree second = TREE_OPERAND (lhs, 2);
! tree cond;
!
! if (TREE_TYPE (first) != void_type_node)
! first = build_modify_expr (cp_convert (TREE_TYPE (lhs), first),
! modifycode, rhs);
!
! if (TREE_TYPE (second) != void_type_node)
! second = build_modify_expr (cp_convert (TREE_TYPE (lhs), second),
! modifycode, rhs);
!
! cond = build_conditional_expr (TREE_OPERAND (lhs, 0),
! first, second);
if (cond == error_mark_node)
return cond;
/* Make sure the code to compute the rhs comes out
// Build don't link:
// Special g++ Options: -W -pedantic -ansi
// Copyright (C) 1999 Free Software Foundation, Inc.
// Contributed by Nathan Sidwell 11 Apr 1999 <nathan@acm.org>
// Derived from bug report from Gabriel Dos Reis
// <Gabriel.Dos-Reis@cmla.ens-cachan.fr>
// http://egcs.cygnus.com/ml/egcs-bugs/1999-03/msg00883.html
// conditional exprs have some funny rules when one of the types is void.
// [expr.cond] 5.16, make sure we do the right things
// We implement an extension, allowing one side to be void, check we
// pedantically moan.
// We have checks for mismatching enumerations, check we give them (they had
// got lost due to changes in add_builtin_candidate).
struct X {};
enum E1 {e1 = -1};
enum E2 {e2 = -1};
void fn(int i)
{
double d;
int j;
(i ? throw X() : j) = 1; // ok, int &
j = (i ? e1 : e2); // WARNING - mismatch
d = (i ? e1 : 1.0); // WARNING - mismatch
d = (i ? 1.0 : e2); // WARNING - mismatch
j = (i ? 1 : e2); // ok
j = (i ? e1 : 1); // ok
j = (i ? throw X() : 1); // ok, int
j = (i ? 1 : throw X()); // ok, int
(i ? throw X() : throw X()); // ok, void
(i ? i : j) = 1; // ok, int &
(i ? throw X() : j) = 1; // ok, int &
(i ? j : throw X()) = 1; // ok, int &
(i ? throw X() : throw X()) = 1; // ERROR - void
(i ? (void)1 : i++); // WARNING - ANSI forbids
(i ? i++ : (void)1); // WARNING - ANSI forbids
__extension__ (void)(i ? (void)1 : 1); // WARNING - no effect
__extension__ (void)(i ? 1 : (void)1); // WARNING - no effect
}