Re: [PING][PATCH] correct handling of indices into arrays with elements larger than 1 (PR c++/96511)

[Jason Merrill <jason@redhat.com>]

On 10/8/20 3:18 PM, Martin Sebor wrote:
> On 10/7/20 3:01 PM, Jason Merrill wrote:
>> On 10/7/20 4:11 PM, Martin Sebor wrote:
> ...
> 
>>>>>>>>>> For the various member functions, please include the comments 
>>>>>>>>>> with the definition as well as the in-class declaration.
>>>>>>>>>
>>>>>>>>> Only one access_ref member function is defined out-of-line: 
>>>>>>>>> offset_bounded().  I've adjusted the comment and copied it above
>>>>>>>>> the function definition.
>>
>> And size_remaining, as quoted above?
> 
> I have this in my tree:
> 
> /* Return the maximum amount of space remaining and if non-null, set
>     argument to the minimum.  */
> 
> I'll add it when I commit the patch.
> 
>>
>> I also don't see a comment above the definition of offset_bounded in 
>> the new patch?
> 
> There is a comment in the latest patch.
> 
> ...
>>>>>>>>> The goal of conditionals is to avoid overwhelming the user with
>>>>>>>>> excessive numbers that may not be meaningful or even relevant
>>>>>>>>> to the warning.  I've corrected the function body, tweaked and
>>>>>>>>> renamed the get_range function to get_offset_range to do a better
>>>>>>>>> job of extracting ranges from the types of some nonconstant
>>>>>>>>> expressions the front end passes it, and added a new test for
>>>>>>>>> all this.  Attached is the new revision.
>>
>> offset_bounded looks unchanged in the new patch.  It still returns 
>> true iff either the range is a single value or one of the bounds are 
>> unrepresentable in ptrdiff_t.  I'm still unclear how this corresponds 
>> to "Return true if OFFRNG is bounded to a subrange of possible offset 
>> values."
> 
> I don't think you're looking at the latest patch.  It has this:
> 
> +/* Return true if OFFRNG is bounded to a subrange of offset values
> +   valid for the largest possible object.  */
> +
>   bool
>   access_ref::offset_bounded () const
>   {
> -  if (offrng[0] == offrng[1])
> -    return false;
> -
>     tree min = TYPE_MIN_VALUE (ptrdiff_type_node);
>     tree max = TYPE_MAX_VALUE (ptrdiff_type_node);
> -  return offrng[0] <= wi::to_offset (min) || offrng[1] >= wi::to_offset 
> (max);
> +  return wi::to_offset (min) <= offrng[0] && offrng[1] <= wi::to_offset 
> (max);
>   }
> 
> Here's a link to it in the archive:
> 
> https://gcc.gnu.org/pipermail/gcc-patches/2020-September/555019.html
> https://gcc.gnu.org/pipermail/gcc-patches/attachments/20200928/9026783a/attachment-0003.bin 

Ah, yes, there are two patches in that email; the first introduces the 
broken offset_bounded, and the second one fixes it without mentioning 
that in the ChangeLog.  How about moving the fix to the first patch?

Jason