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

[Jason Merrill <jason@redhat.com>]

On 10/9/20 10:51 AM, Martin Sebor wrote:
> On 10/8/20 1:40 PM, Jason Merrill wrote:
>> 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?
> 
> Sure, I can do that.  Anything else or is the final version okay
> to commit with this adjustment?

OK with that adjustment.

Jason