public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [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).