Skip to content

Commit 6ec3567

Browse files
author
Nicolas Laurent
committed
[GR-20070] Make Array#concat work with multiple or no arguments.
PullRequest: truffleruby/1191
2 parents ba88208 + 253a34f commit 6ec3567

File tree

4 files changed

+71
-20
lines changed

4 files changed

+71
-20
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ Bug fixes:
3939
* `Kernel#sprintf` encoding validity has been fixed (#1852, @XrXr).
4040
* Fixed File.fnmatch causes ArrayIndexOutOfBoundsException (#1845).
4141
* Make `String#concat` work with no or multiple arguments (#1519).
42+
* Make `Array#concat` work with no or multiple arguments (#1519).
4243

4344
Compatibility:
4445

‎spec/tags/core/array/concat_tags.txt

Lines changed: 0 additions & 2 deletions
This file was deleted.

‎src/main/java/org/truffleruby/core/array/ArrayNodes.java

Lines changed: 63 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,9 @@
2727
import org.truffleruby.core.Hashing;
2828
import org.truffleruby.core.array.ArrayEachIteratorNode.ArrayElementConsumerNode;
2929
import org.truffleruby.core.array.ArrayNodesFactory.ReplaceNodeFactory;
30+
import org.truffleruby.core.array.ArrayOperationNodes.ArrayExtractRangeCopyOnWriteNode;
3031
import org.truffleruby.core.cast.CmpIntNode;
32+
import org.truffleruby.core.cast.ToAryNode;
3133
import org.truffleruby.core.cast.ToAryNodeGen;
3234
import org.truffleruby.core.cast.ToIntNode;
3335
import org.truffleruby.core.cast.ToIntNodeGen;
@@ -328,7 +330,7 @@ public abstract static class CompactNode extends ArrayCoreMethodNode {
328330
@Specialization(guards = { "strategy.matches(array)", "strategy.isPrimitive()" }, limit = "STORAGE_STRATEGIES")
329331
protected DynamicObject compactPrimitive(DynamicObject array,
330332
@Cached("of(array)") ArrayStrategy strategy,
331-
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayOperationNodes.ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode) {
333+
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode) {
332334
final int size = strategy.getSize(array);
333335
Object compactMirror = extractRangeCopyOnWriteNode.execute(array, 0, size);
334336
return createArray(compactMirror, size);
@@ -406,25 +408,70 @@ protected Object compactObjectsNonMutable(DynamicObject array,
406408

407409
}
408410

409-
@CoreMethod(names = "concat", required = 1, raiseIfFrozenSelf = true)
410-
@NodeChild(value = "array", type = RubyNode.class)
411-
@NodeChild(value = "other", type = RubyNode.class)
411+
@CoreMethod(names = "concat", optional = 1, rest = true, raiseIfFrozenSelf = true)
412412
@ImportStatic(ArrayGuards.class)
413+
@NodeChild(value = "array", type = RubyNode.class)
414+
@NodeChild(value = "first", type = RubyNode.class)
415+
@NodeChild(value = "rest", type = RubyNode.class)
413416
public abstract static class ConcatNode extends CoreMethodNode {
414417

415-
@Child private ArrayAppendManyNode appendManyNode = ArrayAppendManyNodeGen.create();
416-
417-
@CreateCast("other")
418-
protected RubyNode coerceOtherToAry(RubyNode other) {
419-
return ToAryNodeGen.create(other);
418+
@Specialization(guards = "rest.length == 0")
419+
protected DynamicObject concatZero(DynamicObject array, NotProvided first, Object[] rest) {
420+
return array;
420421
}
421422

422-
@Specialization
423-
protected DynamicObject concat(DynamicObject array, DynamicObject other) {
424-
appendManyNode.executeAppendMany(array, other);
423+
@Specialization(guards = "rest.length == 0")
424+
protected DynamicObject concatOne(DynamicObject array, DynamicObject first, Object[] rest,
425+
@Cached("createInternal()") ToAryNode toAryNode,
426+
@Cached ArrayAppendManyNode appendManyNode) {
427+
appendManyNode.executeAppendMany(array, toAryNode.executeToAry(first));
425428
return array;
426429
}
427430

431+
@ExplodeLoop
432+
@Specialization(guards = { "wasProvided(first)", "rest.length > 0", "rest.length == cachedLength" })
433+
protected Object concatMany(DynamicObject array, DynamicObject first, Object[] rest,
434+
@Cached("rest.length") int cachedLength,
435+
@Cached("createInternal()") ToAryNode toAryNode,
436+
@Cached ArrayAppendManyNode appendManyNode,
437+
@Cached("createBinaryProfile()") ConditionProfile selfArgProfile,
438+
@Cached("of(array)") ArrayStrategy strategy,
439+
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayExtractRangeCopyOnWriteNode extractRangeNode) {
440+
int size = Layouts.ARRAY.getSize(array);
441+
Object store = extractRangeNode.execute(array, 0, size);
442+
DynamicObject copy = createArray(store, size);
443+
DynamicObject result = appendManyNode.executeAppendMany(array, toAryNode.executeToAry(first));
444+
for (int i = 0; i < cachedLength; ++i) {
445+
if (selfArgProfile.profile(rest[i] == array)) {
446+
result = appendManyNode.executeAppendMany(array, copy);
447+
} else {
448+
result = appendManyNode.executeAppendMany(array, toAryNode.executeToAry(rest[i]));
449+
}
450+
}
451+
return result;
452+
}
453+
454+
/** Same implementation as {@link #concatMany}, safe for the use of {@code cachedLength} */
455+
@Specialization(guards = { "wasProvided(first)", "rest.length > 0" }, replaces = "concatMany")
456+
protected Object concatManyGeneral(DynamicObject array, DynamicObject first, Object[] rest,
457+
@Cached("createInternal()") ToAryNode toAryNode,
458+
@Cached ArrayAppendManyNode appendManyNode,
459+
@Cached("createBinaryProfile()") ConditionProfile selfArgProfile,
460+
@Cached("of(array)") ArrayStrategy strategy,
461+
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayExtractRangeCopyOnWriteNode extractRangeNode) {
462+
int size = Layouts.ARRAY.getSize(array);
463+
Object store = extractRangeNode.execute(array, 0, size);
464+
465+
DynamicObject result = appendManyNode.executeAppendMany(array, toAryNode.executeToAry(first));
466+
for (Object arg : rest) {
467+
if (selfArgProfile.profile(arg == array)) {
468+
result = appendManyNode.executeAppendMany(array, createArray(store, size));
469+
} else {
470+
result = appendManyNode.executeAppendMany(array, toAryNode.executeToAry(arg));
471+
}
472+
}
473+
return result;
474+
}
428475
}
429476

430477
@CoreMethod(names = "delete", required = 1, needsBlock = true)
@@ -1635,7 +1682,7 @@ protected RubyNode coerceOtherToAry(RubyNode index) {
16351682
protected DynamicObject replace(DynamicObject array, DynamicObject other,
16361683
@Cached("of(array)") ArrayStrategy arrayStrategy,
16371684
@Cached("of(other)") ArrayStrategy otherStrategy,
1638-
@Cached("otherStrategy.extractRangeCopyOnWriteNode()") ArrayOperationNodes.ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode) {
1685+
@Cached("otherStrategy.extractRangeCopyOnWriteNode()") ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode) {
16391686
propagateSharingNode.propagate(array, other);
16401687

16411688
final int size = getSize(other);
@@ -1830,7 +1877,7 @@ protected Object shiftEmpty(DynamicObject array, NotProvided n) {
18301877
@Specialization(guards = { "strategy.matches(array)", "!isEmptyArray(array)" }, limit = "STORAGE_STRATEGIES")
18311878
protected Object shiftOther(DynamicObject array, NotProvided n,
18321879
@Cached("of(array)") ArrayStrategy strategy,
1833-
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayOperationNodes.ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode,
1880+
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode,
18341881
@Cached("strategy.getNode()") ArrayOperationNodes.ArrayGetNode getNode) {
18351882
final int size = strategy.getSize(array);
18361883
final Object value = getNode.execute(Layouts.ARRAY.getStore(array), 0);
@@ -1862,8 +1909,8 @@ protected Object shiftManyEmpty(DynamicObject array, int n) {
18621909
limit = "STORAGE_STRATEGIES")
18631910
protected Object shiftMany(DynamicObject array, int n,
18641911
@Cached("of(array)") ArrayStrategy strategy,
1865-
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayOperationNodes.ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode1,
1866-
@Cached("strategy.sharedStorageStrategy().extractRangeCopyOnWriteNode()") ArrayOperationNodes.ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode2,
1912+
@Cached("strategy.extractRangeCopyOnWriteNode()") ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode1,
1913+
@Cached("strategy.sharedStorageStrategy().extractRangeCopyOnWriteNode()") ArrayExtractRangeCopyOnWriteNode extractRangeCopyOnWriteNode2,
18671914
@Cached("createBinaryProfile()") ConditionProfile minProfile) {
18681915
final int size = strategy.getSize(array);
18691916
final int numShift = minProfile.profile(size < n) ? size : n;

‎src/main/java/org/truffleruby/core/cast/ToAryNode.java

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@
1919
import com.oracle.truffle.api.dsl.Cached;
2020
import com.oracle.truffle.api.dsl.NodeChild;
2121
import com.oracle.truffle.api.dsl.Specialization;
22-
import com.oracle.truffle.api.frame.VirtualFrame;
2322
import com.oracle.truffle.api.object.DynamicObject;
2423
import com.oracle.truffle.api.profiles.BranchProfile;
2524

@@ -29,13 +28,19 @@ public abstract class ToAryNode extends RubyNode {
2928

3029
@Child private CallDispatchHeadNode toAryNode;
3130

31+
public static ToAryNode createInternal() {
32+
return ToAryNodeGen.create(null);
33+
}
34+
35+
public abstract DynamicObject executeToAry(Object object);
36+
3237
@Specialization(guards = "isRubyArray(array)")
3338
protected DynamicObject coerceRubyArray(DynamicObject array) {
3439
return array;
3540
}
3641

3742
@Specialization(guards = "!isRubyArray(object)")
38-
protected DynamicObject coerceObject(VirtualFrame frame, Object object,
43+
protected DynamicObject coerceObject(Object object,
3944
@Cached BranchProfile errorProfile) {
4045
if (toAryNode == null) {
4146
CompilerDirectives.transferToInterpreterAndInvalidate();

0 commit comments

Comments
 (0)