ruby-core@ruby-lang.org archive (unofficial mirror)
 help / color / mirror / Atom feed
From: eregontp@gmail.com
To: ruby-core@ruby-lang.org
Subject: [ruby-core:99223] [Ruby master Bug#17017] Range#max & Range#minmax incorrectly use Float end as max
Date: Sun, 19 Jul 2020 09:59:46 +0000 (UTC)	[thread overview]
Message-ID: <redmine.journal-86604.20200719095945.41304@ruby-lang.org> (raw)
In-Reply-To: redmine.issue-17017.20200707201640.41304@ruby-lang.org

Issue #17017 has been updated by Eregon (Benoit Daloze).


marcandre (Marc-Andre Lafortune) wrote in #note-15:
> Unless I'm mistaken, this behavior change was not approved by Matz (or anybody else), changes a behavior that dates back to Ruby 1.8, breaks (that we know of) `activemodel` and `rubocop`, doesn't even raise the right error and isn't tested, ...

Which change is that?
The first PR above, https://github.com/ruby/ruby/pull/3306 ?
That seems to have some tests.

> Yet when you propose that that commit remains as is and that someone else has to bring up the situation with Matz, no other committer seems confused by this.

I think the first PR should already have matz's or at least someone's else review.

Range is full of edge and special cases though, and there seems to be no global vision for it.
Maybe a good step here would be try to define the semantics we'd like for it. Obviously it's quite some work.

If reverting the first PR helps, maybe we should do that first?

----------------------------------------
Bug #17017: Range#max & Range#minmax incorrectly use Float end as max
https://bugs.ruby-lang.org/issues/17017#change-86604

* Author: sambostock (Sam Bostock)
* Status: Open
* Priority: Normal
* ruby -v: ruby 2.8.0dev (2020-07-14T04:19:55Z master e60cd14d85) [x86_64-darwin17]
* Backport: 2.5: UNKNOWN, 2.6: UNKNOWN, 2.7: UNKNOWN
----------------------------------------
While continuing to add edge cases to [`Range#minmax` specs](https://github.com/ruby/spec/pull/777), I discovered the following bug:

```ruby
(1..3.1).to_a        == [1, 2, 3] # As expected

(1..3.1).to_a.max    == 3         # As expected
(1..3.1).to_a.minmax == [1, 3]    # As expected

(1..3.1).max    == 3.1            # Should be 3, as above
(1..3.1).minmax == [1, 3.1]       # Should be [1, 3], as above
```

One way to detect this scenario might be to do (whatever the C equivalent is of)

```ruby
range_end.is_a?(Numeric)                      // Is this a numeric range?
  && (range_end - range_begin).modulo(1) == 0 // Can we reach the range_end using the standard step size (1)
```

As for how to handle it, a couple options come to mind:

- We could error out and do something similar to what we do for exclusive ranges

```ruby
raise TypeError, 'cannot exclude non Integer end value'
```

- We might be able to calculate the range end by doing something like

```ruby
num_steps = (range_end / range_beg).to_i - 1 # one fewer steps than would exceed the range_end
max = range_beg + num_steps                  # take that many steps all at once
```

- We could delegate to `super` and enumerate the range to find the max

```ruby
super
```

- We could update the documentation to define the max for this case as the `range_end`, similarly to how the documentation for `include?` says it behaves like `cover?` for numeric ranges.



-- 
https://bugs.ruby-lang.org/

  parent reply	other threads:[~2020-07-19 10:00 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-07-07 20:16 [ruby-core:99079] [Ruby master Bug#17017] Range#max & Range#minmax incorrectly use Float end as max bosticko
2020-07-07 20:41 ` [ruby-core:99080] " bosticko
2020-07-07 20:43 ` [ruby-core:99081] " bosticko
2020-07-07 20:56 ` [ruby-core:99082] " bosticko
2020-07-10 16:29 ` [ruby-core:99113] " merch-redmine
2020-07-15  0:47 ` [ruby-core:99171] " koic.ito
2020-07-15  5:24 ` [ruby-core:99174] " merch-redmine
2020-07-15  7:19 ` [ruby-core:99175] " koic.ito
2020-07-15 15:37 ` [ruby-core:99178] " marcandre-ruby-core
2020-07-15 15:58 ` [ruby-core:99179] " merch-redmine
2020-07-15 16:40 ` [ruby-core:99181] " marcandre-ruby-core
2020-07-16  9:31 ` [ruby-core:99192] " yasuo.honda
2020-07-16 17:17 ` [ruby-core:99194] " merch-redmine
2020-07-19  1:17 ` [ruby-core:99216] " yasuo.honda
2020-07-19  6:41 ` [ruby-core:99222] " marcandre-ruby-core
2020-07-19  9:59 ` eregontp [this message]
2020-07-19 10:02 ` [ruby-core:99224] " eregontp
2020-07-19 12:38 ` [ruby-core:99225] " merch-redmine
2020-07-19 14:12 ` [ruby-core:99226] " marcandre-ruby-core
2020-07-19 14:19 ` [ruby-core:99227] " marcandre-ruby-core
2020-08-14 16:09 ` [ruby-core:99589] " daniel
2020-08-14 17:54 ` [ruby-core:99590] " merch-redmine
2020-09-01  5:13 ` [ruby-core:99813] " matz
2020-09-01 14:49 ` [ruby-core:99816] " merch-redmine
2020-09-01 17:54 ` [ruby-core:99821] " merch-redmine

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-list from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.ruby-lang.org/en/community/mailing-lists/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=redmine.journal-86604.20200719095945.41304@ruby-lang.org \
    --to=ruby-core@ruby-lang.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).