On Thu, Jan 25, 2018 at 01:13:56PM -0700, Martin Sebor wrote:
> On 01/24/2018 04:19 PM, Jakub Jelinek wrote:
> > Hi!
> > 
> > In constexpr evaluation of array references for arrays with unknown bounds,
> > we need to diagnose out of bounds accesses, but really don't know the bounds
> > at compile time, right now GCC will see nelts as error_mark_node + 1 and
> > will not consider them a constant expression at all.
> > From the clang commit message it seems that CWG is leaning towards allowing
> > &array_with_unknown_bound[0] and array_with_unknown_bound, but disallowing
> > any other indexes (i.e. assume the array could have zero elements).
> > The following patch implements that.  Bootstrapped/regtested on x86_64-linux
> > and i686-linux, ok for trunk?
> 
> Unless your goal is to have GCC 8 reject just some subset of
> expressions in the reported test case (and fix the rest in
> the future) then there still are other potentially invalid
> expressions that GCC will continue to accept.  For example,
> the initialization of p in f() is rejected with the patch but
> the use of the same expression in g() is still accepted (Clang
> silently accepts both).  It could be due to the same problem
> as bug 82877.  Other similar issues are being tracked in bug
> 82876 and 82874.

The goal is to fix the PR and improve the handling of unknown
bound arrays while at it.
There is no time nor good timing for something much more substantial.
After all, with the patch we still don't reject e.g.
some cases if g++.dg/cpp0x/pr83993.C testcase uses the [a-z]2 pointers
instead of corresponding array in the array refs it tests.

>   extern const int a[];
> 
>   constexpr int f ()
>   {
>     const int *p = &a[7], *q = &a[0];
>     return p - q;
>   }
> 
>   constexpr int g ()
>   {
>     return &a[7] - &a[0];
>   }
> 
>   constexpr const int i = f ();
>   constexpr const int j = g ();

I believe the fix for this is to save bodies of constexpr
functions/methods before the folding and do the constexpr evaluations on
those rather than on something we've folded already, but that is
certainly not suitable for GCC8.

> Regarding the patch, just a couple of minor nits:
> 
> The consensus we have reached some time back was not to quote
> integer constants so I would suggest to follow it in new code
> and take the opportunity to remove the quoting from surrounding
> code.
> 
>   https://gcc.gnu.org/wiki/DiagnosticsGuidelines#Quoting

In this case I was just extending existing warning and wanted
consistency with that.  Both can be changed in a GCC9 follow-up,
or if Jason/Nathan want it now, even for GCC8, sure.

> In addition, I would suggest to phrase the error as "array
> subscript value %E is used *with* array qD" as opposed to
> "on array."  If we wanted to make it clearer that it's

That looks reasonable.

> the fact that the index is non-zero is the problem mentioning
> it in the warning might help ("non-zero array subscript %E...")

Well, even zero subscript is wrong if lval is false, i.e. a[0] + 1
rather than e.g. &a[0].  So we'd need to have another wordings
for the case when the index is zero.

> Alternatively, since the bounds are unknown is evident from
> the type of the array (e.g., "int[]") just: "array subscript
> value %E may be outside the bounds of array %qD of type %qT"
> might be good enough.

Dunno, perhaps.

        Jakub

Reply via email to