On Thu, Jan 25, 2018 at 5:53 PM, Jakub Jelinek <ja...@redhat.com> wrote:
> 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.
But if lval is false we want the value of the array, and if we know
the value we know the bounds, so that shouldn't be needed.
Jason
