Skip to content
Permalink
Browse files
GROOVY-10617: no cast for return insertion
  • Loading branch information
eric-milles committed May 5, 2022
1 parent 0765333 commit dd904cf07260d93dca273fab3d8d2d28a8984217
Showing 7 changed files with 136 additions and 81 deletions.
@@ -32,34 +32,37 @@
import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveShort;

public class PrimitiveHelper {

private PrimitiveHelper() {
}

public static Expression getDefaultValueForPrimitive(ClassNode type) {
public static Expression getDefaultValueForPrimitive(final ClassNode type) {
if (isPrimitiveInt(type)) {
return new ConstantExpression(0);
return new ConstantExpression(0, true);
}
if (isPrimitiveLong(type)) {
return new ConstantExpression(0L);
return new ConstantExpression(0L, true);
}
if (isPrimitiveDouble(type)) {
return new ConstantExpression(0.0);
}
if (isPrimitiveFloat(type)) {
return new ConstantExpression(0.0F);
return new ConstantExpression(0.0, true);
}
if (isPrimitiveBoolean(type)) {
return ConstantExpression.FALSE;
}
if (isPrimitiveShort(type)) {
return new ConstantExpression((short) 0);
return new ConstantExpression(Boolean.FALSE, true);
}

if (isPrimitiveByte(type)) {
return new ConstantExpression((byte) 0);
return new ConstantExpression((byte) 0, true);
}
if (isPrimitiveChar(type)) {
return new ConstantExpression((char) 0);
return new ConstantExpression((char) 0, true);
}
if (isPrimitiveFloat(type)) {
return new ConstantExpression((float) 0, true);
}
if (isPrimitiveShort(type)) {
return new ConstantExpression((short) 0, true);
}

return null;
}
}
@@ -18,6 +18,7 @@
*/
package org.codehaus.groovy.classgen;

import org.codehaus.groovy.ast.ClassNode;
import org.codehaus.groovy.ast.MethodNode;
import org.codehaus.groovy.ast.VariableScope;
import org.codehaus.groovy.ast.expr.Expression;
@@ -40,7 +41,7 @@
import java.util.List;
import java.util.Objects;

import static org.codehaus.groovy.ast.tools.GeneralUtils.nullX;
import static org.codehaus.groovy.ast.tools.GeneralUtils.defaultValueX;
import static org.codehaus.groovy.runtime.DefaultGroovyMethods.last;

/**
@@ -94,20 +95,20 @@ public void visitMethod(final MethodNode node) {
if (!node.isVoidMethod()) {
Statement code = node.getCode();
if (code != null) { // happens with @interface methods
code = addReturnsIfNeeded(code, node.getVariableScope());
code = addReturnsIfNeeded(code, node.getReturnType(), node.getVariableScope());
if (doAdd) node.setCode(code);
}
}
}

private Statement addReturnsIfNeeded(final Statement statement, final VariableScope scope) {
private Statement addReturnsIfNeeded(final Statement statement, final ClassNode rtype, final VariableScope scope) {
if (statement instanceof ReturnStatement || statement instanceof ThrowStatement
|| statement instanceof EmptyStatement || statement instanceof BytecodeSequence) {
return statement;
}

if (statement == null) {
ReturnStatement returnStatement = new ReturnStatement(nullX());
ReturnStatement returnStatement = new ReturnStatement(defaultValueX(rtype));
listener.returnStatementAdded(returnStatement);
return returnStatement;
}
@@ -123,15 +124,15 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc

if (statement instanceof SynchronizedStatement) {
SynchronizedStatement syncStatement = (SynchronizedStatement) statement;
Statement code = addReturnsIfNeeded(syncStatement.getCode(), scope);
Statement code = addReturnsIfNeeded(syncStatement.getCode(), rtype, scope);
if (doAdd) syncStatement.setCode(code);
return syncStatement;
}

if (statement instanceof IfStatement) {
IfStatement ifElseStatement = (IfStatement) statement;
Statement ifBlock = addReturnsIfNeeded(ifElseStatement.getIfBlock(), scope);
Statement elseBlock = addReturnsIfNeeded(ifElseStatement.getElseBlock(), scope);
Statement ifBlock = addReturnsIfNeeded(ifElseStatement.getIfBlock(), rtype, scope);
Statement elseBlock = addReturnsIfNeeded(ifElseStatement.getElseBlock(), rtype, scope);
if (doAdd) {
ifElseStatement.setIfBlock(ifBlock);
ifElseStatement.setElseBlock(elseBlock);
@@ -145,12 +146,12 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc
List<CaseStatement> caseStatements = switchStatement.getCaseStatements();
for (Iterator<CaseStatement> it = caseStatements.iterator(); it.hasNext(); ) {
CaseStatement caseStatement = it.next();
Statement code = adjustSwitchCaseCode(caseStatement.getCode(), scope,
Statement code = adjustSwitchCaseCode(caseStatement.getCode(), rtype, scope,
// GROOVY-9896: return if no default and last case lacks break
defaultStatement == EmptyStatement.INSTANCE && !it.hasNext());
if (doAdd) caseStatement.setCode(code);
}
defaultStatement = adjustSwitchCaseCode(defaultStatement, scope, true);
defaultStatement = adjustSwitchCaseCode(defaultStatement, rtype, scope, true);
if (doAdd) switchStatement.setDefaultStatement(defaultStatement);
return switchStatement;
}
@@ -159,18 +160,18 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc
TryCatchStatement tryCatchFinally = (TryCatchStatement) statement;
boolean[] missesReturn = new boolean[1];
new ReturnAdder(returnStatement -> missesReturn[0] = true)
.addReturnsIfNeeded(tryCatchFinally.getFinallyStatement(), scope);
.addReturnsIfNeeded(tryCatchFinally.getFinallyStatement(), rtype, scope);
boolean hasFinally = !(tryCatchFinally.getFinallyStatement() instanceof EmptyStatement);

// if there is no missing return in the finally block and the block exists
// there is nothing to do
if (hasFinally && !missesReturn[0]) return tryCatchFinally;

// add returns to try and catch blocks
Statement tryStatement = addReturnsIfNeeded(tryCatchFinally.getTryStatement(), scope);
Statement tryStatement = addReturnsIfNeeded(tryCatchFinally.getTryStatement(), rtype, scope);
if (doAdd) tryCatchFinally.setTryStatement(tryStatement);
for (CatchStatement catchStatement : tryCatchFinally.getCatchStatements()) {
Statement code = addReturnsIfNeeded(catchStatement.getCode(), scope);
Statement code = addReturnsIfNeeded(catchStatement.getCode(), rtype, scope);
if (doAdd) catchStatement.setCode(code);
}
return tryCatchFinally;
@@ -179,14 +180,14 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc
if (statement instanceof BlockStatement) {
BlockStatement blockStatement = (BlockStatement) statement;
if (blockStatement.isEmpty()) {
ReturnStatement returnStatement = new ReturnStatement(nullX());
ReturnStatement returnStatement = new ReturnStatement(defaultValueX(rtype));
returnStatement.copyStatementLabels(blockStatement);
returnStatement.setSourcePosition(blockStatement);
listener.returnStatementAdded(returnStatement);
return returnStatement;
} else {
List<Statement> statements = blockStatement.getStatements(); int lastIndex = statements.size() - 1;
Statement last = addReturnsIfNeeded(statements.get(lastIndex), blockStatement.getVariableScope());
Statement last = addReturnsIfNeeded(statements.get(lastIndex), rtype, blockStatement.getVariableScope());
if (doAdd) statements.set(lastIndex, last);
return blockStatement;
}
@@ -195,7 +196,7 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc
List<Statement> statements = new ArrayList<>(2);
statements.add(statement);

ReturnStatement returnStatement = new ReturnStatement(nullX());
ReturnStatement returnStatement = new ReturnStatement(defaultValueX(rtype));
listener.returnStatementAdded(returnStatement);
statements.add(returnStatement);

@@ -204,26 +205,26 @@ private Statement addReturnsIfNeeded(final Statement statement, final VariableSc
return blockStatement;
}

private Statement adjustSwitchCaseCode(final Statement statement, final VariableScope scope, final boolean lastCase) {
private Statement adjustSwitchCaseCode(final Statement statement, final ClassNode rtype, final VariableScope scope, final boolean lastCase) {
if (!statement.isEmpty() && statement instanceof BlockStatement) {
BlockStatement block = (BlockStatement) statement;
int breakIndex = block.getStatements().size() - 1;
if (block.getStatements().get(breakIndex) instanceof BreakStatement) {
if (doAdd) {
Statement breakStatement = block.getStatements().remove(breakIndex);
if (breakIndex == 0) block.addStatement(EmptyStatement.INSTANCE);
addReturnsIfNeeded(block, scope);
addReturnsIfNeeded(block, rtype, scope);
// GROOVY-9880: some code structures will fall through
Statement lastStatement = last(block.getStatements());
if (!(lastStatement instanceof ReturnStatement
|| lastStatement instanceof ThrowStatement)) {
block.addStatement(breakStatement);
}
} else {
addReturnsIfNeeded(new BlockStatement(block.getStatements().subList(0, breakIndex), null), scope);
addReturnsIfNeeded(new BlockStatement(block.getStatements().subList(0, breakIndex), null), rtype, scope);
}
} else if (lastCase) {
return addReturnsIfNeeded(statement, scope);
return addReturnsIfNeeded(statement, rtype, scope);
}
}
return statement;
@@ -487,21 +487,20 @@ private boolean convertPrimitive(final ClassNode top, final ClassNode target) {
public void pushConstant(final ConstantExpression expression) {
MethodVisitor mv = controller.getMethodVisitor();
Object value = expression.getValue();
ClassNode origType = expression.getType().redirect();
ClassNode type = ClassHelper.getUnwrapper(origType);
boolean boxing = !origType.equals(type);
boolean asPrimitive = boxing || ClassHelper.isPrimitiveType(type);
ClassNode exprType = expression.getType();
ClassNode type = ClassHelper.getUnwrapper(exprType);
boolean boxing = !exprType.equals(type);
boolean primitive = boxing || ClassHelper.isPrimitiveType(type);

if (value == null) {
mv.visitInsn(ACONST_NULL);
} else if (boxing && value instanceof Boolean) {
// special path for boxed boolean
Boolean bool = (Boolean) value;
String text = bool ? "TRUE" : "FALSE";
type = ClassHelper.OBJECT_TYPE;
} else if (boxing && value instanceof Boolean) { // load static value
String text = ((Boolean) value).booleanValue() ? "TRUE" : "FALSE";
mv.visitFieldInsn(GETSTATIC, "java/lang/Boolean", text, "Ljava/lang/Boolean;");
boxing = false;
type = origType;
} else if (asPrimitive) {
type = exprType;
} else if (primitive) {
pushPrimitiveConstant(mv, value, type);
} else if (value instanceof BigDecimal) {
newInstance(mv, value);
@@ -53,7 +53,7 @@
import java.util.Optional;
import java.util.function.Consumer;

import static org.codehaus.groovy.ast.ClassHelper.isPrimitiveVoid;
import static org.apache.groovy.ast.tools.ExpressionUtils.isNullConstant;
import static org.codehaus.groovy.ast.tools.GeneralUtils.maybeFallsThrough;
import static org.objectweb.asm.Opcodes.ALOAD;
import static org.objectweb.asm.Opcodes.ATHROW;
@@ -572,34 +572,38 @@ public void writeThrow(final ThrowStatement statement) {
public void writeReturn(final ReturnStatement statement) {
controller.getAcg().onLineNumber(statement, "visitReturnStatement");
writeStatementLabel(statement);
ClassNode rType = controller.getReturnType();
CompileStack cs = controller.getCompileStack();
OperandStack os = controller.getOperandStack();
MethodVisitor mv = controller.getMethodVisitor();
OperandStack operandStack = controller.getOperandStack();
ClassNode returnType = controller.getReturnType();

if (isPrimitiveVoid(returnType)) {
if (ClassHelper.isPrimitiveVoid(rType)) {
if (!statement.isReturningNullOrVoid()) { // TODO: move to Verifier
controller.getAcg().throwException("Cannot use return statement with an expression on a method that returns void");
}
controller.getCompileStack().applyBlockRecorder();
cs.applyBlockRecorder();
mv.visitInsn(RETURN);
return;
}
} else {
Expression expression = statement.getExpression();
expression.visit(controller.getAcg());

Expression expression = statement.getExpression();
expression.visit(controller.getAcg());
if (!isNullConstant(expression) || ClassHelper.isPrimitiveType(rType)) {
os.doGroovyCast(rType);
} else { // GROOVY-10617
os.replace(rType);
}

operandStack.doGroovyCast(returnType);
if (cs.hasBlockRecorder()) {
ClassNode top = os.getTopOperand();
int returnVal = cs.defineTemporaryVariable("returnValue", rType, true);
cs.applyBlockRecorder();
os.load(top, returnVal);
cs.removeVar(returnVal);
}

if (controller.getCompileStack().hasBlockRecorder()) {
ClassNode type = operandStack.getTopOperand();
int returnValueIdx = controller.getCompileStack().defineTemporaryVariable("returnValue", returnType, true);
controller.getCompileStack().applyBlockRecorder();
operandStack.load(type, returnValueIdx);
controller.getCompileStack().removeVar(returnValueIdx);
BytecodeHelper.doReturn(mv, rType);
os.remove(1);
}

BytecodeHelper.doReturn(mv, returnType);
operandStack.remove(1);
}

public void writeExpressionStatement(final ExpressionStatement statement) {
@@ -16,21 +16,22 @@
* specific language governing permissions and limitations
* under the License.
*/
package bugs
package groovy.bugs

import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase

import static groovy.test.GroovyAssert.isAtLeastJdk

class Groovy10565 extends AbstractBytecodeTestCase {
final class Groovy10565 extends AbstractBytecodeTestCase {

void testPermittedSubclassName() {
if (!isAtLeastJdk('17.0')) return
def bytecode= compile('''

def bytecode = compile '''
package example
sealed class Foo permits Bar { }
class Bar extends Foo { }
''')
'''
assert bytecode.hasSequence(['PERMITTEDSUBCLASS example/Bar'])
}
}
@@ -0,0 +1,33 @@
/*
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
package groovy.bugs

import org.codehaus.groovy.classgen.asm.AbstractBytecodeTestCase

final class Groovy10617 extends AbstractBytecodeTestCase {

void testReturnInsertion() {
def bytecode = compile '''
String m() {
// return null (implicit)
}
'''
assert !bytecode.hasSequence(['INVOKEDYNAMIC cast(Ljava/lang/Object;)Ljava/lang/String;'])
}
}

0 comments on commit dd904cf

Please sign in to comment.