[
https://issues.apache.org/jira/browse/GROOVY-10909?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Eric Milles closed GROOVY-10909.
--------------------------------
Resolution: Incomplete
I was able to reduce the number of branches by 2 with this kind of thing, but
the extra complexity did not seem worth it. And you would still need to
provide null for both in operands to get the branch coverage.
{code:java}
private Expression transformInOperation(final BinaryExpression bin, final
boolean in) {
Expression leftExpression = bin.getLeftExpression(), rightExpression =
bin.getRightExpression();
// transform "left [!]in right" into "right.is[Not]Case(left)"
MethodCallExpression call = callX(rightExpression, in ? "isCase" :
"isNotCase", leftExpression);
call.setImplicitThis(false); call.setSourcePosition(bin);
call.copyNodeMetaData(bin);
call.setMethodTarget(bin.getNodeMetaData(StaticTypesMarker.DIRECT_METHOD_CALL_TARGET));
// GROOVY-7473: no null test for simple cases
if (rightExpression instanceof ListExpression
|| rightExpression instanceof MapExpression
|| rightExpression instanceof RangeExpression
|| rightExpression instanceof ClassExpression
||(rightExpression instanceof ConstantExpression
&& !isNullConstant(rightExpression))
// rightExpression instanceof VariableExpression
|| isThisOrSuper(rightExpression))//GROOVY-10909
return staticCompilationTransformer.transform(call);
// GROOVY-6137, GROOVY-7473: null safety and one-time evaluation
/*
call.setObjectExpression(rightExpression =
transformRepeatedReference(rightExpression));
Expression safe = ternaryX(new
CompareToNullExpression(rightExpression,true), new
CompareToNullExpression(leftExpression,in), call);
safe.putNodeMetaData("classgen.callback",
classgenCallback(call.getObjectExpression()));
return staticCompilationTransformer.transform(safe);
*/
return new
BinaryExpression(staticCompilationTransformer.transform(leftExpression),
bin.getOperation(), staticCompilationTransformer.transform(rightExpression)) {
{
setType(ClassHelper.boolean_TYPE);
}
@Override
public Expression transformExpression(ExpressionTransformer
transformer) {
setRightExpression(transformer.transform(getRightExpression()));
setLeftExpression(transformer.transform(getLeftExpression()));
return this;
}
@Override
public void visit(GroovyCodeVisitor visitor) {
if (!(visitor instanceof AsmClassGenerator)) {
visitor.visitBinaryExpression(this);
return;
}
WriterController wc =
((AsmClassGenerator)visitor).getController();
MethodVisitor mv = wc.getMethodVisitor();
Label done = new Label();
getRightExpression().visit(visitor); // case value
ClassNode caseType = wc.getOperandStack().getTopOperand();
if (!ClassHelper.isPrimitiveType(caseType)) {
wc.getOperandStack().dup();
Label makeCall =
wc.getOperandStack().jump(Opcodes.IFNONNULL);
wc.getOperandStack().pop(); // drop null
new CompareToNullExpression(getLeftExpression(),
in).visit(visitor);
mv.visitJumpInsn(Opcodes.GOTO, done);
mv.visitLabel(makeCall);
}
wc.getOperandStack().remove(1); // case value or null check
int tmp =
wc.getCompileStack().defineTemporaryVariable("$caseValue", caseType, false);
BytecodeHelper.store(mv, caseType, tmp);
int top = wc.getOperandStack().getStackLength();
call.setObjectExpression(bytecodeX(caseType, x -> {
BytecodeHelper.load(mv, caseType, tmp); }));
call.setArguments(getLeftExpression()); // switch value
call.visit(visitor);
wc.getOperandStack().castToBool(top, false);
wc.getCompileStack().removeVar(tmp);
mv.visitLabel(done);
/*
Expression caseValue =
transformRepeatedReference(getRightExpression());
caseValue.visit(visitor);
ClassNode caseType = wc.getOperandStack().getTopOperand();
if (!ClassHelper.isPrimitiveType(caseType)) {
Label makeCall =
wc.getOperandStack().jump(Opcodes.IFNONNULL);
new CompareToNullExpression(getLeftExpression(),
in).visit(visitor);
mv.visitJumpInsn(Opcodes.GOTO, done);
mv.visitLabel(makeCall);
} else {
wc.getOperandStack().pop();
}
int mark = wc.getOperandStack().getStackLength();
call.setArguments(getLeftExpression());
call.setObjectExpression(caseValue);
call.visit(visitor);
wc.getOperandStack().castToBool(mark, false);
wc.getOperandStack().remove(1);
mv.visitLabel(done);
classgenCallback(caseValue).accept(wc);
*/
}
};
}
{code}
> bytecode for in, !in, !() and ?:
> --------------------------------
>
> Key: GROOVY-10909
> URL: https://issues.apache.org/jira/browse/GROOVY-10909
> Project: Groovy
> Issue Type: Improvement
> Components: bytecode, Compiler, Static compilation
> Reporter: Eric Milles
> Assignee: Eric Milles
> Priority: Minor
> Labels: bytecode
>
> Consider the following:
> {code:groovy}
> List getList() { ['xx'] }
> @groovy.transform.CompileStatic
> void test(object) {
> def others = [], strings = []
> if (object in list) { // 4 of 8 branches missed
> strings << object
> }
> if (!(object in list)) { // 4 of 10 branched missed
> others << object
> }
> println others
> println strings
> }
> test(null)
> test('xx')
> test([''])
> {code}
> When unit testing, I noticed 4 of 10 branches missed for "if (!(item in
> list)) ...". When compiled and executed dynamically, it shows 2 of 2
> branches covered (safe call to "list.isCase(item)").
> Static compilation transforms "item in list" into "list == null ? item ==
> null : list.isCase(item)". This is certainly where all the branches are
> coming from.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
