Adjust code to review findings

This commit is contained in:
wechman 2022-07-26 13:58:05 +02:00
parent 1084a34f28
commit a8bf1f255d
5 changed files with 52 additions and 30 deletions

View File

@ -61,6 +61,8 @@ bool ControlFlowBuilder::visit(BinaryOperation const& _operation)
switch (_operation.getOperator()) switch (_operation.getOperator())
{ {
case Token::Conditional:
return true;
case Token::Or: case Token::Or:
case Token::And: case Token::And:
{ {
@ -71,7 +73,6 @@ bool ControlFlowBuilder::visit(BinaryOperation const& _operation)
auto nodes = splitFlow<2>(); auto nodes = splitFlow<2>();
nodes[0] = createFlow(nodes[0], _operation.rightExpression()); nodes[0] = createFlow(nodes[0], _operation.rightExpression());
mergeFlow(nodes, nodes[1]); mergeFlow(nodes, nodes[1]);
return false; return false;
} }
default: default:
@ -79,15 +80,17 @@ bool ControlFlowBuilder::visit(BinaryOperation const& _operation)
if (_operation.annotation().userDefinedFunction) if (_operation.annotation().userDefinedFunction)
{ {
visitNode(_operation); visitNode(_operation);
_operation.leftExpression().accept(*this);
_operation.rightExpression().accept(*this);
solAssert(!m_currentNode->resolveFunctionCall(nullptr)); solAssert(!m_currentNode->resolveFunctionCall(nullptr));
m_currentNode->functionCall = _operation.annotation().userDefinedFunction; m_currentNode->functionCall = _operation.annotation().userDefinedFunction;
auto nextNode = newLabel(); auto nextNode = newLabel();
connect(m_currentNode, nextNode); connect(m_currentNode, nextNode);
m_currentNode = nextNode; m_currentNode = nextNode;
} }
return true; return false;
} }
} }
} }

View File

@ -3931,7 +3931,8 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor)
"." "."
); );
else if ( else if (
!TokenTraits::isBinaryOp(*operator_) && TokenTraits::isUnaryOp(*operator_) && !TokenTraits::isBinaryOp(*operator_) &&
TokenTraits::isUnaryOp(*operator_) &&
functionType->parameterTypesIncludingSelf().size() != 1 functionType->parameterTypesIncludingSelf().size() != 1
) )
m_errorReporter.typeError( m_errorReporter.typeError(

View File

@ -413,30 +413,27 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation)
if (FunctionDefinition const* function = _unaryOperation.annotation().userDefinedFunction) if (FunctionDefinition const* function = _unaryOperation.annotation().userDefinedFunction)
{ {
solAssert(function->isFree(), "Only free functions can be bound to a user type operator.");
FunctionType const* functionType = dynamic_cast<FunctionType const*>( FunctionType const* functionType = dynamic_cast<FunctionType const*>(
function->libraryFunction() ? function->typeViaContractName() : function->type()); function->libraryFunction() ? function->typeViaContractName() : function->type());
solAssert(functionType); solAssert(functionType);
functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction(); functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction();
solAssert(functionType); solAssert(functionType);
evmasm::AssemblyItem returnLabel = m_context.pushNewTag(); evmasm::AssemblyItem returnLabel = m_context.pushNewTag();
_unaryOperation.subExpression().accept(*this); acceptAndConvert(_unaryOperation.subExpression(), *functionType->selfType());
m_context << m_context.functionEntryLabel(*function).pushTag(); m_context << m_context.functionEntryLabel(*function).pushTag();
unsigned parameterSize =
CompilerUtils::sizeOnStack(functionType->parameterTypes()) +
functionType->selfType()->sizeOnStack();
if (m_context.runtimeContext())
// We have a runtime context, so we need the creation part.
utils().rightShiftNumberOnStack(32);
else
// Extract the runtime part.
m_context << ((u256(1) << 32) - 1) << Instruction::AND;
m_context.appendJump(evmasm::AssemblyItem::JumpType::IntoFunction); m_context.appendJump(evmasm::AssemblyItem::JumpType::IntoFunction);
m_context << returnLabel; m_context << returnLabel;
solAssert(
functionType->parameterTypes().size() == 0,
"Functions with parameters other than self parameter cannot be bound to a user type unary operator."
);
unsigned parameterSize = functionType->selfType()->sizeOnStack();
unsigned returnParametersSize = CompilerUtils::sizeOnStack(functionType->returnParameterTypes()); unsigned returnParametersSize = CompilerUtils::sizeOnStack(functionType->returnParameterTypes());
// callee adds return parameters, but removes arguments and return label // callee adds return parameters, but removes arguments and return label
@ -445,8 +442,6 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation)
return false; return false;
} }
Type const& type = *_unaryOperation.annotation().type; Type const& type = *_unaryOperation.annotation().type;
if (type.category() == Type::Category::RationalNumber) if (type.category() == Type::Category::RationalNumber)
{ {
@ -541,6 +536,7 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation)
Expression const& rightExpression = _binaryOperation.rightExpression(); Expression const& rightExpression = _binaryOperation.rightExpression();
if (FunctionDefinition const* function =_binaryOperation.annotation().userDefinedFunction) if (FunctionDefinition const* function =_binaryOperation.annotation().userDefinedFunction)
{ {
solAssert(function->isFree(), "Only free function can be bound to a user type operator.");
FunctionType const* functionType = dynamic_cast<FunctionType const*>( FunctionType const* functionType = dynamic_cast<FunctionType const*>(
function->libraryFunction() ? function->typeViaContractName() : function->type() function->libraryFunction() ? function->typeViaContractName() : function->type()
); );
@ -548,26 +544,22 @@ bool ExpressionCompiler::visit(BinaryOperation const& _binaryOperation)
functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction(); functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction();
solAssert(functionType); solAssert(functionType);
solAssert(
functionType->parameterTypes().size() == 1,
"Only functions with one parameter other than self parameter can be bound to a user type binary operator."
);
evmasm::AssemblyItem returnLabel = m_context.pushNewTag(); evmasm::AssemblyItem returnLabel = m_context.pushNewTag();
acceptAndConvert(leftExpression, *functionType->selfType()); acceptAndConvert(leftExpression, *functionType->selfType());
acceptAndConvert(rightExpression, *functionType->parameterTypes().at(0)); acceptAndConvert(rightExpression, *functionType->parameterTypes().at(0));
m_context << m_context.functionEntryLabel(*function).pushTag(); m_context << m_context.functionEntryLabel(*function).pushTag();
m_context.appendJump(evmasm::AssemblyItem::JumpType::IntoFunction);
m_context << returnLabel;
unsigned parameterSize = unsigned parameterSize =
CompilerUtils::sizeOnStack(functionType->parameterTypes()) + CompilerUtils::sizeOnStack(functionType->parameterTypes()) +
functionType->selfType()->sizeOnStack(); functionType->selfType()->sizeOnStack();
if (m_context.runtimeContext())
// We have a runtime context, so we need the creation part.
utils().rightShiftNumberOnStack(32);
else
// Extract the runtime part.
m_context << ((u256(1) << 32) - 1) << Instruction::AND;
m_context.appendJump(evmasm::AssemblyItem::JumpType::IntoFunction);
m_context << returnLabel;
unsigned returnParametersSize = CompilerUtils::sizeOnStack(functionType->returnParameterTypes()); unsigned returnParametersSize = CompilerUtils::sizeOnStack(functionType->returnParameterTypes());
// callee adds return parameters, but removes arguments and return label // callee adds return parameters, but removes arguments and return label
m_context.adjustStackOffset(static_cast<int>(returnParametersSize - parameterSize) - 1); m_context.adjustStackOffset(static_cast<int>(returnParametersSize - parameterSize) - 1);

View File

@ -689,11 +689,23 @@ bool IRGeneratorForStatements::visit(UnaryOperation const& _unaryOperation)
solAssert(functionType); solAssert(functionType);
functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction(); functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction();
solAssert(functionType); solAssert(functionType);
solAssert(
functionType->parameterTypes().size() == 0,
"Functions with parameters other than self parameter cannot be bound to a user type unary operator."
);
string parameter = expressionAsType(_unaryOperation.subExpression(), *functionType->selfType()); string parameter = expressionAsType(_unaryOperation.subExpression(), *functionType->selfType());
solAssert(!parameter.empty()); solAssert(!parameter.empty());
solAssert(function->isImplemented(), ""); solAssert(function->isImplemented(), "");
solAssert(
function->returnParameters().size() == 1,
"A function bound to the user type operator is supposed to return exactly one value."
);
solAssert(*_unaryOperation.annotation().type == *function->returnParameters().at(0)->type(),
"A return type of the bound function is supposed to be same as a operator type."
);
define(_unaryOperation) << define(_unaryOperation) <<
m_context.enqueueFunctionForCodeGeneration(*function) << m_context.enqueueFunctionForCodeGeneration(*function) <<
("(" + parameter + ")\n"); ("(" + parameter + ")\n");
@ -821,6 +833,10 @@ bool IRGeneratorForStatements::visit(BinaryOperation const& _binOp)
solAssert(functionType); solAssert(functionType);
functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction(); functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction();
solAssert(functionType); solAssert(functionType);
solAssert(
functionType->parameterTypes().size() == 1,
"Only functions with one parameter other than self parameter can be bound to a user type binary operator."
);
string left = expressionAsType(_binOp.leftExpression(), *functionType->selfType()); string left = expressionAsType(_binOp.leftExpression(), *functionType->selfType());
string right = expressionAsType(_binOp.rightExpression(), *functionType->parameterTypes().at(0)); string right = expressionAsType(_binOp.rightExpression(), *functionType->parameterTypes().at(0));
@ -828,6 +844,14 @@ bool IRGeneratorForStatements::visit(BinaryOperation const& _binOp)
solAssert(function->isImplemented(), ""); solAssert(function->isImplemented(), "");
solAssert(
function->returnParameters().size() == 1,
"A function bound to the user type operator is supposed to return exactly one value."
);
solAssert(*_binOp.annotation().type == *function->returnParameters().at(0)->type(),
"A return type of the bound function is supposed to be same as a operator type."
);
define(_binOp) << define(_binOp) <<
m_context.enqueueFunctionForCodeGeneration(*function) << m_context.enqueueFunctionForCodeGeneration(*function) <<
("(" + left + ", " + right + ")\n"); ("(" + left + ", " + right + ")\n");

View File

@ -8,3 +8,5 @@ contract C {
} }
// --- // ---
// ---- // ----
// TypeError 3464: (137-138): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.
// TypeError 3464: (141-142): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.