Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ruby 2.7: Beginless ranges fully supported #2211

Merged
merged 12 commits into from
Jan 12, 2021
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,10 @@ Compatibility:
* Allow `Range#include?` and `#member?` with `Time` (#2202, @wildmaples).
* Implemented `Comparable#clamp(Range)` (#2200, @wildmaples).
* Added a `Array#minmax` to override `Enumerable#minmax` (#2199, @wildmaples).
* Added beginless range support for `Range#{new, bsearch, count, each, equal_value, first, inspect, max, min, size, cover?, include?, ===}`.
* Added beginless range support for `Array#{[], []=, slice, slice!, to_a, fill, values_at}` (#2155, @LillianZ).
* Added beginless range support for `String#{byteslice, slice, slice!}` and `Symbol#slice` (#2211, @LillianZ).
* Added beginless range support for `Kernel#{caller, caller_locations}` and `Thread#backtrace_locations` (#2211, @LillianZ).

Performance:

Expand Down
8 changes: 8 additions & 0 deletions spec/ruby/core/kernel/caller_locations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,14 @@
end
end

ruby_version_is "2.7" do
it "works with beginless ranges" do
locations1 = caller_locations(0)
locations2 = caller_locations(eval("(...5)"))
locations2.map(&:to_s)[eval("(2..)")].should == locations1[eval("(...5)")].map(&:to_s)[eval("(2..)")]
end
end

it "can be called with a range whose end is negative" do
locations1 = caller_locations(0)
locations2 = caller_locations(2..-1)
Expand Down
8 changes: 8 additions & 0 deletions spec/ruby/core/kernel/caller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,14 @@
end
end

ruby_version_is "2.7" do
it "works with beginless ranges" do
locations1 = KernelSpecs::CallerTest.locations(0)
locations2 = KernelSpecs::CallerTest.locations(eval("(..5)"))
locations2.map(&:to_s)[eval("(2..)")].should == locations1[eval("(..5)")].map(&:to_s)[eval("(2..)")]
end
end

guard -> { Kernel.instance_method(:tap).source_location } do
it "includes core library methods defined in Ruby" do
file, line = Kernel.instance_method(:tap).source_location
Expand Down
42 changes: 42 additions & 0 deletions spec/ruby/core/range/shared/cover.rb
Original file line number Diff line number Diff line change
Expand Up @@ -152,4 +152,46 @@
end
end
end
ruby_version_is "2.7" do
it "allows self to be a beginless range" do
eval("(...10)").send(@method, (3..7)).should be_true
eval("(...10)").send(@method, (3..15)).should be_false

eval("(..7.9)").send(@method, (2.5..6.5)).should be_true
eval("(..7.9)").send(@method, (2.5..8.5)).should be_false

eval("(..'i')").send(@method, ('d'..'f')).should be_true
eval("(..'i')").send(@method, ('d'..'z')).should be_false
end

it "allows self to be a endless range" do
eval("(0...)").send(@method, (3..7)).should be_true
eval("(5...)").send(@method, (3..15)).should be_false

eval("(1.1..)").send(@method, (2.5..6.5)).should be_true
eval("(3.3..)").send(@method, (2.5..8.5)).should be_false

eval("('a'..)").send(@method, ('d'..'f')).should be_true
eval("('p'..)").send(@method, ('d'..'z')).should be_false
end

it "accepts beginless range argument" do
eval("(..10)").send(@method, eval("(...10)")).should be_true
(0..10).send(@method, eval("(...10)")).should be_false

(1.1..7.9).send(@method, eval("(...10.5)")).should be_false

('c'..'i').send(@method, eval("(..'i')")).should be_false
end

it "accepts endless range argument" do
eval("(0..)").send(@method, eval("(0...)")).should be_true
(0..10).send(@method, eval("(0...)")).should be_false

(1.1..7.9).send(@method, eval("(0.8...)")).should be_false

('c'..'i').send(@method, eval("('a'..)")).should be_false
end

end
end
7 changes: 7 additions & 0 deletions spec/ruby/core/range/shared/cover_and_include.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,13 @@
end
end

ruby_version_is "2.7" do
it "returns true if other is an element of self for beginless ranges" do
eval("(..10)").send(@method, 2.4).should == true
eval("(...10.5)").send(@method, 2.4).should == true
end
end

it "compares values using <=>" do
rng = (1..5)
m = mock("int")
Expand Down
7 changes: 7 additions & 0 deletions spec/ruby/core/range/to_s_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,13 @@
end
end

ruby_version_is "2.7" do
it "can show beginless ranges" do
eval("(..1)").to_s.should == "..1"
eval("(...1.0)").to_s.should == "...1.0"
end
end

ruby_version_is ''...'2.7' do
it "returns a tainted string if either end is tainted" do
(("a".taint)..."c").to_s.tainted?.should be_true
Expand Down
10 changes: 10 additions & 0 deletions spec/ruby/core/string/shared/slice.rb
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,16 @@
"hello there".send(@method, eval("(-4...)")).should == "here"
end
end

ruby_version_is "2.7" do
it "works with beginless ranges" do
"hello there".send(@method, eval("(..5)")).should == "hello "
"hello there".send(@method, eval("(...5)")).should == "hello"
"hello there".send(@method, eval("(..-4)")).should == "hello th"
"hello there".send(@method, eval("(...-4)")).should == "hello t"
"hello there".send(@method, eval("(...nil)")).should == "hello there"
end
end
end

describe :string_slice_regexp, shared: true do
Expand Down
8 changes: 8 additions & 0 deletions spec/ruby/core/thread/backtrace_locations_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,14 @@
end
end

ruby_version_is "2.7" do
it "can be called with an beginless range" do
locations1 = Thread.current.backtrace_locations(0)
locations2 = Thread.current.backtrace_locations(eval("(..5)"))
locations2.map(&:to_s)[eval("(2..)")].should == locations1[eval("(..5)")].map(&:to_s)[eval("(2..)")]
end
end

it "returns nil if omitting more locations than available" do
Thread.current.backtrace_locations(100).should == nil
Thread.current.backtrace_locations(100..-1).should == nil
Expand Down
13 changes: 13 additions & 0 deletions src/main/java/org/truffleruby/core/string/StringNodes.java
Original file line number Diff line number Diff line change
Expand Up @@ -656,12 +656,25 @@ protected Object sliceEndlessRange(Object string, RubyObjectRange range, NotProv
return sliceRange(string, libString, toLong(range.begin), stringEnd, range.excludedEnd);
}

@Specialization(guards = "range.isBeginless()")
protected Object sliceBeginlessRange(Object string, RubyObjectRange range, NotProvided length,
@CachedLibrary(limit = "2") RubyStringLibrary libString) {
return sliceRange(string, libString, 0L, toLong(range.end), range.excludedEnd);
}

@Specialization(guards = "range.isBounded()")
protected Object sliceObjectRange(Object string, RubyObjectRange range, NotProvided length,
@CachedLibrary(limit = "2") RubyStringLibrary libString) {
return sliceRange(string, libString, toLong(range.begin), toLong(range.end), range.excludedEnd);
}

@Specialization(guards = "range.isBoundless()")
protected Object sliceBoundlessRange(Object string, RubyObjectRange range, NotProvided length,
@CachedLibrary(limit = "2") RubyStringLibrary libString) {
final int stringEnd = range.excludedEnd ? Integer.MAX_VALUE : Integer.MAX_VALUE - 1;
return sliceRange(string, libString, 0L, stringEnd, range.excludedEnd);
}

// endregion
// region Range Slice Logic

Expand Down
28 changes: 17 additions & 11 deletions src/main/ruby/truffleruby/core/kernel.rb
Original file line number Diff line number Diff line change
Expand Up @@ -726,17 +726,23 @@ def printf(*args)
end

def caller(start = 1, limit = nil)
args = if start.is_a? Range
if Primitive.nil? start.end
[start.begin + 1]
else
[start.begin + 1, start.size]
end
elsif Primitive.nil? limit
[start + 1]
else
[start + 1, limit]
end
args = if start.is_a? Range
if Primitive.nil?(start.begin) and Primitive.nil?(start.end)
[1]
elsif Primitive.nil? start.begin
size = start.end + 1
size -= 1 if start.exclude_end?
[1, size]
elsif Primitive.nil? start.end
[start.begin + 1]
else
[start.begin + 1, start.size]
end
elsif Primitive.nil? limit
[start + 1]
else
[start + 1, limit]
end
Kernel.caller_locations(*args).map(&:to_s)
end
module_function :caller
Expand Down
6 changes: 5 additions & 1 deletion src/main/ruby/truffleruby/core/truffle/kernel_operations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,11 @@ def self.normalize_backtrace_args(omit, length)
end
if Range === omit
range = omit
omit = Primitive.rb_to_int(range.begin)
if Primitive.nil? range.begin
omit = 0
else
omit = Primitive.rb_to_int(range.begin)
end
unless Primitive.nil? range.end
end_index = Primitive.rb_to_int(range.end)
if end_index < 0
Expand Down
87 changes: 42 additions & 45 deletions src/main/ruby/truffleruby/core/truffle/range_operations.rb
Original file line number Diff line number Diff line change
Expand Up @@ -74,66 +74,63 @@ def self.validate_step_size(first, last, step_size)

# MRI: r_cover_range_p
def self.range_cover?(range, other)
e = range.end
other_b = other.begin
other_e = other.end
b1 = range.begin
b2 = other.begin
e1 = range.end
e2 = other.end

return false if !Primitive.nil?(e) && Primitive.nil?(other_e)
return false if Primitive.nil?(b2) && !Primitive.nil?(b1)
return false if Primitive.nil?(e2) && !Primitive.nil?(e1)

return false if !Primitive.nil?(other_e) &&
range_less(other_b, other_e) > (other.exclude_end? ? -1 : 0)
return false unless (Primitive.nil?(b2) || cover?(range, b2))

return false unless cover?(range, other_b)
return true if Primitive.nil?(e2)

compare_end = range_less(e, other_e)

if (range.exclude_end? && other.exclude_end?) ||
(!range.exclude_end? && !other.exclude_end?)
return compare_end >= 0
elsif range.exclude_end?
return compare_end > 0
elsif compare_end >= 0
return true
if e1 == e2
!(range.exclude_end? && !other.exclude_end?)
else
if Integer === e2 && other.exclude_end?
cover?(range, e2 -1)
else
cover?(range, e2)
end
end
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as cover_range?, hard to follow because of the existing code.

Proposal (completely untested):

def self.cover?(range, value)
  # Check lower bound.
  if !Primitive.nil?(range.begin)
    # MRI uses <=> to compare, so must we.
    cmp = (range.begin <=> value)
    return false if Compare.compare_int(cmp) > 0
  end

  # Check upper bound.
  if !Primitive.nil(range.end)
    value = value + (range.exclude_end? 1 : 0)
    cmp = (value <=> range.end)
    return false if Comparable.compare_int(cmp) > 0
  end

  true
end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also much more beautiful, thanks!

Also, r_less is no longer being used, so I commented it out; not sure if it should be removed altogether.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, let's remove range_less if it's unused :)
(we can always use the git history to restore it if it's useful in the future)


other_max = begin
other.max
rescue TypeError
nil
end
return false if Primitive.nil?(other_max)

range_less(e, other_max) >= 0
end

# MRI: r_cover_p
# # MRI: r_cover_p
def self.cover?(range, value)
# MRI uses <=> to compare, so must we.
beg_compare = (range.begin <=> value)
return false unless beg_compare

if Comparable.compare_int(beg_compare) <= 0
return true if Primitive.nil? range.end
end_compare = (value <=> range.end)
# Check lower bound.
if !Primitive.nil?(range.begin)
# MRI uses <=> to compare, so must we.
cmp = (range.begin <=> value)
return false unless cmp
return false if Comparable.compare_int(cmp) > 0
end

# Check upper bound.
if !Primitive.nil?(range.end)
cmp = (value <=> range.end)
if Primitive.nil? cmp
p range, value
end
if range.exclude_end?
return true if Comparable.compare_int(end_compare) < 0
return false if Comparable.compare_int(cmp) >= 0
else
return true if Comparable.compare_int(end_compare) <= 0
return false if Comparable.compare_int(cmp) > 0
end
end

false
true
end

# MRI: r_less
def self.range_less(a, b)
compare = a <=> b
if Primitive.nil?(compare)
1
else
Comparable.compare_int(compare)
end
end
# # MRI: r_less
# def self.range_less(a, b)
# compare = a <=> b
# if Primitive.nil?(compare)
# 1
# else
# Comparable.compare_int(compare)
# end
# end
end
end