public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* C++ PATCH for c++/65398 (valid constexpr rejected)
@ 2015-03-13 14:41 Marek Polacek
  2015-03-18 10:08 ` Richard Biener
  0 siblings, 1 reply; 15+ messages in thread
From: Marek Polacek @ 2015-03-13 14:41 UTC (permalink / raw)
  To: GCC Patches, Jason Merrill

We started to reject this (IMHO valid) testcase with r214941 that did away with
try_move_mult_to_index -- meaning that we are no longer able to fold *(&s[0] + 1)
into s[1], while we are able to fold *(s + 1) into s[1].

I suppose cxx_fold_indirect_ref ought to be able to handle both cases, so I added
some code to that effect, it should handle now at least the simple cases...
Or should that be handled in the middle end?

I have also tried:

constexpr char s[] = "abc";
constexpr int i = 0;
constexpr char c = *(&s[0] + i);

and that is accepted even without this patch.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2015-03-13  Marek Polacek  <polacek@redhat.com>

	PR c++/65398
	* constexpr.c (cxx_fold_indirect_ref): Transform *(&A[i] p+ j) into
	A[i + j].

	* g++.dg/cpp0x/pr65398.C: New test.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 1b5f50c..18f4d8c 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -2427,6 +2427,17 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
 		    break;
 		  }
 	    }
+	  /* *(&A[i] p+ j) => A[i + j] */
+	  else if (TREE_CODE (op00) == ARRAY_REF
+		   && TREE_CODE (TREE_OPERAND (op00, 1)) == INTEGER_CST
+		   && TREE_CODE (op01) == INTEGER_CST)
+	    {
+	      tree t = fold_convert_loc (loc, TREE_TYPE (op01),
+					 TREE_OPERAND (op00, 1));
+	      t = size_binop_loc (loc, PLUS_EXPR, op01, t);
+	      return build4_loc (loc, ARRAY_REF, type, TREE_OPERAND (op00, 0),
+				 t, NULL_TREE, NULL_TREE);
+	    }
 	}
     }
   /* *(foo *)fooarrptr => (*fooarrptr)[0] */
diff --git gcc/testsuite/g++.dg/cpp0x/pr65398.C gcc/testsuite/g++.dg/cpp0x/pr65398.C
index e69de29..dc22d27 100644
--- gcc/testsuite/g++.dg/cpp0x/pr65398.C
+++ gcc/testsuite/g++.dg/cpp0x/pr65398.C
@@ -0,0 +1,19 @@
+// PR c++/65398
+// { dg-do compile { target c++11 } }
+
+#define SA(X) static_assert((X),#X)
+
+constexpr char s[] = "abc";
+constexpr char c1 = *(&s[0] + 0);
+constexpr char c2 = *(&s[0] + 1);
+constexpr char c3 = *(&s[1] + 0);
+constexpr char c4 = *(&s[1] + 1);
+constexpr char c5 = *(&s[2] + 0);
+constexpr char c6 = *(&s[0] + 2);
+
+SA (c1 == 'a');
+SA (c2 == 'b');
+SA (c3 == 'b');
+SA (c4 == 'c');
+SA (c5 == 'c');
+SA (c6 == 'c');

	Marek

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-13 14:41 C++ PATCH for c++/65398 (valid constexpr rejected) Marek Polacek
@ 2015-03-18 10:08 ` Richard Biener
  2015-03-19 18:05   ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Richard Biener @ 2015-03-18 10:08 UTC (permalink / raw)
  To: Marek Polacek; +Cc: GCC Patches, Jason Merrill

On Fri, Mar 13, 2015 at 3:40 PM, Marek Polacek <polacek@redhat.com> wrote:
> We started to reject this (IMHO valid) testcase with r214941 that did away with
> try_move_mult_to_index -- meaning that we are no longer able to fold *(&s[0] + 1)
> into s[1], while we are able to fold *(s + 1) into s[1].
>
> I suppose cxx_fold_indirect_ref ought to be able to handle both cases, so I added
> some code to that effect, it should handle now at least the simple cases...
> Or should that be handled in the middle end?

It's "correct" for constexpr folding but not correct to hand s[1] down to
the middle-end IL (both cases).  Well, in the particular case with
in-array-bound constant and a non-pointer base it's good enough at
least.

Richard.

> I have also tried:
>
> constexpr char s[] = "abc";
> constexpr int i = 0;
> constexpr char c = *(&s[0] + i);
>
> and that is accepted even without this patch.
>
> Bootstrapped/regtested on x86_64-linux, ok for trunk?
>
> 2015-03-13  Marek Polacek  <polacek@redhat.com>
>
>         PR c++/65398
>         * constexpr.c (cxx_fold_indirect_ref): Transform *(&A[i] p+ j) into
>         A[i + j].
>
>         * g++.dg/cpp0x/pr65398.C: New test.
>
> diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
> index 1b5f50c..18f4d8c 100644
> --- gcc/cp/constexpr.c
> +++ gcc/cp/constexpr.c
> @@ -2427,6 +2427,17 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
>                     break;
>                   }
>             }
> +         /* *(&A[i] p+ j) => A[i + j] */
> +         else if (TREE_CODE (op00) == ARRAY_REF
> +                  && TREE_CODE (TREE_OPERAND (op00, 1)) == INTEGER_CST
> +                  && TREE_CODE (op01) == INTEGER_CST)
> +           {
> +             tree t = fold_convert_loc (loc, TREE_TYPE (op01),
> +                                        TREE_OPERAND (op00, 1));
> +             t = size_binop_loc (loc, PLUS_EXPR, op01, t);
> +             return build4_loc (loc, ARRAY_REF, type, TREE_OPERAND (op00, 0),
> +                                t, NULL_TREE, NULL_TREE);
> +           }
>         }
>      }
>    /* *(foo *)fooarrptr => (*fooarrptr)[0] */
> diff --git gcc/testsuite/g++.dg/cpp0x/pr65398.C gcc/testsuite/g++.dg/cpp0x/pr65398.C
> index e69de29..dc22d27 100644
> --- gcc/testsuite/g++.dg/cpp0x/pr65398.C
> +++ gcc/testsuite/g++.dg/cpp0x/pr65398.C
> @@ -0,0 +1,19 @@
> +// PR c++/65398
> +// { dg-do compile { target c++11 } }
> +
> +#define SA(X) static_assert((X),#X)
> +
> +constexpr char s[] = "abc";
> +constexpr char c1 = *(&s[0] + 0);
> +constexpr char c2 = *(&s[0] + 1);
> +constexpr char c3 = *(&s[1] + 0);
> +constexpr char c4 = *(&s[1] + 1);
> +constexpr char c5 = *(&s[2] + 0);
> +constexpr char c6 = *(&s[0] + 2);
> +
> +SA (c1 == 'a');
> +SA (c2 == 'b');
> +SA (c3 == 'b');
> +SA (c4 == 'c');
> +SA (c5 == 'c');
> +SA (c6 == 'c');
>
>         Marek

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-18 10:08 ` Richard Biener
@ 2015-03-19 18:05   ` Jakub Jelinek
  2015-03-19 18:13     ` Marek Polacek
  2015-03-20 14:53     ` Jason Merrill
  0 siblings, 2 replies; 15+ messages in thread
From: Jakub Jelinek @ 2015-03-19 18:05 UTC (permalink / raw)
  To: Richard Biener, Jason Merrill; +Cc: Marek Polacek, GCC Patches

On Wed, Mar 18, 2015 at 11:08:15AM +0100, Richard Biener wrote:
> On Fri, Mar 13, 2015 at 3:40 PM, Marek Polacek <polacek@redhat.com> wrote:
> > We started to reject this (IMHO valid) testcase with r214941 that did away with
> > try_move_mult_to_index -- meaning that we are no longer able to fold *(&s[0] + 1)
> > into s[1], while we are able to fold *(s + 1) into s[1].
> >
> > I suppose cxx_fold_indirect_ref ought to be able to handle both cases, so I added
> > some code to that effect, it should handle now at least the simple cases...
> > Or should that be handled in the middle end?
> 
> It's "correct" for constexpr folding but not correct to hand s[1] down to
> the middle-end IL (both cases).  Well, in the particular case with
> in-array-bound constant and a non-pointer base it's good enough at
> least.

I believe cxx_fold_indirect_ref result is not passed through to the
middle-end, unless it can be folded into a constant.

Though, a question is if we do (or, if we don't and should) reject say
constexpr char s[] = "abc";
constexpr int j = 4;
constexpr char c = *(&s[j] - 2);
because there was out of bound access in there.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-19 18:05   ` Jakub Jelinek
@ 2015-03-19 18:13     ` Marek Polacek
  2015-03-19 18:17       ` Jakub Jelinek
  2015-03-20 14:53     ` Jason Merrill
  1 sibling, 1 reply; 15+ messages in thread
From: Marek Polacek @ 2015-03-19 18:13 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, Jason Merrill, GCC Patches

On Thu, Mar 19, 2015 at 07:05:36PM +0100, Jakub Jelinek wrote:
> On Wed, Mar 18, 2015 at 11:08:15AM +0100, Richard Biener wrote:
> > On Fri, Mar 13, 2015 at 3:40 PM, Marek Polacek <polacek@redhat.com> wrote:
> > > We started to reject this (IMHO valid) testcase with r214941 that did away with
> > > try_move_mult_to_index -- meaning that we are no longer able to fold *(&s[0] + 1)
> > > into s[1], while we are able to fold *(s + 1) into s[1].
> > >
> > > I suppose cxx_fold_indirect_ref ought to be able to handle both cases, so I added
> > > some code to that effect, it should handle now at least the simple cases...
> > > Or should that be handled in the middle end?
> > 
> > It's "correct" for constexpr folding but not correct to hand s[1] down to
> > the middle-end IL (both cases).  Well, in the particular case with
> > in-array-bound constant and a non-pointer base it's good enough at
> > least.
> 
> I believe cxx_fold_indirect_ref result is not passed through to the
> middle-end, unless it can be folded into a constant.
> 
> Though, a question is if we do (or, if we don't and should) reject say
> constexpr char s[] = "abc";
> constexpr int j = 4;
> constexpr char c = *(&s[j] - 2);
> because there was out of bound access in there.

That is rejected even with my patch with:
error: overflow in constant expression [-fpermissive]
and without the patch:
error: ‘*((& s[4]) + 18446744073709551614u)’ is not a constant expression
(a valid constexpr can't have UB).

	Marek

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-19 18:13     ` Marek Polacek
@ 2015-03-19 18:17       ` Jakub Jelinek
  2015-03-19 18:33         ` Marek Polacek
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2015-03-19 18:17 UTC (permalink / raw)
  To: Marek Polacek; +Cc: Richard Biener, Jason Merrill, GCC Patches

On Thu, Mar 19, 2015 at 07:13:47PM +0100, Marek Polacek wrote:
> On Thu, Mar 19, 2015 at 07:05:36PM +0100, Jakub Jelinek wrote:
> > I believe cxx_fold_indirect_ref result is not passed through to the
> > middle-end, unless it can be folded into a constant.
> > 
> > Though, a question is if we do (or, if we don't and should) reject say
> > constexpr char s[] = "abc";
> > constexpr int j = 4;
> > constexpr char c = *(&s[j] - 2);
> > because there was out of bound access in there.
> 
> That is rejected even with my patch with:
> error: overflow in constant expression [-fpermissive]
> and without the patch:
> error: ‘*((& s[4]) + 18446744073709551614u)’ is not a constant expression
> (a valid constexpr can't have UB).

But s/j = 4/j = 3/ should be valid, don't we reject even that?
I mean, isn't the rejection because we fold the - 2 early into sizetype
(unsigned) + -2UL?

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-19 18:17       ` Jakub Jelinek
@ 2015-03-19 18:33         ` Marek Polacek
  0 siblings, 0 replies; 15+ messages in thread
From: Marek Polacek @ 2015-03-19 18:33 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, Jason Merrill, GCC Patches

On Thu, Mar 19, 2015 at 07:17:23PM +0100, Jakub Jelinek wrote:
> On Thu, Mar 19, 2015 at 07:13:47PM +0100, Marek Polacek wrote:
> > On Thu, Mar 19, 2015 at 07:05:36PM +0100, Jakub Jelinek wrote:
> > > I believe cxx_fold_indirect_ref result is not passed through to the
> > > middle-end, unless it can be folded into a constant.
> > > 
> > > Though, a question is if we do (or, if we don't and should) reject say
> > > constexpr char s[] = "abc";
> > > constexpr int j = 4;
> > > constexpr char c = *(&s[j] - 2);
> > > because there was out of bound access in there.
> > 
> > That is rejected even with my patch with:
> > error: overflow in constant expression [-fpermissive]
> > and without the patch:
> > error: ‘*((& s[4]) + 18446744073709551614u)’ is not a constant expression
> > (a valid constexpr can't have UB).
> 
> But s/j = 4/j = 3/ should be valid, don't we reject even that?
> I mean, isn't the rejection because we fold the - 2 early into sizetype
> (unsigned) + -2UL?

Unfortunately we reject even that (regardless the patch), and yeah, it's
because of how POINTER_PLUS_EXPR uses sizetype as a type of the second operand.

E.g.

constexpr char s[] = "abc";
constexpr int j = 1;
constexpr char c = *(&s[j] + 3);

is correctly rejected with the patch:
error: array subscript out of bound

	Marek

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-19 18:05   ` Jakub Jelinek
  2015-03-19 18:13     ` Marek Polacek
@ 2015-03-20 14:53     ` Jason Merrill
  2015-03-20 14:59       ` Jakub Jelinek
  2015-03-20 20:57       ` C++ PATCH for c++/65398 (valid constexpr rejected) (take 2) Marek Polacek
  1 sibling, 2 replies; 15+ messages in thread
From: Jason Merrill @ 2015-03-20 14:53 UTC (permalink / raw)
  To: Jakub Jelinek, Richard Biener; +Cc: Marek Polacek, GCC Patches

On 03/19/2015 02:05 PM, Jakub Jelinek wrote:
> Though, a question is if we do (or, if we don't and should) reject say
> constexpr char s[] = "abc";
> constexpr int j = 4;
> constexpr char c = *(&s[j] - 2);
> because there was out of bound access in there.

I don't see an out-of-bound access in this example; taking the address 
of one-past-the-end is OK as long as you don't try to access through it.

> Unfortunately we reject even that (regardless the patch), and yeah, it's
> because of how POINTER_PLUS_EXPR uses sizetype as a type of the second operand.

This seems like something to fix in this patch.

> +	      tree t = fold_convert_loc (loc, TREE_TYPE (op01),
> +					 TREE_OPERAND (op00, 1));
> +	      t = size_binop_loc (loc, PLUS_EXPR, op01, t);

This seems to be assuming that the elements are size 1.

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-20 14:53     ` Jason Merrill
@ 2015-03-20 14:59       ` Jakub Jelinek
  2015-03-20 15:03         ` Jason Merrill
  2015-03-20 20:57       ` C++ PATCH for c++/65398 (valid constexpr rejected) (take 2) Marek Polacek
  1 sibling, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2015-03-20 14:59 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, Marek Polacek, GCC Patches

On Fri, Mar 20, 2015 at 10:53:50AM -0400, Jason Merrill wrote:
> On 03/19/2015 02:05 PM, Jakub Jelinek wrote:
> >Though, a question is if we do (or, if we don't and should) reject say
> >constexpr char s[] = "abc";
> >constexpr int j = 4;
> >constexpr char c = *(&s[j] - 2);
> >because there was out of bound access in there.
> 
> I don't see an out-of-bound access in this example; taking the address of
> one-past-the-end is OK as long as you don't try to access through it.

It is taking address of two past the end though - &s[3] is fine, sure.
But &s[4] is invalid already.

> >Unfortunately we reject even that (regardless the patch), and yeah, it's
> >because of how POINTER_PLUS_EXPR uses sizetype as a type of the second operand.
> 
> This seems like something to fix in this patch.
> 
> >+	      tree t = fold_convert_loc (loc, TREE_TYPE (op01),
> >+					 TREE_OPERAND (op00, 1));
> >+	      t = size_binop_loc (loc, PLUS_EXPR, op01, t);
> 
> This seems to be assuming that the elements are size 1.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-20 14:59       ` Jakub Jelinek
@ 2015-03-20 15:03         ` Jason Merrill
  2015-03-20 15:11           ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Jason Merrill @ 2015-03-20 15:03 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, Marek Polacek, GCC Patches

On 03/20/2015 10:59 AM, Jakub Jelinek wrote:
> On Fri, Mar 20, 2015 at 10:53:50AM -0400, Jason Merrill wrote:
>> On 03/19/2015 02:05 PM, Jakub Jelinek wrote:
>>> Though, a question is if we do (or, if we don't and should) reject say
>>> constexpr char s[] = "abc";
>>> constexpr int j = 4;
>>> constexpr char c = *(&s[j] - 2);
>>> because there was out of bound access in there.
>>
>> I don't see an out-of-bound access in this example; taking the address of
>> one-past-the-end is OK as long as you don't try to access through it.
>
> It is taking address of two past the end though - &s[3] is fine, sure.
> But &s[4] is invalid already.

&s[3] is the address of the terminal \0.

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-20 15:03         ` Jason Merrill
@ 2015-03-20 15:11           ` Jakub Jelinek
  2015-03-20 15:15             ` Jason Merrill
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2015-03-20 15:11 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, Marek Polacek, GCC Patches

On Fri, Mar 20, 2015 at 11:02:59AM -0400, Jason Merrill wrote:
> On 03/20/2015 10:59 AM, Jakub Jelinek wrote:
> >On Fri, Mar 20, 2015 at 10:53:50AM -0400, Jason Merrill wrote:
> >>On 03/19/2015 02:05 PM, Jakub Jelinek wrote:
> >>>Though, a question is if we do (or, if we don't and should) reject say
> >>>constexpr char s[] = "abc";
> >>>constexpr int j = 4;
> >>>constexpr char c = *(&s[j] - 2);
> >>>because there was out of bound access in there.
> >>
> >>I don't see an out-of-bound access in this example; taking the address of
> >>one-past-the-end is OK as long as you don't try to access through it.
> >
> >It is taking address of two past the end though - &s[3] is fine, sure.
> >But &s[4] is invalid already.
> 
> &s[3] is the address of the terminal \0.

Yeah, sure.  But the above testcase does &s[4], which is out of bounds
arithmetics, and then subtracts 2 to point it back into range.
I'm not saying it is absolutely necessary to handle this for GCC 5, but
we at least should not treat the POINTER_PLUS_EXPR offset in the constexpr
handling as unsigned, but think of it as signed, otherwise we reject even
valid code - say constexpr char d = *(&s[3] - 1).

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-20 15:11           ` Jakub Jelinek
@ 2015-03-20 15:15             ` Jason Merrill
  2015-03-20 15:28               ` Jakub Jelinek
  0 siblings, 1 reply; 15+ messages in thread
From: Jason Merrill @ 2015-03-20 15:15 UTC (permalink / raw)
  To: Jakub Jelinek; +Cc: Richard Biener, Marek Polacek, GCC Patches

On 03/20/2015 11:11 AM, Jakub Jelinek wrote:
>> &s[3] is the address of the terminal \0.
>
> Yeah, sure.  But the above testcase does &s[4], which is out of bounds
> arithmetics

But since &s[3] is the address of an element of the array (the NUL), 
&s[4] is one-past-the-end, which is fine.  &s[5] would be out of bounds.

> I'm not saying it is absolutely necessary to handle this for GCC 5, but
> we at least should not treat the POINTER_PLUS_EXPR offset in the constexpr
> handling as unsigned, but think of it as signed, otherwise we reject even
> valid code - say constexpr char d = *(&s[3] - 1).

Agreed.  And I still think it makes sense to fix this as part of this patch.

Jason

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected)
  2015-03-20 15:15             ` Jason Merrill
@ 2015-03-20 15:28               ` Jakub Jelinek
  0 siblings, 0 replies; 15+ messages in thread
From: Jakub Jelinek @ 2015-03-20 15:28 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Richard Biener, Marek Polacek, GCC Patches

On Fri, Mar 20, 2015 at 11:15:30AM -0400, Jason Merrill wrote:
> On 03/20/2015 11:11 AM, Jakub Jelinek wrote:
> >>&s[3] is the address of the terminal \0.
> >
> >Yeah, sure.  But the above testcase does &s[4], which is out of bounds
> >arithmetics
> 
> But since &s[3] is the address of an element of the array (the NUL), &s[4]
> is one-past-the-end, which is fine.  &s[5] would be out of bounds.

Ashamed...
Obviously I meant if we should diagnose *(&s[5] - 2).
clang++ does.

> Agreed.  And I still think it makes sense to fix this as part of this patch.

Yes.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected) (take 2)
  2015-03-20 14:53     ` Jason Merrill
  2015-03-20 14:59       ` Jakub Jelinek
@ 2015-03-20 20:57       ` Marek Polacek
  2015-03-20 21:04         ` Jakub Jelinek
  1 sibling, 1 reply; 15+ messages in thread
From: Marek Polacek @ 2015-03-20 20:57 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Jakub Jelinek, Richard Biener, GCC Patches

On Fri, Mar 20, 2015 at 10:53:50AM -0400, Jason Merrill wrote:
> On 03/19/2015 02:05 PM, Jakub Jelinek wrote:
> >Though, a question is if we do (or, if we don't and should) reject say
> >constexpr char s[] = "abc";
> >constexpr int j = 4;
> >constexpr char c = *(&s[j] - 2);
> >because there was out of bound access in there.
> 
> I don't see an out-of-bound access in this example; taking the address of
> one-past-the-end is OK as long as you don't try to access through it.
> 
> >Unfortunately we reject even that (regardless the patch), and yeah, it's
> >because of how POINTER_PLUS_EXPR uses sizetype as a type of the second operand.
> 
> This seems like something to fix in this patch.
 
In the following version of the patch I tried to address various diagnostic
issues raised in this thread.  Please see the testcase if what I did makes
sense.

> >+	      tree t = fold_convert_loc (loc, TREE_TYPE (op01),
> >+					 TREE_OPERAND (op00, 1));
> >+	      t = size_binop_loc (loc, PLUS_EXPR, op01, t);
> 
> This seems to be assuming that the elements are size 1.

Dunno how I flubbed that.  Fixed.

Bootstrapped/regtested on x86_64-linux, ok for trunk?

2015-03-20  Marek Polacek  <polacek@redhat.com>

	PR c++/65398
	* constexpr.c (cxx_fold_indirect_ref): Transform *(&A[i] p+ j) into
	A[i + j].

	* g++.dg/cpp0x/pr65398.C: New test.

diff --git gcc/cp/constexpr.c gcc/cp/constexpr.c
index 1b5f50c..37b619d 100644
--- gcc/cp/constexpr.c
+++ gcc/cp/constexpr.c
@@ -2427,6 +2427,27 @@ cxx_fold_indirect_ref (location_t loc, tree type, tree op0, bool *empty_base)
 		    break;
 		  }
 	    }
+	  /* *(&A[i] p+ j) => A[i + j] */
+	  else if (TREE_CODE (op00) == ARRAY_REF
+		   && TREE_CODE (TREE_OPERAND (op00, 1)) == INTEGER_CST
+		   && TREE_CODE (op01) == INTEGER_CST)
+	    {
+	      tree t = fold_convert_loc (loc, ssizetype,
+					 TREE_OPERAND (op00, 1));
+	      tree nelts
+		= array_type_nelts_top (TREE_TYPE (TREE_OPERAND (op00, 0)));
+	      /* Don't fold an out-of-bound access.  */
+	      if (!tree_int_cst_le (t, nelts))
+		return NULL_TREE;
+	      /* Make sure to treat the second operand of POINTER_PLUS_EXPR
+		 as signed.  */
+	      op01 = fold_build2_loc (loc, EXACT_DIV_EXPR, ssizetype,
+				      cp_fold_convert (ssizetype, op01),
+				      TYPE_SIZE_UNIT (type));
+	      t = size_binop_loc (loc, PLUS_EXPR, op01, t);
+	      return build4_loc (loc, ARRAY_REF, type, TREE_OPERAND (op00, 0),
+				 t, NULL_TREE, NULL_TREE);
+	    }
 	}
     }
   /* *(foo *)fooarrptr => (*fooarrptr)[0] */
diff --git gcc/testsuite/g++.dg/cpp0x/pr65398.C gcc/testsuite/g++.dg/cpp0x/pr65398.C
index e69de29..4aa7455 100644
--- gcc/testsuite/g++.dg/cpp0x/pr65398.C
+++ gcc/testsuite/g++.dg/cpp0x/pr65398.C
@@ -0,0 +1,62 @@
+// PR c++/65398
+// { dg-do compile { target c++11 } }
+
+#define SA(X) static_assert((X),#X)
+
+constexpr char s[] = "abc";
+constexpr char c1 = *(&s[0] + 0);
+constexpr char c2 = *(&s[0] + 1);
+constexpr char c3 = *(&s[1] + 0);
+constexpr char c4 = *(&s[1] + 1);
+constexpr char c5 = *(&s[2] + 0);
+constexpr char c6 = *(&s[0] + 2);
+constexpr char c7 = *(&s[2] + 1);
+
+constexpr char d1 = *(&s[4] - 0); // { dg-error "array subscript out of bound" }
+constexpr char d2 = *(&s[4] - 1);
+constexpr char d3 = *(&s[4] - 2);
+constexpr char d4 = *(&s[4] - 3);
+constexpr char d5 = *(&s[4] - 4);
+constexpr char d6 = *(&s[4] - 5);  // { dg-error "negative array subscript" }
+
+/* Don't accept invalid stuff.  */
+constexpr char e1 = *(&s[5] - 1); // { dg-error "is not a constant expression" }
+constexpr char e2 = *(&s[5] - 2); // { dg-error "is not a constant expression" }
+constexpr char e3 = *(&s[5] - 3); // { dg-error "is not a constant expression" }
+
+SA (c1 == 'a');
+SA (c2 == 'b');
+SA (c3 == 'b');
+SA (c4 == 'c');
+SA (c5 == 'c');
+SA (c6 == 'c');
+SA (c7 == '\0');
+
+constexpr int l[] = { 'c', 'd', 'e', '\0' };
+constexpr int i1 = *(&l[0] + 0);
+constexpr int i2 = *(&l[0] + 1);
+constexpr int i3 = *(&l[1] + 0);
+constexpr int i4 = *(&l[1] + 1);
+constexpr int i5 = *(&l[2] + 0);
+constexpr int i6 = *(&l[0] + 2);
+constexpr int i7 = *(&l[2] + 1);
+
+constexpr char j1 = *(&l[4] - 0); // { dg-error "array subscript out of bound" }
+constexpr char j2 = *(&l[4] - 1);
+constexpr char j3 = *(&l[4] - 2);
+constexpr char j4 = *(&l[4] - 3);
+constexpr char j5 = *(&l[4] - 4);
+constexpr char j6 = *(&l[4] - 5);  // { dg-error "negative array subscript" }
+
+/* Don't accept invalid stuff.  */
+constexpr char k1 = *(&l[5] - 1); // { dg-error "is not a constant expression" }
+constexpr char k2 = *(&l[5] - 2); // { dg-error "is not a constant expression" }
+constexpr char k3 = *(&l[5] - 3); // { dg-error "is not a constant expression" }
+
+SA (i1 == 'c');
+SA (i2 == 'd');
+SA (i3 == 'd');
+SA (i4 == 'e');
+SA (i5 == 'e');
+SA (i6 == 'e');
+SA (i7 == '\0');

	Marek

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected) (take 2)
  2015-03-20 20:57       ` C++ PATCH for c++/65398 (valid constexpr rejected) (take 2) Marek Polacek
@ 2015-03-20 21:04         ` Jakub Jelinek
  2015-03-20 21:10           ` Jason Merrill
  0 siblings, 1 reply; 15+ messages in thread
From: Jakub Jelinek @ 2015-03-20 21:04 UTC (permalink / raw)
  To: Marek Polacek; +Cc: Jason Merrill, Richard Biener, GCC Patches

Hi!

Just for completeness:

On Fri, Mar 20, 2015 at 09:56:52PM +0100, Marek Polacek wrote:
> +constexpr char d1 = *(&s[4] - 0); // { dg-error "array subscript out of bound" }
> +constexpr char d2 = *(&s[4] - 1);
> +constexpr char d3 = *(&s[4] - 2);
> +constexpr char d4 = *(&s[4] - 3);
> +constexpr char d5 = *(&s[4] - 4);
> +constexpr char d6 = *(&s[4] - 5);  // { dg-error "negative array subscript" }
> +
> +/* Don't accept invalid stuff.  */
> +constexpr char e1 = *(&s[5] - 1); // { dg-error "is not a constant expression" }
> +constexpr char e2 = *(&s[5] - 2); // { dg-error "is not a constant expression" }
> +constexpr char e3 = *(&s[5] - 3); // { dg-error "is not a constant expression" }
> +
> +SA (c1 == 'a');
> +SA (c2 == 'b');
> +SA (c3 == 'b');
> +SA (c4 == 'c');
> +SA (c5 == 'c');
> +SA (c6 == 'c');
> +SA (c7 == '\0');

Miss SA here for d2-d5.

> +constexpr char j1 = *(&l[4] - 0); // { dg-error "array subscript out of bound" }
> +constexpr char j2 = *(&l[4] - 1);
> +constexpr char j3 = *(&l[4] - 2);
> +constexpr char j4 = *(&l[4] - 3);
> +constexpr char j5 = *(&l[4] - 4);
> +constexpr char j6 = *(&l[4] - 5);  // { dg-error "negative array subscript" }

> +SA (i1 == 'c');
> +SA (i2 == 'd');
> +SA (i3 == 'd');
> +SA (i4 == 'e');
> +SA (i5 == 'e');
> +SA (i6 == 'e');
> +SA (i7 == '\0');

And SA here for j2-j5.

	Jakub

^ permalink raw reply	[flat|nested] 15+ messages in thread

* Re: C++ PATCH for c++/65398 (valid constexpr rejected) (take 2)
  2015-03-20 21:04         ` Jakub Jelinek
@ 2015-03-20 21:10           ` Jason Merrill
  0 siblings, 0 replies; 15+ messages in thread
From: Jason Merrill @ 2015-03-20 21:10 UTC (permalink / raw)
  To: Jakub Jelinek, Marek Polacek; +Cc: Richard Biener, GCC Patches

On 03/20/2015 05:03 PM, Jakub Jelinek wrote:
> Hi!
>
> Just for completeness:
>
> On Fri, Mar 20, 2015 at 09:56:52PM +0100, Marek Polacek wrote:
>> +constexpr char d1 = *(&s[4] - 0); // { dg-error "array subscript out of bound" }
>> +constexpr char d2 = *(&s[4] - 1);
>> +constexpr char d3 = *(&s[4] - 2);
>> +constexpr char d4 = *(&s[4] - 3);
>> +constexpr char d5 = *(&s[4] - 4);
>> +constexpr char d6 = *(&s[4] - 5);  // { dg-error "negative array subscript" }
>> +
>> +/* Don't accept invalid stuff.  */
>> +constexpr char e1 = *(&s[5] - 1); // { dg-error "is not a constant expression" }
>> +constexpr char e2 = *(&s[5] - 2); // { dg-error "is not a constant expression" }
>> +constexpr char e3 = *(&s[5] - 3); // { dg-error "is not a constant expression" }
>> +
>> +SA (c1 == 'a');
>> +SA (c2 == 'b');
>> +SA (c3 == 'b');
>> +SA (c4 == 'c');
>> +SA (c5 == 'c');
>> +SA (c6 == 'c');
>> +SA (c7 == '\0');
>
> Miss SA here for d2-d5.
>
>> +constexpr char j1 = *(&l[4] - 0); // { dg-error "array subscript out of bound" }
>> +constexpr char j2 = *(&l[4] - 1);
>> +constexpr char j3 = *(&l[4] - 2);
>> +constexpr char j4 = *(&l[4] - 3);
>> +constexpr char j5 = *(&l[4] - 4);
>> +constexpr char j6 = *(&l[4] - 5);  // { dg-error "negative array subscript" }
>
>> +SA (i1 == 'c');
>> +SA (i2 == 'd');
>> +SA (i3 == 'd');
>> +SA (i4 == 'e');
>> +SA (i5 == 'e');
>> +SA (i6 == 'e');
>> +SA (i7 == '\0');
>
> And SA here for j2-j5.

OK with these changes.

Jason


^ permalink raw reply	[flat|nested] 15+ messages in thread

end of thread, other threads:[~2015-03-20 21:10 UTC | newest]

Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-03-13 14:41 C++ PATCH for c++/65398 (valid constexpr rejected) Marek Polacek
2015-03-18 10:08 ` Richard Biener
2015-03-19 18:05   ` Jakub Jelinek
2015-03-19 18:13     ` Marek Polacek
2015-03-19 18:17       ` Jakub Jelinek
2015-03-19 18:33         ` Marek Polacek
2015-03-20 14:53     ` Jason Merrill
2015-03-20 14:59       ` Jakub Jelinek
2015-03-20 15:03         ` Jason Merrill
2015-03-20 15:11           ` Jakub Jelinek
2015-03-20 15:15             ` Jason Merrill
2015-03-20 15:28               ` Jakub Jelinek
2015-03-20 20:57       ` C++ PATCH for c++/65398 (valid constexpr rejected) (take 2) Marek Polacek
2015-03-20 21:04         ` Jakub Jelinek
2015-03-20 21:10           ` 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).