Skip to content

Commit 1cc58d6

Browse files
committed
[GR-38430] Fix String#[Regexp, Integer] when the capture group exists but is not matched
PullRequest: truffleruby/3325
2 parents dc3fc7e + 44cb454 commit 1cc58d6

File tree

3 files changed

+24
-5
lines changed

3 files changed

+24
-5
lines changed

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ Bug fixes:
1010
* Fix `MatchData#[]` exception when supplying a large negative index along with a length argument (@nirvdrum).
1111
* Fix capacity computation for huge `Hash` (#2635, @eregon).
1212
* Fix aliased methods to return the correct owner when method is from a superclass (@bjfish).
13+
* Fix `String#[Regexp, Integer]` when the capture group exists but is not matched (@eregon).
1314

1415
Compatibility:
1516

spec/ruby/core/string/shared/slice.rb

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -375,10 +375,20 @@
375375
"hello there".send(@method, /(what?)/, 1).should == nil
376376
end
377377

378+
it "returns nil if the index is larger than the number of captures" do
379+
"hello there".send(@method, /hello (.)/, 2).should == nil
380+
# You can't refer to 0 using negative indices
381+
"hello there".send(@method, /hello (.)/, -2).should == nil
382+
end
383+
378384
it "returns nil if there is no capture for the given index" do
379385
"hello there".send(@method, /[aeiou](.)\1/, 2).should == nil
380-
# You can't refer to 0 using negative indices
381-
"hello there".send(@method, /[aeiou](.)\1/, -2).should == nil
386+
end
387+
388+
it "returns nil if the given capture group was not matched but still sets $~" do
389+
"test".send(@method, /te(z)?/, 1).should == nil
390+
$~[0].should == "te"
391+
$~[1].should == nil
382392
end
383393

384394
it "calls to_int on the given index" do

src/main/java/org/truffleruby/core/string/StringNodes.java

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -818,6 +818,8 @@ protected Object sliceCapture(VirtualFrame frame, Object string, RubyRegexp rege
818818
@Cached ReadCallerVariablesNode readCallerStorageNode,
819819
@Cached ConditionProfile unsetProfile,
820820
@Cached ConditionProfile sameThreadProfile,
821+
@Cached ConditionProfile notMatchedProfile,
822+
@Cached ConditionProfile captureSetProfile,
821823
@Cached StringDupAsStringInstanceNode dupNode) {
822824
final Object capture = RubyGuards.wasProvided(maybeCapture) ? maybeCapture : 0;
823825
final Object matchStrPair = callNode.call(
@@ -828,13 +830,19 @@ protected Object sliceCapture(VirtualFrame frame, Object string, RubyRegexp rege
828830
capture);
829831

830832
final SpecialVariableStorage variables = readCallerStorageNode.execute(frame);
831-
if (matchStrPair == nil) {
833+
if (notMatchedProfile.profile(matchStrPair == nil)) {
832834
variables.setLastMatch(nil, getContext(), unsetProfile, sameThreadProfile);
833835
return nil;
834836
} else {
835837
final Object[] array = (Object[]) ((RubyArray) matchStrPair).store;
836-
variables.setLastMatch(array[0], getContext(), unsetProfile, sameThreadProfile);
837-
return dupNode.executeDupAsStringInstance(array[1]);
838+
final Object matchData = array[0];
839+
final Object captureStringOrNil = array[1];
840+
variables.setLastMatch(matchData, getContext(), unsetProfile, sameThreadProfile);
841+
if (captureSetProfile.profile(captureStringOrNil != nil)) {
842+
return dupNode.executeDupAsStringInstance(captureStringOrNil);
843+
} else {
844+
return nil;
845+
}
838846
}
839847
}
840848

0 commit comments

Comments
 (0)