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?

Martin



Jason


Reply via email to