Skip to content

Excessive kwargs duplicate key warns #7502

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.

Sign up for GitHub

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

Merged
enebo merged 1 commit into from
Nov 30, 2022
Merged

Excessive kwargs duplicate key warns #7502

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions core/src/main/java/org/jruby/ir/IRBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -715,7 +715,7 @@ private Operand buildRestKeywordArgs(HashNode keywordArgs, int[] flags) {
Variable splatValue = copy(new Hash(new ArrayList<>()));
for (KeyValuePair<Node, Node> pair: pairs) {
Operand splat = buildWithOrder(pair.getValue(), keywordArgs.containsVariableAssignment());
addInstr(new RuntimeHelperCall(splatValue, MERGE_KWARGS, new Operand[] { splatValue, splat }));
addInstr(new RuntimeHelperCall(splatValue, MERGE_KWARGS, new Operand[] { splatValue, splat, fals() }));
}

return splatValue;
Expand Down Expand Up @@ -3660,11 +3660,11 @@ public Operand buildHash(HashNode hashNode) {
hash = copy(new Hash(args, hashNode.isLiteral()));
args = new ArrayList<>(); // Used args but we may find more after the splat so we reset
} else if (!args.isEmpty()) {
addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, new Hash(args) }));
addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, new Hash(args), tru()}));
args = new ArrayList<>();
}
Operand splat = buildWithOrder(pair.getValue(), hasAssignments);
addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, splat}));
addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, splat, tru()}));
continue;
} else {
keyOperand = buildWithOrder(key, hasAssignments);
Expand All @@ -3676,7 +3676,7 @@ public Operand buildHash(HashNode hashNode) {
if (hash == null) { // non-**arg ordinary hash
hash = copy(new Hash(args, hashNode.isLiteral()));
} else if (!args.isEmpty()) { // ordinary hash values encountered after a **arg
addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, new Hash(args) }));
addInstr(new RuntimeHelperCall(hash, MERGE_KWARGS, new Operand[] { hash, new Hash(args), tru()}));
}

return hash;
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/jruby/ir/instructions/RuntimeHelperCall.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,8 @@ public Object callHelper(ThreadContext context, StaticScope currScope, DynamicSc
(IRubyObject) operands[3].retrieve(context, self, currScope, currDynScope, temp));
case MERGE_KWARGS:
return IRRuntimeHelpers.mergeKeywordArguments(context, (IRubyObject) arg1,
(IRubyObject) getArgs()[1].retrieve(context, self, currScope, currDynScope, temp));
(IRubyObject) getArgs()[1].retrieve(context, self, currScope, currDynScope, temp),
getArgs()[2] == context.runtime.getIRManager().getTrue());
case IS_HASH_EMPTY:
return IRRuntimeHelpers.isHashEmpty(context, (IRubyObject) arg1);
case HASH_CHECK:
Expand Down
10 changes: 8 additions & 2 deletions core/src/main/java/org/jruby/ir/runtime/IRRuntimeHelpers.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1017,7 +1017,8 @@ public static RubyModule getModuleFromScope(ThreadContext context, StaticScope s
}

@JIT @Interp
public static IRubyObject mergeKeywordArguments(ThreadContext context, IRubyObject restKwarg, IRubyObject explicitKwarg) {
public static IRubyObject mergeKeywordArguments(ThreadContext context, IRubyObject restKwarg,
IRubyObject explicitKwarg, boolean checkForDuplicates) {
// FIXME: JIT is generating a hash which is empty but seems to contain an %undefined within it.
// This was crashing because it would dup it and then try and dup the undefined within it.
// This replacement logic is correct even if that was figured out but this should just be
Expand All @@ -1041,7 +1042,12 @@ public static IRubyObject mergeKeywordArguments(ThreadContext context, IRubyObje
return hash;
}

otherHash.visitAll(context, new KwargMergeVisitor(hash), Block.NULL_BLOCK);
if (checkForDuplicates) {
otherHash.visitAll(context, new KwargMergeVisitor(hash), Block.NULL_BLOCK);
} else {
hash.merge_bang(context, new IRubyObject[] { otherHash }, Block.NULL_BLOCK);
}

context.callInfo = CALL_KEYWORD | CALL_KEYWORD_REST;

return hash;
Expand Down
3 changes: 2 additions & 1 deletion core/src/main/java/org/jruby/ir/targets/JVMVisitor.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2270,7 +2270,8 @@ public void RuntimeHelperCall(RuntimeHelperCall runtimehelpercall) {
jvmMethod().loadContext();
visit(runtimehelpercall.getArgs()[0]);
visit(runtimehelpercall.getArgs()[1]);
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "mergeKeywordArguments", sig(IRubyObject.class, ThreadContext.class, IRubyObject.class, IRubyObject.class));
jvmAdapter().ldc(runtimehelpercall.getArgs()[2] == runtime.getIRManager().getTrue());
jvmAdapter().invokestatic(p(IRRuntimeHelpers.class), "mergeKeywordArguments", sig(IRubyObject.class, ThreadContext.class, IRubyObject.class, IRubyObject.class, boolean.class));
jvmStoreLocal(runtimehelpercall.getResult());
break;
case HASH_CHECK:
Expand Down