Skip to content

Commit cd2d0a0

Browse files
eneboenebo
authored
Merge pull request #8198 from enebo/better_case_dup_warn
Give same error message for duplicated case arms
2 parents 637573f + 38b25fd commit cd2d0a0

File tree

3 files changed

+32
-23
lines changed

3 files changed

+32
-23
lines changed

‎core/src/main/java/org/jruby/ir/builder/IRBuilder.java‎

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1114,10 +1114,11 @@ protected Operand buildCase(U predicate, U[] arms, U elsey) {
11141114
Variable result = temp(); // final result value of the case statement.
11151115
Map<Label, U> bodies = new HashMap<>(); // we save bodies and emit them after processing when values.
11161116
Set<IRubyObject> seenLiterals = new HashSet<>(); // track to warn on duplicated values in when clauses.
1117+
Map<IRubyObject, java.lang.Integer> originalLocs = new HashMap<>();
11171118

11181119
for (U arm: arms) { // Emit each when value test against the case value.
11191120
Label bodyLabel = getNewLabel();
1120-
buildWhenArgs((W) arm, testValue, bodyLabel, seenLiterals);
1121+
buildWhenArgs((W) arm, testValue, bodyLabel, seenLiterals, originalLocs);
11211122
bodies.put(bodyLabel, whenBody((W) arm));
11221123
}
11231124

@@ -2731,11 +2732,15 @@ public Operand buildVAlias(RubySymbol left, RubySymbol right) {
27312732
return nil();
27322733
}
27332734

2734-
protected abstract void buildWhenArgs(W whenNode, Operand testValue, Label bodyLabel, Set<IRubyObject> seenLiterals);
2735+
// FIXME: This needs to pass node/line of first reference of literals have been shown.
2736+
protected abstract void buildWhenArgs(W whenNode, Operand testValue, Label bodyLabel,
2737+
Set<IRubyObject> seenLiterals,
2738+
Map<IRubyObject, java.lang.Integer> origLocs);
27352739

27362740
protected void buildWhenValue(Variable eqqResult, Operand testValue, Label bodyLabel, U node,
2737-
Set<IRubyObject> seenLiterals, boolean needsSplat) {
2738-
if (literalWhenCheck(node, seenLiterals)) { // we only emit first literal of the same value.
2741+
Set<IRubyObject> seenLiterals, Map<IRubyObject, java.lang.Integer> origLocs,
2742+
boolean needsSplat) {
2743+
if (literalWhenCheck(node, seenLiterals, origLocs)) { // we only emit first literal of the same value.
27392744
Operand expression;
27402745
if (isLiteralString(node)) { // compile literal string whens as fstrings
27412746
expression = frozen_string(node);
@@ -2749,9 +2754,9 @@ protected void buildWhenValue(Variable eqqResult, Operand testValue, Label bodyL
27492754
}
27502755

27512756
protected void buildWhenValues(Variable eqqResult, U[] exprValues, Operand testValue, Label bodyLabel,
2752-
Set<IRubyObject> seenLiterals) {
2757+
Set<IRubyObject> seenLiterals, Map<IRubyObject, java.lang.Integer> origLocs) {
27532758
for (U value: exprValues) {
2754-
buildWhenValue(eqqResult, testValue, bodyLabel, value, seenLiterals, false);
2759+
buildWhenValue(eqqResult, testValue, bodyLabel, value, seenLiterals, origLocs, false);
27552760
}
27562761
}
27572762

@@ -2770,16 +2775,17 @@ protected void buildWhenValues(Variable eqqResult, U[] exprValues, Operand testV
27702775
protected abstract Operand setupCallClosure(U args, U iter);
27712776

27722777
// returns true if we should emit an eqq for this value (e.g. it has not already been seen yet).
2773-
protected boolean literalWhenCheck(U value, Set<IRubyObject> seenLiterals) {
2778+
protected boolean literalWhenCheck(U value, Set<IRubyObject> seenLiterals, Map<IRubyObject, java.lang.Integer> origLocs) {
27742779
IRubyObject literal = getWhenLiteral(value);
27752780

27762781
if (literal != null) {
27772782
if (seenLiterals.contains(literal)) {
2778-
scope.getManager().getRuntime().getWarnings().warning(IRubyWarnings.ID.MISCELLANEOUS,
2779-
getFileName(), getLine(value), "duplicated when clause is ignored");
2783+
getManager().getRuntime().getWarnings().warning(IRubyWarnings.ID.MISCELLANEOUS, getFileName(), getLine(value),
2784+
"duplicated 'when' clause with line " + (origLocs.get(literal) + 1) + " is ignored");
27802785
return false;
27812786
} else {
27822787
seenLiterals.add(literal);
2788+
origLocs.put(literal, getLine(value));
27832789
return true;
27842790
}
27852791
}

‎core/src/main/java/org/jruby/ir/builder/IRBuilderAST.java‎

Lines changed: 17 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1073,34 +1073,35 @@ protected int getLine(Node node) {
10731073
}
10741074

10751075
private void buildWhenSplatValues(Variable eqqResult, Node node, Operand testValue, Label bodyLabel,
1076-
Set<IRubyObject> seenLiterals) {
1076+
Set<IRubyObject> seenLiterals, Map<IRubyObject, java.lang.Integer> origLocs) {
10771077
if (node instanceof ListNode && !(node instanceof DNode) && !(node instanceof ArrayNode)) {
1078-
buildWhenValues(eqqResult, ((ListNode) node).children(), testValue, bodyLabel, seenLiterals);
1078+
buildWhenValues(eqqResult, ((ListNode) node).children(), testValue, bodyLabel, seenLiterals, origLocs);
10791079
} else if (node instanceof SplatNode) {
1080-
buildWhenValue(eqqResult, testValue, bodyLabel, node, seenLiterals, true);
1080+
buildWhenValue(eqqResult, testValue, bodyLabel, node, seenLiterals, origLocs, true);
10811081
} else if (node instanceof ArgsCatNode) {
10821082
ArgsCatNode catNode = (ArgsCatNode) node;
1083-
buildWhenSplatValues(eqqResult, catNode.getFirstNode(), testValue, bodyLabel, seenLiterals);
1084-
buildWhenSplatValues(eqqResult, catNode.getSecondNode(), testValue, bodyLabel, seenLiterals);
1083+
buildWhenSplatValues(eqqResult, catNode.getFirstNode(), testValue, bodyLabel, seenLiterals, origLocs);
1084+
buildWhenSplatValues(eqqResult, catNode.getSecondNode(), testValue, bodyLabel, seenLiterals, origLocs);
10851085
} else if (node instanceof ArgsPushNode) {
10861086
ArgsPushNode pushNode = (ArgsPushNode) node;
1087-
buildWhenSplatValues(eqqResult, pushNode.getFirstNode(), testValue, bodyLabel, seenLiterals);
1088-
buildWhenValue(eqqResult, testValue, bodyLabel, pushNode.getSecondNode(), seenLiterals, false);
1087+
buildWhenSplatValues(eqqResult, pushNode.getFirstNode(), testValue, bodyLabel, seenLiterals, origLocs);
1088+
buildWhenValue(eqqResult, testValue, bodyLabel, pushNode.getSecondNode(), seenLiterals, origLocs, false);
10891089
} else {
1090-
buildWhenValue(eqqResult, testValue, bodyLabel, node, seenLiterals, true);
1090+
buildWhenValue(eqqResult, testValue, bodyLabel, node, seenLiterals, origLocs, true);
10911091
}
10921092
}
10931093

1094-
protected void buildWhenArgs(WhenNode whenNode, Operand testValue, Label bodyLabel, Set<IRubyObject> seenLiterals) {
1094+
protected void buildWhenArgs(WhenNode whenNode, Operand testValue, Label bodyLabel,
1095+
Set<IRubyObject> seenLiterals, Map<IRubyObject, java.lang.Integer> origLocs) {
10951096
Variable eqqResult = temp();
10961097
Node exprNodes = whenNode.getExpressionNodes();
10971098

10981099
if (exprNodes instanceof ListNode && !(exprNodes instanceof DNode) && !(exprNodes instanceof ArrayNode) && !(exprNodes instanceof ZArrayNode)) {
1099-
buildWhenValues(eqqResult, ((ListNode) exprNodes).children(), testValue, bodyLabel, seenLiterals);
1100+
buildWhenValues(eqqResult, ((ListNode) exprNodes).children(), testValue, bodyLabel, seenLiterals, origLocs);
11001101
} else if (exprNodes instanceof ArgsPushNode || exprNodes instanceof SplatNode || exprNodes instanceof ArgsCatNode) {
1101-
buildWhenSplatValues(eqqResult, exprNodes, testValue, bodyLabel, seenLiterals);
1102+
buildWhenSplatValues(eqqResult, exprNodes, testValue, bodyLabel, seenLiterals, origLocs);
11021103
} else {
1103-
buildWhenValue(eqqResult, testValue, bodyLabel, exprNodes, seenLiterals, false);
1104+
buildWhenValue(eqqResult, testValue, bodyLabel, exprNodes, seenLiterals, origLocs, false);
11041105
}
11051106
}
11061107

@@ -1164,6 +1165,7 @@ private <T extends Node & ILiteralNode> Variable buildOptimizedCaseWhen(
11641165
private <T extends Node & ILiteralNode> Map<java.lang.Integer, Label> gatherLiteralWhenBodies(
11651166
CaseNode caseNode, Map<Node, Label> nodeBodies, Function<T, Long> caseFunction) {
11661167
Map<java.lang.Integer, Label> jumpTable = new HashMap<>();
1168+
Map<java.lang.Integer, Node> origTable = new HashMap<>();
11671169

11681170
// gather literal when bodies or bail
11691171
for (Node aCase : caseNode.getCases().children()) {
@@ -1176,9 +1178,11 @@ private <T extends Node & ILiteralNode> Map<java.lang.Integer, Label> gatherLite
11761178

11771179
if (jumpTable.get((int) exprLong) == null) {
11781180
jumpTable.put((int) exprLong, bodyLabel);
1181+
origTable.put((int) exprLong, whenNode);
11791182
nodeBodies.put(whenNode, bodyLabel);
11801183
} else {
1181-
scope.getManager().getRuntime().getWarnings().warning(IRubyWarnings.ID.MISCELLANEOUS, getFileName(), expr.getLine(), "duplicated when clause is ignored");
1184+
getManager().getRuntime().getWarnings().warning(IRubyWarnings.ID.MISCELLANEOUS, getFileName(), expr.getLine() + 1,
1185+
"duplicated 'when' clause with line " + (origTable.get((int) exprLong).getLine() + 1) + " is ignored");
11821186
}
11831187
}
11841188

‎spec/tags/ruby/language/case_tags.txt‎

Lines changed: 0 additions & 1 deletion
This file was deleted.

0 commit comments

Comments
 (0)