Message ID | 4E8119C5.5030708@redhat.com |
---|---|
State | New |
Headers | show |
On Mon, Sep 26, 2011 at 7:33 PM, Jason Merrill <jason@redhat.com> wrote: > cxx_eval_logical_expression was assuming that a folded first operand of && > would be either boolean_true_node or boolean_false_node, but in fact it can > be a constant with a typedef of bool, which doesn't compare equal with ==. > So we should use tree_int_cst_equal to compare them instead. > > Tested x86_64-pc-linux-gnu, applying to trunk. > Thanks. It is weird though that GCC does not maintain a properly typed internal representation. -- Gaby
On 10/01/2011 08:05 PM, Gabriel Dos Reis wrote: > It is weird though that GCC does not maintain a properly typed > internal representation. Huh? Different typedefs need to be compared with same_type_p rather than ==. I don't see how that makes the representation not properly typed. Jason
On Sun, Oct 2, 2011 at 7:37 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/01/2011 08:05 PM, Gabriel Dos Reis wrote: >> >> It is weird though that GCC does not maintain a properly typed >> internal representation. > > Huh? Different typedefs need to be compared with same_type_p rather than > ==. I don't see how that makes the representation not properly typed. > The comment wasn't about comparison of typedefs -- the patch did not compare typedefs. *Value* representations should not change just because a type name was introduced via a typedef. In particular, in my opinion comparing for "true" or "false" should just be an equality test to boolean_true_node or boolean_false_nore, not a comparison of their integer representation. -- Gaby
On 10/02/2011 12:10 PM, Gabriel Dos Reis wrote: > The comment wasn't about comparison of typedefs -- the patch did not compare > typedefs. > *Value* representations should not change just because a type name was > introduced via a typedef. Values (and expressions in general) have types. If the types aren't ==, then neither are two equal values with those types. Are you suggesting we strip typedefs for constant values? Jason
On Sun, Oct 2, 2011 at 11:51 AM, Jason Merrill <jason@redhat.com> wrote: > On 10/02/2011 12:10 PM, Gabriel Dos Reis wrote: >> >> The comment wasn't about comparison of typedefs -- the patch did not >> compare >> typedefs. > >> *Value* representations should not change just because a type name was >> introduced via a typedef. > > Values (and expressions in general) have types. Yes, values have types. A value has exactly one type, not two, or three, etc, typedef notwithstanding. So a value representation should be unique too. > If the types aren't ==, then neither are two equal values with those types. Hmm, I do not think that follows. A value is unique. We do not have zillion boolean values "true". We only have one. So, when we reduce a well-formed boolean expression to a value, it should be either the boolean "true" or the "boolean "false". In my opinion, we should not be looking at the integer representation, we should just compare it (with ==) against one of the canonical boolean nodes. > Are you suggesting we strip typedefs for constant values? I guess it amounts to that, yes. For a given type, we should have only one node for a given value, without having to compare representations. -- Gaby
commit 75fff91977aadc3ba784553b881a8e5222308194 Author: Jason Merrill <jason@redhat.com> Date: Mon Sep 26 17:28:27 2011 -0400 PR c++/50508 * semantics.c (cxx_eval_logical_expression): Use tree_int_cst_equal rather than ==. diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c index 19ecbee..89c76d5 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -6680,9 +6680,9 @@ cxx_eval_logical_expression (const constexpr_call *call, tree t, allow_non_constant, addr, non_constant_p); VERIFY_CONSTANT (lhs); - if (lhs == bailout_value) + if (tree_int_cst_equal (lhs, bailout_value)) return lhs; - gcc_assert (lhs == continue_value); + gcc_assert (tree_int_cst_equal (lhs, continue_value)); r = cxx_eval_constant_expression (call, TREE_OPERAND (t, 1), allow_non_constant, addr, non_constant_p); VERIFY_CONSTANT (r); diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-typedef1.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-typedef1.C new file mode 100644 index 0000000..2719e3a --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-typedef1.C @@ -0,0 +1,11 @@ +// PR c++/50508 +// { dg-options -std=c++0x } + +template <class T> + struct integral_constant { + typedef T value_type; + constexpr operator value_type() { return true; } + }; + +static constexpr bool value = integral_constant<bool>() + && true;