diff mbox series

c++: constexpr RANGE_EXPR ctor indexes [PR95241]

Message ID 20200527205816.3647257-1-ppalka@redhat.com
State New
Headers show
Series c++: constexpr RANGE_EXPR ctor indexes [PR95241] | expand

Commit Message

Patrick Palka May 27, 2020, 8:58 p.m. UTC 
In the testcase below, the CONSTRUCTOR for 'field' contains a
RANGE_EXPR index:

  {aggr_init_expr<...>, [1...2]={.off=1}}

but get_or_insert_ctor_field isn't prepared to handle RANGE_EXPR
indexes.

This patch adds limited support for RANGE_EXPR indexes to
get_or_insert_ctor_field.  The limited scope of this patch should make
it more suitable for backporting, and support for more access patterns
would be needed only to handle self-modifying CONSTRUCTORs containing a
RANGE_EXPR index, but I haven't yet been able to come up with a testcase
that exhibits such a CONSTRUCTOR.

Passes 'make check-c++', does this look OK to commit to master and to
the GCC 10 branch after a full bootstrap and regtest?

gcc/cp/ChangeLog:

	PR c++/95241
	* constexpr.c (get_or_insert_ctor_field): Add limited support
	for RANGE_EXPR indexes.

gcc/testsuite/ChangeLog:

	PR c++/95241
	* g++.dg/cpp0x/constexpr-array25.C: New test.
---
 gcc/cp/constexpr.c                            | 12 +++++++++++
 .../g++.dg/cpp0x/constexpr-array25.C          | 21 +++++++++++++++++++
 2 files changed, 33 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C

Comments

Patrick Palka May 27, 2020, 9:03 p.m. UTC | #1 
On Wed, 27 May 2020, Patrick Palka wrote:

> In the testcase below, the CONSTRUCTOR for 'field' contains a
> RANGE_EXPR index:
> 
>   {aggr_init_expr<...>, [1...2]={.off=1}}
> 
> but get_or_insert_ctor_field isn't prepared to handle RANGE_EXPR
> indexes.
> 
> This patch adds limited support for RANGE_EXPR indexes to
> get_or_insert_ctor_field.  The limited scope of this patch should make
> it more suitable for backporting, and support for more access patterns
> would be needed only to handle self-modifying CONSTRUCTORs containing a
> RANGE_EXPR index, but I haven't yet been able to come up with a testcase
> that exhibits such a CONSTRUCTOR.
> 
> Passes 'make check-c++', does this look OK to commit to master and to
> the GCC 10 branch after a full bootstrap and regtest?
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/95241
> 	* constexpr.c (get_or_insert_ctor_field): Add limited support
> 	for RANGE_EXPR indexes.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/95241
> 	* g++.dg/cpp0x/constexpr-array25.C: New test.
> ---
>  gcc/cp/constexpr.c                            | 12 +++++++++++
>  .../g++.dg/cpp0x/constexpr-array25.C          | 21 +++++++++++++++++++
>  2 files changed, 33 insertions(+)
>  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 4e441ac8d2f..6f9bafbe8d8 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -3301,6 +3301,18 @@ get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1)
>      }
>    else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
>      {
> +      if (TREE_CODE (index) == RANGE_EXPR)
> +	{
> +	  /* Our support for RANGE_EXPR indexes is limited to accessing an
> +	     existing one via POS_HINT, and appending a new one to the end of
> +	     CTOR.  ??? Support for other access patterns might be needed.  */
> +	  tree lo = TREE_OPERAND (index, 0);
> +	  auto *elts = CONSTRUCTOR_ELTS (ctor);
> +	  gcc_assert (vec_safe_is_empty (elts)
> +		      || array_index_cmp (lo, elts->last().index) > 0);
> +	  return vec_safe_push (elts, {index, NULL_TREE});
> +	}
> +

Oops, it just occurred to me that the use of C++11 features here would
make this patch unsuitable for backporting.  C++98-compatible patch
incoming...

>        HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
>        gcc_assert (i >= 0);
>        constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
> new file mode 100644
> index 00000000000..9162943249f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
> @@ -0,0 +1,21 @@
> +// PR c++/95241
> +// { dg-do compile { target c++11 } }
> +
> +struct Fragment
> +{
> +  int off;
> +  constexpr Fragment(int _off) : off(_off) { }
> +  constexpr Fragment() : Fragment(1) { }
> +};
> +
> +struct Field
> +{
> +  Fragment fragments[3];
> +  constexpr Field(int off) : fragments{{off}} { }
> +};
> +
> +constexpr Field field{0};
> +
> +static_assert(field.fragments[0].off == 0
> +	      && field.fragments[1].off == 1
> +	      && field.fragments[2].off == 1, "");
> -- 
> 2.27.0.rc1.5.gae92ac8ae3
> 
>
Patrick Palka May 27, 2020, 9:15 p.m. UTC | #2 
On Wed, 27 May 2020, Patrick Palka wrote:

> On Wed, 27 May 2020, Patrick Palka wrote:
> 
> > In the testcase below, the CONSTRUCTOR for 'field' contains a
> > RANGE_EXPR index:
> > 
> >   {aggr_init_expr<...>, [1...2]={.off=1}}
> > 
> > but get_or_insert_ctor_field isn't prepared to handle RANGE_EXPR
> > indexes.
> > 
> > This patch adds limited support for RANGE_EXPR indexes to
> > get_or_insert_ctor_field.  The limited scope of this patch should make
> > it more suitable for backporting, and support for more access patterns
> > would be needed only to handle self-modifying CONSTRUCTORs containing a
> > RANGE_EXPR index, but I haven't yet been able to come up with a testcase
> > that exhibits such a CONSTRUCTOR.
> > 
> > Passes 'make check-c++', does this look OK to commit to master and to
> > the GCC 10 branch after a full bootstrap and regtest?
> > 
> > gcc/cp/ChangeLog:
> > 
> > 	PR c++/95241
> > 	* constexpr.c (get_or_insert_ctor_field): Add limited support
> > 	for RANGE_EXPR indexes.
> > 
> > gcc/testsuite/ChangeLog:
> > 
> > 	PR c++/95241
> > 	* g++.dg/cpp0x/constexpr-array25.C: New test.
> > ---
> >  gcc/cp/constexpr.c                            | 12 +++++++++++
> >  .../g++.dg/cpp0x/constexpr-array25.C          | 21 +++++++++++++++++++
> >  2 files changed, 33 insertions(+)
> >  create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
> > 
> > diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> > index 4e441ac8d2f..6f9bafbe8d8 100644
> > --- a/gcc/cp/constexpr.c
> > +++ b/gcc/cp/constexpr.c
> > @@ -3301,6 +3301,18 @@ get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1)
> >      }
> >    else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
> >      {
> > +      if (TREE_CODE (index) == RANGE_EXPR)
> > +	{
> > +	  /* Our support for RANGE_EXPR indexes is limited to accessing an
> > +	     existing one via POS_HINT, and appending a new one to the end of
> > +	     CTOR.  ??? Support for other access patterns might be needed.  */
> > +	  tree lo = TREE_OPERAND (index, 0);
> > +	  auto *elts = CONSTRUCTOR_ELTS (ctor);
> > +	  gcc_assert (vec_safe_is_empty (elts)
> > +		      || array_index_cmp (lo, elts->last().index) > 0);
> > +	  return vec_safe_push (elts, {index, NULL_TREE});
> > +	}
> > +
> 
> Oops, it just occurred to me that the use of C++11 features here would
> make this patch unsuitable for backporting.  C++98-compatible patch
> incoming...

Here it is.  Does the following look OK to commit to master and to the
GCC 10 branch after a full bootstrap and regtest?

-- >8 --

Subject: [PATCH] c++: constexpr RANGE_EXPR ctor indexes [PR95241]

In the testcase below, the CONSTRUCTOR for 'field' contains a
RANGE_EXPR index:

  {aggr_init_expr<...>, [1...2]={.off=1}}

but get_or_insert_ctor_field isn't prepared to handle RANGE_EXPR
indexes.

This patch adds limited support for RANGE_EXPR indexes to
get_or_insert_ctor_field.  The limited scope of this patch should make
it more suitable for backporting, and support for more access patterns
would be needed only to handle self-modifying CONSTRUCTORs containing a
RANGE_EXPR index, but I haven't yet been able to come up with a testcase
that exhibits such a CONSTRUCTOR.

gcc/cp/ChangeLog:

	PR c++/95241
	* constexpr.c (get_or_insert_ctor_field): Add limited support
	for RANGE_EXPR indexes.

gcc/testsuite/ChangeLog:

	PR c++/95241
	* g++.dg/cpp0x/constexpr-array25.C: New test.
---
 gcc/cp/constexpr.c                            | 15 +++++++++++++
 .../g++.dg/cpp0x/constexpr-array25.C          | 21 +++++++++++++++++++
 2 files changed, 36 insertions(+)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 4e441ac8d2f..32f2ef96fc7 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3301,6 +3301,21 @@ get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1)
     }
   else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
     {
+      if (TREE_CODE (index) == RANGE_EXPR)
+	{
+	  /* ??? Support for RANGE_EXPR indexes is currently limited to
+	     accessing one via POS_HINT, or appending a new one to the end
+	     of CTOR.  Support for other access patterns may be needed.  */
+	  vec<constructor_elt, va_gc> *elts = CONSTRUCTOR_ELTS (ctor);
+	  if (vec_safe_length (elts))
+	    {
+	      tree lo = TREE_OPERAND (index, 0);
+	      gcc_assert (array_index_cmp (lo, elts->last().index) > 0);
+	    }
+	  CONSTRUCTOR_APPEND_ELT (elts, index, NULL_TREE);
+	  return &elts->last();
+	}
+
       HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
       gcc_assert (i >= 0);
       constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
new file mode 100644
index 00000000000..9162943249f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
@@ -0,0 +1,21 @@
+// PR c++/95241
+// { dg-do compile { target c++11 } }
+
+struct Fragment
+{
+  int off;
+  constexpr Fragment(int _off) : off(_off) { }
+  constexpr Fragment() : Fragment(1) { }
+};
+
+struct Field
+{
+  Fragment fragments[3];
+  constexpr Field(int off) : fragments{{off}} { }
+};
+
+constexpr Field field{0};
+
+static_assert(field.fragments[0].off == 0
+	      && field.fragments[1].off == 1
+	      && field.fragments[2].off == 1, "");
Jason Merrill May 28, 2020, 8:44 p.m. UTC | #3 
On 5/27/20 5:15 PM, Patrick Palka wrote:
> On Wed, 27 May 2020, Patrick Palka wrote:
> 
>> On Wed, 27 May 2020, Patrick Palka wrote:
>>
>>> In the testcase below, the CONSTRUCTOR for 'field' contains a
>>> RANGE_EXPR index:
>>>
>>>    {aggr_init_expr<...>, [1...2]={.off=1}}
>>>
>>> but get_or_insert_ctor_field isn't prepared to handle RANGE_EXPR
>>> indexes.
>>>
>>> This patch adds limited support for RANGE_EXPR indexes to
>>> get_or_insert_ctor_field.  The limited scope of this patch should make
>>> it more suitable for backporting, and support for more access patterns
>>> would be needed only to handle self-modifying CONSTRUCTORs containing a
>>> RANGE_EXPR index, but I haven't yet been able to come up with a testcase
>>> that exhibits such a CONSTRUCTOR.
>>>
>>> Passes 'make check-c++', does this look OK to commit to master and to
>>> the GCC 10 branch after a full bootstrap and regtest?

OK.

>>> gcc/cp/ChangeLog:
>>>
>>> 	PR c++/95241
>>> 	* constexpr.c (get_or_insert_ctor_field): Add limited support
>>> 	for RANGE_EXPR indexes.
>>>
>>> gcc/testsuite/ChangeLog:
>>>
>>> 	PR c++/95241
>>> 	* g++.dg/cpp0x/constexpr-array25.C: New test.
>>> ---
>>>   gcc/cp/constexpr.c                            | 12 +++++++++++
>>>   .../g++.dg/cpp0x/constexpr-array25.C          | 21 +++++++++++++++++++
>>>   2 files changed, 33 insertions(+)
>>>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
>>>
>>> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
>>> index 4e441ac8d2f..6f9bafbe8d8 100644
>>> --- a/gcc/cp/constexpr.c
>>> +++ b/gcc/cp/constexpr.c
>>> @@ -3301,6 +3301,18 @@ get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1)
>>>       }
>>>     else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
>>>       {
>>> +      if (TREE_CODE (index) == RANGE_EXPR)
>>> +	{
>>> +	  /* Our support for RANGE_EXPR indexes is limited to accessing an
>>> +	     existing one via POS_HINT, and appending a new one to the end of
>>> +	     CTOR.  ??? Support for other access patterns might be needed.  */
>>> +	  tree lo = TREE_OPERAND (index, 0);
>>> +	  auto *elts = CONSTRUCTOR_ELTS (ctor);
>>> +	  gcc_assert (vec_safe_is_empty (elts)
>>> +		      || array_index_cmp (lo, elts->last().index) > 0);
>>> +	  return vec_safe_push (elts, {index, NULL_TREE});
>>> +	}
>>> +
>>
>> Oops, it just occurred to me that the use of C++11 features here would
>> make this patch unsuitable for backporting.  C++98-compatible patch
>> incoming...
> 
> Here it is.  Does the following look OK to commit to master and to the
> GCC 10 branch after a full bootstrap and regtest?
> 
> -- >8 --
> 
> Subject: [PATCH] c++: constexpr RANGE_EXPR ctor indexes [PR95241]
> 
> In the testcase below, the CONSTRUCTOR for 'field' contains a
> RANGE_EXPR index:
> 
>    {aggr_init_expr<...>, [1...2]={.off=1}}
> 
> but get_or_insert_ctor_field isn't prepared to handle RANGE_EXPR
> indexes.
> 
> This patch adds limited support for RANGE_EXPR indexes to
> get_or_insert_ctor_field.  The limited scope of this patch should make
> it more suitable for backporting, and support for more access patterns
> would be needed only to handle self-modifying CONSTRUCTORs containing a
> RANGE_EXPR index, but I haven't yet been able to come up with a testcase
> that exhibits such a CONSTRUCTOR.
> 
> gcc/cp/ChangeLog:
> 
> 	PR c++/95241
> 	* constexpr.c (get_or_insert_ctor_field): Add limited support
> 	for RANGE_EXPR indexes.
> 
> gcc/testsuite/ChangeLog:
> 
> 	PR c++/95241
> 	* g++.dg/cpp0x/constexpr-array25.C: New test.
> ---
>   gcc/cp/constexpr.c                            | 15 +++++++++++++
>   .../g++.dg/cpp0x/constexpr-array25.C          | 21 +++++++++++++++++++
>   2 files changed, 36 insertions(+)
>   create mode 100644 gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
> 
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index 4e441ac8d2f..32f2ef96fc7 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -3301,6 +3301,21 @@ get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1)
>       }
>     else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
>       {
> +      if (TREE_CODE (index) == RANGE_EXPR)
> +	{
> +	  /* ??? Support for RANGE_EXPR indexes is currently limited to
> +	     accessing one via POS_HINT, or appending a new one to the end
> +	     of CTOR.  Support for other access patterns may be needed.  */
> +	  vec<constructor_elt, va_gc> *elts = CONSTRUCTOR_ELTS (ctor);
> +	  if (vec_safe_length (elts))
> +	    {
> +	      tree lo = TREE_OPERAND (index, 0);
> +	      gcc_assert (array_index_cmp (lo, elts->last().index) > 0);
> +	    }
> +	  CONSTRUCTOR_APPEND_ELT (elts, index, NULL_TREE);
> +	  return &elts->last();
> +	}
> +
>         HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
>         gcc_assert (i >= 0);
>         constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
> diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
> new file mode 100644
> index 00000000000..9162943249f
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
> @@ -0,0 +1,21 @@
> +// PR c++/95241
> +// { dg-do compile { target c++11 } }
> +
> +struct Fragment
> +{
> +  int off;
> +  constexpr Fragment(int _off) : off(_off) { }
> +  constexpr Fragment() : Fragment(1) { }
> +};
> +
> +struct Field
> +{
> +  Fragment fragments[3];
> +  constexpr Field(int off) : fragments{{off}} { }
> +};
> +
> +constexpr Field field{0};
> +
> +static_assert(field.fragments[0].off == 0
> +	      && field.fragments[1].off == 1
> +	      && field.fragments[2].off == 1, "");
>
diff mbox series

Patch

diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index 4e441ac8d2f..6f9bafbe8d8 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -3301,6 +3301,18 @@  get_or_insert_ctor_field (tree ctor, tree index, int pos_hint = -1)
     }
   else if (TREE_CODE (type) == ARRAY_TYPE || TREE_CODE (type) == VECTOR_TYPE)
     {
+      if (TREE_CODE (index) == RANGE_EXPR)
+	{
+	  /* Our support for RANGE_EXPR indexes is limited to accessing an
+	     existing one via POS_HINT, and appending a new one to the end of
+	     CTOR.  ??? Support for other access patterns might be needed.  */
+	  tree lo = TREE_OPERAND (index, 0);
+	  auto *elts = CONSTRUCTOR_ELTS (ctor);
+	  gcc_assert (vec_safe_is_empty (elts)
+		      || array_index_cmp (lo, elts->last().index) > 0);
+	  return vec_safe_push (elts, {index, NULL_TREE});
+	}
+
       HOST_WIDE_INT i = find_array_ctor_elt (ctor, index, /*insert*/true);
       gcc_assert (i >= 0);
       constructor_elt *cep = CONSTRUCTOR_ELT (ctor, i);
diff --git a/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
new file mode 100644
index 00000000000..9162943249f
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/constexpr-array25.C
@@ -0,0 +1,21 @@ 
+// PR c++/95241
+// { dg-do compile { target c++11 } }
+
+struct Fragment
+{
+  int off;
+  constexpr Fragment(int _off) : off(_off) { }
+  constexpr Fragment() : Fragment(1) { }
+};
+
+struct Field
+{
+  Fragment fragments[3];
+  constexpr Field(int off) : fragments{{off}} { }
+};
+
+constexpr Field field{0};
+
+static_assert(field.fragments[0].off == 0
+	      && field.fragments[1].off == 1
+	      && field.fragments[2].off == 1, "");