* [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
@ 2015-12-21 23:07 Patrick Palka
2016-01-11 3:19 ` Patrick Palka
2016-02-03 16:06 ` Jason Merrill
0 siblings, 2 replies; 11+ messages in thread
From: Patrick Palka @ 2015-12-21 23:07 UTC (permalink / raw)
To: gcc-patches; +Cc: jason, Patrick Palka
We are currently failing to fold equality comparisons involving
PTRMEM_CSTs since (I think) r229018 thus making this a GCC 6 regression.
This regression shows up in the boost testsuite.
Fixed in a straightforward way. OK to commit after bootstrap + regtest?
gcc/cp/ChangeLog:
* constexpr.c (cxx_eval_binary_expression): Expand
the PTRMEM_CST operands of an equality comparison.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-ptrmem5.C: New test.
---
gcc/cp/constexpr.c | 9 +++++++++
gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C | 14 ++++++++++++++
2 files changed, 23 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 208f43b..1ad87aa 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -1630,6 +1630,15 @@ 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);
+
+ if (code == EQ_EXPR || code == NE_EXPR)
+ {
+ if (TREE_CODE (lhs) == PTRMEM_CST && CONSTANT_CLASS_P (rhs))
+ lhs = cplus_expand_constant (lhs);
+ if (TREE_CODE (rhs) == PTRMEM_CST && CONSTANT_CLASS_P (lhs))
+ rhs = cplus_expand_constant (rhs);
+ }
+
r = fold_binary_loc (loc, code, type, lhs, rhs);
if (r == NULL_TREE)
{
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..e28c917
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C
@@ -0,0 +1,14 @@
+// { 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::b);
+ SA ((!&X::b) == 0);
+}
--
2.7.0.rc0.50.g1470d8f.dirty
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2015-12-21 23:07 [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members Patrick Palka
@ 2016-01-11 3:19 ` Patrick Palka
2016-01-25 0:13 ` Patrick Palka
2016-02-03 16:06 ` Jason Merrill
1 sibling, 1 reply; 11+ messages in thread
From: Patrick Palka @ 2016-01-11 3:19 UTC (permalink / raw)
To: GCC Patches; +Cc: Jason Merrill, Patrick Palka
On Mon, Dec 21, 2015 at 6:07 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> We are currently failing to fold equality comparisons involving
> PTRMEM_CSTs since (I think) r229018 thus making this a GCC 6 regression.
> This regression shows up in the boost testsuite.
>
> Fixed in a straightforward way. OK to commit after bootstrap + regtest?
>
> gcc/cp/ChangeLog:
>
> * constexpr.c (cxx_eval_binary_expression): Expand
> the PTRMEM_CST operands of an equality comparison.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/cpp0x/constexpr-ptrmem5.C: New test.
> ---
> gcc/cp/constexpr.c | 9 +++++++++
> gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C | 14 ++++++++++++++
> 2 files changed, 23 insertions(+)
> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C
>
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 208f43b..1ad87aa 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -1630,6 +1630,15 @@ 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);
> +
> + if (code == EQ_EXPR || code == NE_EXPR)
> + {
> + if (TREE_CODE (lhs) == PTRMEM_CST && CONSTANT_CLASS_P (rhs))
> + lhs = cplus_expand_constant (lhs);
> + if (TREE_CODE (rhs) == PTRMEM_CST && CONSTANT_CLASS_P (lhs))
> + rhs = cplus_expand_constant (rhs);
> + }
> +
> r = fold_binary_loc (loc, code, type, lhs, rhs);
> if (r == NULL_TREE)
> {
> 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..e28c917
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C
> @@ -0,0 +1,14 @@
> +// { 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::b);
> + SA ((!&X::b) == 0);
> +}
> --
> 2.7.0.rc0.50.g1470d8f.dirty
>
Ping.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-01-11 3:19 ` Patrick Palka
@ 2016-01-25 0:13 ` Patrick Palka
0 siblings, 0 replies; 11+ messages in thread
From: Patrick Palka @ 2016-01-25 0:13 UTC (permalink / raw)
To: GCC Patches; +Cc: Jason Merrill, Patrick Palka
On Sun, Jan 10, 2016 at 10:19 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
> On Mon, Dec 21, 2015 at 6:07 PM, Patrick Palka <patrick@parcs.ath.cx> wrote:
>> We are currently failing to fold equality comparisons involving
>> PTRMEM_CSTs since (I think) r229018 thus making this a GCC 6 regression.
>> This regression shows up in the boost testsuite.
>>
>> Fixed in a straightforward way. OK to commit after bootstrap + regtest?
>>
>> gcc/cp/ChangeLog:
>>
>> * constexpr.c (cxx_eval_binary_expression): Expand
>> the PTRMEM_CST operands of an equality comparison.
>>
>> gcc/testsuite/ChangeLog:
>>
>> * g++.dg/cpp0x/constexpr-ptrmem5.C: New test.
>> ---
>> gcc/cp/constexpr.c | 9 +++++++++
>> gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C | 14 ++++++++++++++
>> 2 files changed, 23 insertions(+)
>> create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C
>>
>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>> index 208f43b..1ad87aa 100644
>> --- a/gcc/cp/constexpr.c
>> +++ b/gcc/cp/constexpr.c
>> @@ -1630,6 +1630,15 @@ 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);
>> +
>> + if (code == EQ_EXPR || code == NE_EXPR)
>> + {
>> + if (TREE_CODE (lhs) == PTRMEM_CST && CONSTANT_CLASS_P (rhs))
>> + lhs = cplus_expand_constant (lhs);
>> + if (TREE_CODE (rhs) == PTRMEM_CST && CONSTANT_CLASS_P (lhs))
>> + rhs = cplus_expand_constant (rhs);
>> + }
>> +
>> r = fold_binary_loc (loc, code, type, lhs, rhs);
>> if (r == NULL_TREE)
>> {
>> 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..e28c917
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C
>> @@ -0,0 +1,14 @@
>> +// { 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::b);
>> + SA ((!&X::b) == 0);
>> +}
>> --
>> 2.7.0.rc0.50.g1470d8f.dirty
>>
>
> Ping.
Ping.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2015-12-21 23:07 [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members Patrick Palka
2016-01-11 3:19 ` Patrick Palka
@ 2016-02-03 16:06 ` Jason Merrill
2016-02-03 17:51 ` Patrick Palka
1 sibling, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2016-02-03 16:06 UTC (permalink / raw)
To: Patrick Palka, gcc-patches
On 12/22/2015 12:07 AM, Patrick Palka wrote:
> + if (code == EQ_EXPR || code == NE_EXPR)
> + {
> + if (TREE_CODE (lhs) == PTRMEM_CST && CONSTANT_CLASS_P (rhs))
> + lhs = cplus_expand_constant (lhs);
> + if (TREE_CODE (rhs) == PTRMEM_CST && CONSTANT_CLASS_P (lhs))
> + rhs = cplus_expand_constant (rhs);
> + }
If both sides are PTRMEM_CST, we should be able to compare them without
expanding, using cp_tree_equal.
Jason
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-02-03 16:06 ` Jason Merrill
@ 2016-02-03 17:51 ` Patrick Palka
2016-02-04 14:24 ` Jason Merrill
0 siblings, 1 reply; 11+ messages in thread
From: Patrick Palka @ 2016-02-03 17:51 UTC (permalink / raw)
To: Jason Merrill; +Cc: Patrick Palka, gcc-patches
On Tue, 2 Feb 2016, Jason Merrill wrote:
> On 12/22/2015 12:07 AM, Patrick Palka wrote:
>> + if (code == EQ_EXPR || code == NE_EXPR)
>> + {
>> + if (TREE_CODE (lhs) == PTRMEM_CST && CONSTANT_CLASS_P (rhs))
>> + lhs = cplus_expand_constant (lhs);
>> + if (TREE_CODE (rhs) == PTRMEM_CST && CONSTANT_CLASS_P (lhs))
>> + rhs = cplus_expand_constant (rhs);
>> + }
>
> If both sides are PTRMEM_CST, we should be able to compare them without
> expanding, using cp_tree_equal.
Ah, okay. Here's an updated patch that uses cp_tree_equal and avoids
calling cplus_expand_constant altogether, by assuming that we should
only expect to compare a PTRMEM_CST against another PTRMEM_CST or
against the constant -1. I also improved the coverage of the test case.
Does this look reasonable?
gcc/cp/ChangeLog:
* constexpr.c (cxx_eval_binary_expression): Fold equality
comparisons involving PTRMEM_CSTs.
gcc/testsuite/ChangeLog:
* g++.dg/cpp0x/constexpr-ptrmem5.C: New test.
---
gcc/cp/constexpr.c | 21 +++++++++++++++++++--
gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C | 17 +++++++++++++++++
2 files changed, 36 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-ptrmem5.C
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));
+}
--
2.7.0.240.g19e8eb6
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-02-03 17:51 ` Patrick Palka
@ 2016-02-04 14:24 ` Jason Merrill
2016-02-04 15:32 ` Patrick Palka
0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2016-02-04 14:24 UTC (permalink / raw)
To: Patrick Palka; +Cc: gcc-patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-02-04 14:24 ` Jason Merrill
@ 2016-02-04 15:32 ` Patrick Palka
2016-02-04 16:57 ` Jason Merrill
0 siblings, 1 reply; 11+ messages in thread
From: Patrick Palka @ 2016-02-04 15:32 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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"
}
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-02-04 15:32 ` Patrick Palka
@ 2016-02-04 16:57 ` Jason Merrill
2016-02-04 17:25 ` Patrick Palka
0 siblings, 1 reply; 11+ messages in thread
From: Jason Merrill @ 2016-02-04 16:57 UTC (permalink / raw)
To: Patrick Palka; +Cc: GCC Patches
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"
> }
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-02-04 16:57 ` Jason Merrill
@ 2016-02-04 17:25 ` Patrick Palka
2016-02-04 17:41 ` Patrick Palka
2016-02-04 18:48 ` Jason Merrill
0 siblings, 2 replies; 11+ messages in thread
From: Patrick Palka @ 2016-02-04 17:25 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-02-04 17:25 ` Patrick Palka
@ 2016-02-04 17:41 ` Patrick Palka
2016-02-04 18:48 ` Jason Merrill
1 sibling, 0 replies; 11+ messages in thread
From: Patrick Palka @ 2016-02-04 17:41 UTC (permalink / raw)
To: Jason Merrill; +Cc: GCC Patches
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.
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members
2016-02-04 17:25 ` Patrick Palka
2016-02-04 17:41 ` Patrick Palka
@ 2016-02-04 18:48 ` Jason Merrill
1 sibling, 0 replies; 11+ messages in thread
From: Jason Merrill @ 2016-02-04 18:48 UTC (permalink / raw)
To: Patrick Palka; +Cc: GCC Patches
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
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2016-02-04 18:48 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-12-21 23:07 [PATCH] Fix constexpr evaluation of comparisons involving pointer-to-members Patrick Palka
2016-01-11 3:19 ` Patrick Palka
2016-01-25 0:13 ` Patrick Palka
2016-02-03 16:06 ` Jason Merrill
2016-02-03 17:51 ` Patrick Palka
2016-02-04 14:24 ` Jason Merrill
2016-02-04 15:32 ` Patrick Palka
2016-02-04 16:57 ` Jason Merrill
2016-02-04 17:25 ` Patrick Palka
2016-02-04 17:41 ` Patrick Palka
2016-02-04 18:48 ` Jason Merrill
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).