Message ID | alpine.DEB.2.20.9.1602031242530.1505@idea |
---|---|
State | New |
Headers | show |
On 02/03/2016 12:51 PM, Patrick Palka wrote: > + && (integer_minus_onep (lhs) > + || integer_minus_onep (rhs))) Let's use null_member_pointer_value_p here. Please add pointers to member functions to the testcase. Jason
On Thu, Feb 4, 2016 at 9:24 AM, Jason Merrill <jason@redhat.com> wrote: > On 02/03/2016 12:51 PM, Patrick Palka wrote: >> >> + && (integer_minus_onep (lhs) >> + || integer_minus_onep (rhs))) > > > Let's use null_member_pointer_value_p here. Done. > > Please add pointers to member functions to the testcase. This does not currently work properly because comparisons between distinct pointers to (non-virtual) member functions eventually perform a comparison between two distinct FUNCTION_DECLs, and fold_binary_loc currently does not fold away such comparisons. So any static_asserts that perform comparisons between distinct function pointers or between distinct pointers to (non-virtual) member functions fail with "non-constant condition for static assertion". (Comparisons between pointers to distinct virtual member functions _do_ work because in that case we are ultimately just comparing integer offsets, not FUNCTION_DECLs. And comparisons between equivalent pointers trivially work.) I think the problem may lie in symtab_node::equal_address_to, which apparently returns -1, instead of 0, for two obviously distinct FUNCTION_DECLs. Test case: #define SA(x) static_assert ((x), #x) void f (); void g (); struct X { void f (); void g (); }; void foo () { SA (&f != &g); // "non-constant expression" SA (&X::f != &X::g); // "non-constant expression" }
On 02/04/2016 10:32 AM, Patrick Palka wrote: > On Thu, Feb 4, 2016 at 9:24 AM, Jason Merrill <jason@redhat.com> wrote: >> On 02/03/2016 12:51 PM, Patrick Palka wrote: >>> >>> + && (integer_minus_onep (lhs) >>> + || integer_minus_onep (rhs))) >> >> >> Let's use null_member_pointer_value_p here. > > Done. > >> >> Please add pointers to member functions to the testcase. > > This does not currently work properly because comparisons between > distinct pointers to (non-virtual) member functions eventually perform > a comparison between two distinct FUNCTION_DECLs, and fold_binary_loc > currently does not fold away such comparisons. So any static_asserts > that perform comparisons between distinct function pointers or between > distinct pointers to (non-virtual) member functions fail with > "non-constant condition for static assertion". (Comparisons between > pointers to distinct virtual member functions _do_ work because in > that case we are ultimately just comparing integer offsets, not > FUNCTION_DECLs. And comparisons between equivalent pointers trivially > work.) > > I think the problem may lie in symtab_node::equal_address_to, which > apparently returns -1, instead of 0, for two obviously distinct > FUNCTION_DECLs. Right, because they might be defined as aliases elsewhere. But it looks like it should work if you define f and g. > Test case: > > #define SA(x) static_assert ((x), #x) > > void f (); > void g (); > > struct X { void f (); void g (); }; > > void > foo () > { > SA (&f != &g); // "non-constant expression" > SA (&X::f != &X::g); // "non-constant expression" > } >
On Thu, Feb 4, 2016 at 11:57 AM, Jason Merrill <jason@redhat.com> wrote: > On 02/04/2016 10:32 AM, Patrick Palka wrote: >> >> On Thu, Feb 4, 2016 at 9:24 AM, Jason Merrill <jason@redhat.com> wrote: >>> >>> On 02/03/2016 12:51 PM, Patrick Palka wrote: >>>> >>>> >>>> + && (integer_minus_onep (lhs) >>>> + || integer_minus_onep (rhs))) >>> >>> >>> >>> Let's use null_member_pointer_value_p here. >> >> >> Done. >> >>> >>> Please add pointers to member functions to the testcase. >> >> >> This does not currently work properly because comparisons between >> distinct pointers to (non-virtual) member functions eventually perform >> a comparison between two distinct FUNCTION_DECLs, and fold_binary_loc >> currently does not fold away such comparisons. So any static_asserts >> that perform comparisons between distinct function pointers or between >> distinct pointers to (non-virtual) member functions fail with >> "non-constant condition for static assertion". (Comparisons between >> pointers to distinct virtual member functions _do_ work because in >> that case we are ultimately just comparing integer offsets, not >> FUNCTION_DECLs. And comparisons between equivalent pointers trivially >> work.) >> >> I think the problem may lie in symtab_node::equal_address_to, which >> apparently returns -1, instead of 0, for two obviously distinct >> FUNCTION_DECLs. > > > Right, because they might be defined as aliases elsewhere. But it looks > like it should work if you define f and g. That makes sense. But even when I define f and g, the comparisons don't get folded away: #define SA(x) static_assert ((x), #x) void f () { } void g () { } struct X { int a, b; void f (); void g (); }; void X::f () { } void X::g () { } void foo () { SA (&f != &g); SA (&X::f != &X::g); } bool bar = &f != &g; Although the static_asserts fail, the initialization of the variable bar _does_ get folded to 1 during gimplification. When evaluating the static_asserts, symtab_node::equal_address_to again returns -1 but during gimplification the same call to symtab_node::equal_address_to returns 0. So probably the symbols are not fully defined yet in the symbol table at the point when we are evaluating the static_asserts.
On Thu, Feb 4, 2016 at 12:24 PM, Patrick Palka <patrick@parcs.ath.cx> wrote: > On Thu, Feb 4, 2016 at 11:57 AM, Jason Merrill <jason@redhat.com> wrote: >> On 02/04/2016 10:32 AM, Patrick Palka wrote: >>> >>> On Thu, Feb 4, 2016 at 9:24 AM, Jason Merrill <jason@redhat.com> wrote: >>>> >>>> On 02/03/2016 12:51 PM, Patrick Palka wrote: >>>>> >>>>> >>>>> + && (integer_minus_onep (lhs) >>>>> + || integer_minus_onep (rhs))) >>>> >>>> >>>> >>>> Let's use null_member_pointer_value_p here. >>> >>> >>> Done. >>> >>>> >>>> Please add pointers to member functions to the testcase. >>> >>> >>> This does not currently work properly because comparisons between >>> distinct pointers to (non-virtual) member functions eventually perform >>> a comparison between two distinct FUNCTION_DECLs, and fold_binary_loc >>> currently does not fold away such comparisons. So any static_asserts >>> that perform comparisons between distinct function pointers or between >>> distinct pointers to (non-virtual) member functions fail with >>> "non-constant condition for static assertion". (Comparisons between >>> pointers to distinct virtual member functions _do_ work because in >>> that case we are ultimately just comparing integer offsets, not >>> FUNCTION_DECLs. And comparisons between equivalent pointers trivially >>> work.) >>> >>> I think the problem may lie in symtab_node::equal_address_to, which >>> apparently returns -1, instead of 0, for two obviously distinct >>> FUNCTION_DECLs. >> >> >> Right, because they might be defined as aliases elsewhere. But it looks >> like it should work if you define f and g. > > That makes sense. But even when I define f and g, the comparisons > don't get folded away: > > #define SA(x) static_assert ((x), #x) > > void f () { } > void g () { } > > struct X { int a, b; void f (); void g (); }; > > void X::f () { } > void X::g () { } > > void > foo () > { > SA (&f != &g); > SA (&X::f != &X::g); > } > > bool bar = &f != &g; > > Although the static_asserts fail, the initialization of the variable > bar _does_ get folded to 1 during gimplification. When evaluating the > static_asserts, symtab_node::equal_address_to again returns -1 but > during gimplification the same call to symtab_node::equal_address_to > returns 0. So probably the symbols are not fully defined yet in the > symbol table at the point when we are evaluating the static_asserts. Indeed, symtab_node::equal_address_to returns 0 only if both symbols have their ->analyzed flag set. But cgraph_node::analyze() (which sets the analyzed flag) is called during finalization of the compilation unit -- obviously not in time for the static_asserts to get folded.
On 02/04/2016 12:24 PM, Patrick Palka wrote: > On Thu, Feb 4, 2016 at 11:57 AM, Jason Merrill <jason@redhat.com> wrote: >> On 02/04/2016 10:32 AM, Patrick Palka wrote: >>> >>> On Thu, Feb 4, 2016 at 9:24 AM, Jason Merrill <jason@redhat.com> wrote: >>>> >>>> On 02/03/2016 12:51 PM, Patrick Palka wrote: >>>>> >>>>> >>>>> + && (integer_minus_onep (lhs) >>>>> + || integer_minus_onep (rhs))) >>>> >>>> >>>> >>>> Let's use null_member_pointer_value_p here. >>> >>> >>> Done. >>> >>>> >>>> Please add pointers to member functions to the testcase. >>> >>> >>> This does not currently work properly because comparisons between >>> distinct pointers to (non-virtual) member functions eventually perform >>> a comparison between two distinct FUNCTION_DECLs, and fold_binary_loc >>> currently does not fold away such comparisons. So any static_asserts >>> that perform comparisons between distinct function pointers or between >>> distinct pointers to (non-virtual) member functions fail with >>> "non-constant condition for static assertion". (Comparisons between >>> pointers to distinct virtual member functions _do_ work because in >>> that case we are ultimately just comparing integer offsets, not >>> FUNCTION_DECLs. And comparisons between equivalent pointers trivially >>> work.) >>> >>> I think the problem may lie in symtab_node::equal_address_to, which >>> apparently returns -1, instead of 0, for two obviously distinct >>> FUNCTION_DECLs. >> >> >> Right, because they might be defined as aliases elsewhere. But it looks >> like it should work if you define f and g. > > That makes sense. But even when I define f and g, the comparisons > don't get folded away: > > #define SA(x) static_assert ((x), #x) > > void f () { } > void g () { } > > struct X { int a, b; void f (); void g (); }; > > void X::f () { } > void X::g () { } > > void > foo () > { > SA (&f != &g); > SA (&X::f != &X::g); > } > > bool bar = &f != &g; > > Although the static_asserts fail, the initialization of the variable > bar _does_ get folded to 1 during gimplification. When evaluating the > static_asserts, symtab_node::equal_address_to again returns -1 but > during gimplification the same call to symtab_node::equal_address_to > returns 0. So probably the symbols are not fully defined yet in the > symbol table at the point when we are evaluating the static_asserts. Aha. That seems wrong, but certainly not something to fix now. The patch is OK. Jason
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c index b076991..c5e6642 100644 --- a/gcc/cp/constexpr.c +++ b/gcc/cp/constexpr.c @@ -1593,7 +1593,7 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t, bool /*lval*/, bool *non_constant_p, bool *overflow_p) { - tree r; + tree r = NULL_TREE; tree orig_lhs = TREE_OPERAND (t, 0); tree orig_rhs = TREE_OPERAND (t, 1); tree lhs, rhs; @@ -1612,7 +1612,24 @@ cxx_eval_binary_expression (const constexpr_ctx *ctx, tree t, location_t loc = EXPR_LOCATION (t); enum tree_code code = TREE_CODE (t); tree type = TREE_TYPE (t); - r = fold_binary_loc (loc, code, type, lhs, rhs); + + if ((code == EQ_EXPR || code == NE_EXPR)) + { + bool is_code_eq = (code == EQ_EXPR); + + if (TREE_CODE (lhs) == PTRMEM_CST + && TREE_CODE (rhs) == PTRMEM_CST) + r = constant_boolean_node (cp_tree_equal (lhs, rhs) == is_code_eq, + type); + else if ((TREE_CODE (lhs) == PTRMEM_CST + || TREE_CODE (rhs) == PTRMEM_CST) + && (integer_minus_onep (lhs) + || integer_minus_onep (rhs))) + r = constant_boolean_node (code == NE_EXPR, type); + } + + if (r == NULL_TREE) + r = fold_binary_loc (loc, code, type, lhs, rhs); if (r == NULL_TREE) { if (lhs == orig_lhs && rhs == orig_rhs) diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C new file mode 100644 index 0000000..b1318c4 --- /dev/null +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C @@ -0,0 +1,17 @@ +// { dg-do compile { target c++11 } } + +#define SA(x) static_assert ((x), #x) + +struct X { int a, b; }; + +void +foo () +{ + SA (&X::a); + SA (&X::a == &X::a); + SA (!(&X::a != &X::a)); + SA (&X::a != &X::b); + SA (!(&X::a == &X::b)); + SA ((!&X::b) == 0); + SA (!(&X::b == 0)); +}