Skip to content

Commit c51e301

Browse files
eregoneregon
committed
[GR-19220] Remove support for keyword args in Java code (#2433)
PullRequest: truffleruby/2897
2 parents 975e42f + fa47fbe commit c51e301

File tree

5 files changed

+38
-73
lines changed

5 files changed

+38
-73
lines changed

‎src/annotations/java/org/truffleruby/builtins/CoreMethod.java

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -47,10 +47,6 @@
4747

4848
int optional() default 0;
4949

50-
/** Declares that a keyword argument with this name will be passed into the method as if it was an extra trailing
51-
* positional optional argument. */
52-
String keywordAsOptional() default "";
53-
5450
boolean rest() default false;
5551

5652
boolean needsBlock() default false;

‎src/main/java/org/truffleruby/builtins/CoreMethodNodeManager.java

Lines changed: 2 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,7 @@
3636
import org.truffleruby.language.methods.Split;
3737
import org.truffleruby.language.Visibility;
3838
import org.truffleruby.language.arguments.MissingArgumentBehavior;
39-
import org.truffleruby.language.arguments.NotProvidedNode;
4039
import org.truffleruby.language.arguments.ReadBlockFromCurrentFrameArgumentsNode;
41-
import org.truffleruby.language.arguments.ReadKeywordArgumentNode;
4240
import org.truffleruby.language.arguments.ReadPreArgumentNode;
4341
import org.truffleruby.language.arguments.ReadRemainingArgumentsNode;
4442
import org.truffleruby.language.arguments.ReadSelfNode;
@@ -132,14 +130,7 @@ private void addCoreMethod(RubyModule module, MethodDetails methodDetails) {
132130
final Visibility visibility = annotation.visibility();
133131
verifyUsage(module, methodDetails, annotation, visibility);
134132

135-
final String keywordAsOptional = annotation.keywordAsOptional().isEmpty()
136-
? null
137-
: annotation.keywordAsOptional();
138-
final Arity arity = createArity(
139-
annotation.required(),
140-
annotation.optional(),
141-
annotation.rest(),
142-
keywordAsOptional);
133+
final Arity arity = new Arity(annotation.required(), annotation.optional(), annotation.rest());
143134
final NodeFactory<? extends RubyBaseNode> nodeFactory = methodDetails.getNodeFactory();
144135
final boolean onSingleton = annotation.onSingleton() || annotation.constructor();
145136
final boolean isModuleFunc = annotation.isModuleFunction();
@@ -173,11 +164,10 @@ public void addLazyCoreMethod(
173164
int required,
174165
int optional,
175166
boolean rest,
176-
String keywordAsOptional,
177167
String... names) {
178168

179169
final RubyModule module = getModule(moduleName, isClass);
180-
final Arity arity = createArity(required, optional, rest, keywordAsOptional);
170+
final Arity arity = new Arity(required, optional, rest);
181171
final Split finalSplit;
182172
if (alwaysInlined) {
183173
finalSplit = Split.NEVER;
@@ -317,12 +307,6 @@ private static SharedMethodInfo makeSharedMethodInfo(String moduleName, boolean
317307
null);
318308
}
319309

320-
private static Arity createArity(int required, int optional, boolean rest, String keywordAsOptional) {
321-
return keywordAsOptional == null
322-
? new Arity(required, optional, rest)
323-
: new Arity(required, optional, rest, 0, new String[]{ keywordAsOptional }, true, false);
324-
}
325-
326310
public RootCallTarget createCoreMethodRootNode(NodeFactory<? extends RubyBaseNode> nodeFactory,
327311
RubyLanguage language, SharedMethodInfo sharedMethodInfo, Split split, CoreMethod method) {
328312

@@ -372,18 +356,6 @@ public RootCallTarget createCoreMethodRootNode(NodeFactory<? extends RubyBaseNod
372356
argumentsNodes[i++] = new ReadBlockFromCurrentFrameArgumentsNode();
373357
}
374358

375-
if (!method.keywordAsOptional().isEmpty()) {
376-
if (optional > 0) {
377-
throw new UnsupportedOperationException(
378-
"core method has been declared with both optional arguments and a keyword-as-optional argument");
379-
}
380-
381-
argumentsNodes[i++] = ReadKeywordArgumentNode.create(
382-
required,
383-
language.getSymbol(method.keywordAsOptional()),
384-
new NotProvidedNode());
385-
}
386-
387359
RubyNode node = (RubyNode) createNodeFromFactory(nodeFactory, argumentsNodes);
388360
RubyNode methodNode = transformResult(method, node);
389361

‎src/main/java/org/truffleruby/core/kernel/KernelNodes.java

Lines changed: 32 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
import org.truffleruby.builtins.NonStandard;
2929
import org.truffleruby.builtins.Primitive;
3030
import org.truffleruby.builtins.PrimitiveArrayArgumentsNode;
31+
import org.truffleruby.builtins.PrimitiveNode;
3132
import org.truffleruby.builtins.UnaryCoreMethodNode;
3233
import org.truffleruby.core.array.ArrayUtils;
3334
import org.truffleruby.core.array.RubyArray;
@@ -544,10 +545,10 @@ private DispatchingNode allocateNode() {
544545
}
545546
}
546547

547-
@CoreMethod(names = "clone", keywordAsOptional = "freeze")
548-
@NodeChild(value = "self", type = RubyNode.class)
548+
@Primitive(name = "object_clone")
549+
@NodeChild(value = "object", type = RubyNode.class)
549550
@NodeChild(value = "freeze", type = RubyNode.class)
550-
public abstract static class CloneNode extends CoreMethodNode {
551+
public abstract static class CloneNode extends PrimitiveNode {
551552

552553
@Child private CopyNode copyNode = CopyNode.create();
553554
@Child private DispatchNode initializeCloneNode = DispatchNode.create();
@@ -559,95 +560,95 @@ protected RubyNode coerceToBoolean(RubyNode freeze) {
559560
}
560561

561562
@Specialization(limit = "getRubyLibraryCacheLimit()")
562-
protected RubyDynamicObject clone(RubyDynamicObject self, boolean freeze,
563+
protected RubyDynamicObject clone(RubyDynamicObject object, boolean freeze,
563564
@Cached ConditionProfile isSingletonProfile,
564565
@Cached ConditionProfile freezeProfile,
565566
@Cached ConditionProfile isFrozenProfile,
566567
@Cached ConditionProfile isRubyClass,
567-
@CachedLibrary("self") RubyLibrary rubyLibrary,
568+
@CachedLibrary("object") RubyLibrary rubyLibrary,
568569
@CachedLibrary(limit = "getRubyLibraryCacheLimit()") RubyLibrary rubyLibraryFreeze) {
569-
final RubyDynamicObject newObject = copyNode.executeCopy(self);
570+
final RubyDynamicObject newObject = copyNode.executeCopy(object);
570571

571572
// Copy the singleton class if any.
572-
final RubyClass selfMetaClass = self.getMetaClass();
573+
final RubyClass selfMetaClass = object.getMetaClass();
573574
if (isSingletonProfile.profile(selfMetaClass.isSingleton)) {
574575
final RubyClass newObjectMetaClass = executeSingletonClass(newObject);
575576
newObjectMetaClass.fields.initCopy(selfMetaClass);
576577
}
577578

578-
initializeCloneNode.call(newObject, "initialize_clone", self);
579+
initializeCloneNode.call(newObject, "initialize_clone", object);
579580

580-
if (freezeProfile.profile(freeze) && isFrozenProfile.profile(rubyLibrary.isFrozen(self))) {
581+
if (freezeProfile.profile(freeze) && isFrozenProfile.profile(rubyLibrary.isFrozen(object))) {
581582
rubyLibraryFreeze.freeze(newObject);
582583
}
583584

584-
if (isRubyClass.profile(self instanceof RubyClass)) {
585-
((RubyClass) newObject).superclass = ((RubyClass) self).superclass;
585+
if (isRubyClass.profile(object instanceof RubyClass)) {
586+
((RubyClass) newObject).superclass = ((RubyClass) object).superclass;
586587
}
587588

588589
return newObject;
589590
}
590591

591592
@Specialization
592-
protected Object cloneBoolean(boolean self, boolean freeze,
593+
protected Object cloneBoolean(boolean object, boolean freeze,
593594
@Cached ConditionProfile freezeProfile) {
594595
if (freezeProfile.profile(!freeze)) {
595-
raiseCantUnfreezeError(self);
596+
raiseCantUnfreezeError(object);
596597
}
597-
return self;
598+
return object;
598599
}
599600

600601
@Specialization
601-
protected Object cloneInteger(int self, boolean freeze,
602+
protected Object cloneInteger(int object, boolean freeze,
602603
@Cached ConditionProfile freezeProfile) {
603604
if (freezeProfile.profile(!freeze)) {
604-
raiseCantUnfreezeError(self);
605+
raiseCantUnfreezeError(object);
605606
}
606-
return self;
607+
return object;
607608
}
608609

609610
@Specialization
610-
protected Object cloneLong(long self, boolean freeze,
611+
protected Object cloneLong(long object, boolean freeze,
611612
@Cached ConditionProfile freezeProfile) {
612613
if (freezeProfile.profile(!freeze)) {
613-
raiseCantUnfreezeError(self);
614+
raiseCantUnfreezeError(object);
614615
}
615-
return self;
616+
return object;
616617
}
617618

618619
@Specialization
619-
protected Object cloneFloat(double self, boolean freeze,
620+
protected Object cloneFloat(double object, boolean freeze,
620621
@Cached ConditionProfile freezeProfile) {
621622
if (freezeProfile.profile(!freeze)) {
622-
raiseCantUnfreezeError(self);
623+
raiseCantUnfreezeError(object);
623624
}
624-
return self;
625+
return object;
625626
}
626627

627-
@Specialization(guards = "!isImmutableRubyString(value)")
628-
protected Object cloneImmutableObject(ImmutableRubyObject value, boolean freeze,
628+
@Specialization(guards = "!isImmutableRubyString(object)")
629+
protected Object cloneImmutableObject(ImmutableRubyObject object, boolean freeze,
629630
@Cached ConditionProfile freezeProfile) {
630631
if (freezeProfile.profile(!freeze)) {
631-
raiseCantUnfreezeError(value);
632+
raiseCantUnfreezeError(object);
632633
}
633-
return value;
634+
return object;
634635
}
635636

636637
@Specialization
637-
protected RubyDynamicObject cloneImmutableRubyString(ImmutableRubyString self, boolean freeze,
638+
protected RubyDynamicObject cloneImmutableRubyString(ImmutableRubyString object, boolean freeze,
638639
@Cached ConditionProfile freezeProfile,
639640
@CachedLibrary(limit = "getRubyLibraryCacheLimit()") RubyLibrary rubyLibraryFreeze,
640641
@Cached MakeStringNode makeStringNode) {
641-
final RubyDynamicObject newObject = makeStringNode.fromRope(self.rope, self.encoding);
642+
final RubyDynamicObject newObject = makeStringNode.fromRope(object.rope, object.encoding);
642643
if (freezeProfile.profile(freeze)) {
643644
rubyLibraryFreeze.freeze(newObject);
644645
}
645646

646647
return newObject;
647648
}
648649

649-
private void raiseCantUnfreezeError(Object self) {
650-
throw new RaiseException(getContext(), coreExceptions().argumentErrorCantUnfreeze(self, this));
650+
private void raiseCantUnfreezeError(Object object) {
651+
throw new RaiseException(getContext(), coreExceptions().argumentErrorCantUnfreeze(object, this));
651652
}
652653

653654
private RubyClass executeSingletonClass(RubyDynamicObject newObject) {

‎src/main/ruby/truffleruby/core/kernel.rb

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -749,6 +749,10 @@ def fork
749749
Primitive.method_unimplement method(:fork)
750750
Primitive.method_unimplement nil.method(:fork)
751751

752+
def clone(freeze: true)
753+
Primitive.object_clone self, freeze
754+
end
755+
752756
Truffle::Boot.delay do
753757
if Truffle::Boot.get_option('gets-loop')
754758
def chomp(separator=$/)

‎src/processor/java/org/truffleruby/processor/CoreModuleProcessor.java

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -285,10 +285,6 @@ private void processCoreMethod(
285285
coreMethod.required() + ", " +
286286
coreMethod.optional() + ", " +
287287
coreMethod.rest() + ", " +
288-
(coreMethod.keywordAsOptional().isEmpty()
289-
? "null"
290-
: quote(coreMethod.keywordAsOptional())) +
291-
", " +
292288
names + ");");
293289

294290
final boolean hasSelfArgument = !coreMethod.onSingleton() && !coreMethod.constructor() &&
@@ -338,10 +334,6 @@ private void processCoreMethod(
338334
args.add("*" + argumentNames.get(index));
339335
index++;
340336
}
341-
if (!coreMethod.keywordAsOptional().isEmpty()) {
342-
// TODO (pitr-ch 03-Oct-2019): check interaction with names, or remove it
343-
args.add(coreMethod.keywordAsOptional() + ": :unknown_default_value");
344-
}
345337
if (coreMethod.needsBlock()) {
346338
args.add("&" + argumentNames.get(index));
347339
}

0 commit comments

Comments
 (0)