diff options
-rw-r--r-- | Changelog.md | 1 | ||||
-rw-r--r-- | docs/contracts.rst | 26 | ||||
-rw-r--r-- | libsolidity/analysis/TypeChecker.cpp | 80 | ||||
-rw-r--r-- | libsolidity/ast/ASTAnnotations.h | 2 | ||||
-rw-r--r-- | libsolidity/ast/Types.cpp | 12 | ||||
-rw-r--r-- | libsolidity/ast/Types.h | 4 | ||||
-rw-r--r-- | test/libsolidity/SolidityEndToEndTest.cpp | 53 | ||||
-rw-r--r-- | test/libsolidity/SolidityNameAndTypeResolution.cpp | 90 |
8 files changed, 237 insertions, 31 deletions
diff --git a/Changelog.md b/Changelog.md index 68f0037b..69c75615 100644 --- a/Changelog.md +++ b/Changelog.md @@ -20,6 +20,7 @@ Bugfixes: * Type system: Detect cyclic dependencies between constants. * Type system: Disallow arrays with negative length. * Type system: Fix a crash related to invalid binary operators. + * Type system: Warn if constant state variables are not compile-time constants. * Type system: Disallow ``var`` declaration with empty tuple type. * Type system: Correctly convert function argument types to pointers for member functions. * Type system: Move privateness of constructor into AST itself. diff --git a/docs/contracts.rst b/docs/contracts.rst index 1516ba0b..9145f016 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -428,8 +428,25 @@ change by overriding). Constant State Variables ************************ -State variables can be declared as constant (this is not yet implemented -for array and struct types and not possible for mapping types). +State variables can be declared as ``constant``. In this case, they have to be +assigned from an expression which is a constant at compile time. Any expression +that accesses storage, blockchain data (e.g. ``now``, ``this.balance`` or +``block.number``) or +execution data (``msg.gas``) or make calls to external contracts are disallowed. Expressions +that might have a side-effect on memory allocation are allowed, but those that +might have a side-effect on other memory objects are not. The built-in functions +``keccak256``, ``sha256``, ``ripemd160``, ``ecrecover``, ``addmod`` and ``mulmod`` +are allowed (ever though they do call external contracts). + +The reason behind allowing side-effects on the memory allocator is that it +should be possible to construct complex objects like e.g. lookup-tables. +This feature is not yet fully usable. + +The compiler does not reserve a storage slot for these variables and every occurrence is +replaced by the respective constant expression (which might be computed to a single value by the optimizer). + +Not all types for constants are implemented at this time. The only supported types are +value types and strings. :: @@ -438,12 +455,9 @@ for array and struct types and not possible for mapping types). contract C { uint constant x = 32**22 + 8; string constant text = "abc"; + bytes32 constant myHash = keccak256("abc"); } -This has the effect that the compiler does not reserve a storage slot -for these variables and every occurrence is replaced by their constant value. - -The value expression can only contain integer arithmetics. ****************** Constant Functions diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index fbff6865..8e7ec29b 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -467,27 +467,29 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) // TypeChecker at the VariableDeclarationStatement level. TypePointer varType = _variable.annotation().type; solAssert(!!varType, "Failed to infer variable type."); + if (_variable.value()) + expectType(*_variable.value(), *varType); if (_variable.isConstant()) { - if (!dynamic_cast<ContractDefinition const*>(_variable.scope())) + if (!_variable.isStateVariable()) typeError(_variable.location(), "Illegal use of \"constant\" specifier."); - if (!_variable.value()) - typeError(_variable.location(), "Uninitialized \"constant\" variable."); - if (!varType->isValueType()) + if (!_variable.type()->isValueType()) { - bool constImplemented = false; - if (auto arrayType = dynamic_cast<ArrayType const*>(varType.get())) - constImplemented = arrayType->isByteArray(); - if (!constImplemented) - typeError( - _variable.location(), - "Illegal use of \"constant\" specifier. \"constant\" " - "is not yet implemented for this type." - ); + bool allowed = false; + if (auto arrayType = dynamic_cast<ArrayType const*>(_variable.type().get())) + allowed = arrayType->isString(); + if (!allowed) + typeError(_variable.location(), "Constants of non-value type not yet implemented."); } + if (!_variable.value()) + typeError(_variable.location(), "Uninitialized \"constant\" variable."); + else if (!_variable.value()->annotation().isPure) + warning( + _variable.value()->location(), + "Initial value for constant variable has to be compile-time constant. " + "This will fail to compile with the next breaking version change." + ); } - if (_variable.value()) - expectType(*_variable.value(), *varType); if (!_variable.isStateVariable()) { if (varType->dataStoredIn(DataLocation::Memory) || varType->dataStoredIn(DataLocation::CallData)) @@ -928,6 +930,10 @@ bool TypeChecker::visit(Conditional const& _conditional) } _conditional.annotation().type = commonType; + _conditional.annotation().isPure = + _conditional.condition().annotation().isPure && + _conditional.trueExpression().annotation().isPure && + _conditional.falseExpression().annotation().isPure; if (_conditional.annotation().lValueRequested) typeError( @@ -1009,6 +1015,7 @@ bool TypeChecker::visit(TupleExpression const& _tuple) } else { + bool isPure = true; TypePointer inlineArrayType; for (size_t i = 0; i < components.size(); ++i) { @@ -1031,10 +1038,13 @@ bool TypeChecker::visit(TupleExpression const& _tuple) else if (inlineArrayType) inlineArrayType = Type::commonType(inlineArrayType, types[i]); } + if (!components[i]->annotation().isPure) + isPure = false; } else types.push_back(TypePointer()); } + _tuple.annotation().isPure = isPure; if (_tuple.isInlineArray()) { if (!inlineArrayType) @@ -1061,7 +1071,8 @@ bool TypeChecker::visit(UnaryOperation const& _operation) { // Inc, Dec, Add, Sub, Not, BitNot, Delete Token::Value op = _operation.getOperator(); - if (op == Token::Value::Inc || op == Token::Value::Dec || op == Token::Value::Delete) + bool const modifying = (op == Token::Value::Inc || op == Token::Value::Dec || op == Token::Value::Delete); + if (modifying) requireLValue(_operation.subExpression()); else _operation.subExpression().accept(*this); @@ -1079,6 +1090,7 @@ bool TypeChecker::visit(UnaryOperation const& _operation) t = subExprType; } _operation.annotation().type = t; + _operation.annotation().isPure = !modifying && _operation.subExpression().annotation().isPure; return false; } @@ -1105,6 +1117,10 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) Token::isCompareOp(_operation.getOperator()) ? make_shared<BoolType>() : commonType; + _operation.annotation().isPure = + _operation.leftExpression().annotation().isPure && + _operation.rightExpression().annotation().isPure; + if (_operation.getOperator() == Token::Exp) { if ( @@ -1133,6 +1149,8 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) vector<ASTPointer<Expression const>> arguments = _functionCall.arguments(); vector<ASTPointer<ASTString>> const& argumentNames = _functionCall.names(); + bool isPure = true; + // We need to check arguments' type first as they will be needed for overload resolution. shared_ptr<TypePointers> argumentTypes; if (isPositionalCall) @@ -1140,6 +1158,8 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) for (ASTPointer<Expression const> const& argument: arguments) { argument->accept(*this); + if (!argument->annotation().isPure) + isPure = false; // only store them for positional calls if (isPositionalCall) argumentTypes->push_back(type(*argument)); @@ -1177,6 +1197,7 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) typeError(_functionCall.location(), "Explicit type conversion not allowed."); } _functionCall.annotation().type = resultType; + _functionCall.annotation().isPure = isPure; return false; } @@ -1193,9 +1214,16 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) auto const& structType = dynamic_cast<StructType const&>(*t.actualType()); functionType = structType.constructorType(); membersRemovedForStructConstructor = structType.membersMissingInMemory(); + _functionCall.annotation().isPure = isPure; } else + { functionType = dynamic_pointer_cast<FunctionType const>(expressionType); + _functionCall.annotation().isPure = + isPure && + _functionCall.expression().annotation().isPure && + functionType->isPure(); + } if (!functionType) { @@ -1360,6 +1388,7 @@ void TypeChecker::endVisit(NewExpression const& _newExpression) strings(), FunctionType::Location::ObjectCreation ); + _newExpression.annotation().isPure = true; } else fatalTypeError(_newExpression.location(), "Contract or array type expected."); @@ -1445,6 +1474,12 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) annotation.isLValue = annotation.referencedDeclaration->isLValue(); } + // TODO some members might be pure, but for example `address(0x123).balance` is not pure + // although every subexpression is, so leaving this limited for now. + if (auto tt = dynamic_cast<TypeType const*>(exprType.get())) + if (tt->actualType()->category() == Type::Category::Enum) + annotation.isPure = true; + return false; } @@ -1454,6 +1489,7 @@ bool TypeChecker::visit(IndexAccess const& _access) TypePointer baseType = type(_access.baseExpression()); TypePointer resultType; bool isLValue = false; + bool isPure = _access.baseExpression().annotation().isPure; Expression const* index = _access.indexExpression(); switch (baseType->category()) { @@ -1535,6 +1571,9 @@ bool TypeChecker::visit(IndexAccess const& _access) } _access.annotation().type = move(resultType); _access.annotation().isLValue = isLValue; + if (index && !index->annotation().isPure) + isPure = false; + _access.annotation().isPure = isPure; return false; } @@ -1589,18 +1628,22 @@ bool TypeChecker::visit(Identifier const& _identifier) !!annotation.referencedDeclaration, "Referenced declaration is null after overload resolution." ); - auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(annotation.referencedDeclaration); - annotation.isConstant = variableDeclaration != nullptr && variableDeclaration->isConstant(); annotation.isLValue = annotation.referencedDeclaration->isLValue(); annotation.type = annotation.referencedDeclaration->type(); if (!annotation.type) fatalTypeError(_identifier.location(), "Declaration referenced before type could be determined."); + if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(annotation.referencedDeclaration)) + annotation.isPure = annotation.isConstant = variableDeclaration->isConstant(); + else if (dynamic_cast<MagicVariableDeclaration const*>(annotation.referencedDeclaration)) + if (auto functionType = dynamic_cast<FunctionType const*>(annotation.type.get())) + annotation.isPure = functionType->isPure(); return false; } void TypeChecker::endVisit(ElementaryTypeNameExpression const& _expr) { _expr.annotation().type = make_shared<TypeType>(Type::fromElementaryTypeName(_expr.typeName())); + _expr.annotation().isPure = true; } void TypeChecker::endVisit(Literal const& _literal) @@ -1620,6 +1663,7 @@ void TypeChecker::endVisit(Literal const& _literal) ); } _literal.annotation().type = Type::forLiteral(_literal); + _literal.annotation().isPure = true; if (!_literal.annotation().type) fatalTypeError(_literal.location(), "Invalid literal value."); } diff --git a/libsolidity/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index 9c4c3ae8..bd297f9e 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -156,6 +156,8 @@ struct ExpressionAnnotation: ASTAnnotation TypePointer type; /// Whether the expression is a constant variable bool isConstant = false; + /// Whether the expression is pure, i.e. compile-time constant. + bool isPure = false; /// Whether it is an LValue (i.e. something that can be assigned to). bool isLValue = false; /// Whether the expression is used in a context where the LValue is actually required. diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index d2793b6d..0e11c3ec 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -2456,6 +2456,18 @@ u256 FunctionType::externalIdentifier() const return FixedHash<4>::Arith(FixedHash<4>(dev::keccak256(externalSignature()))); } +bool FunctionType::isPure() const +{ + return + m_location == Location::SHA3 || + m_location == Location::ECRecover || + m_location == Location::SHA256 || + m_location == Location::RIPEMD160 || + m_location == Location::AddMod || + m_location == Location::MulMod || + m_location == Location::ObjectCreation; +} + TypePointers FunctionType::parseElementaryTypeVector(strings const& _types) { TypePointers pointers; diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 960b7e86..0a4878b8 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -973,6 +973,10 @@ public: } bool hasDeclaration() const { return !!m_declaration; } bool isConstant() const { return m_isConstant; } + /// @returns true if the the result of this function only depends on its arguments + /// and it does not modify the state. + /// Currently, this will only return true for internal functions like keccak and ecrecover. + bool isPure() const; bool isPayable() const { return m_isPayable; } /// @return A shared pointer of an ASTString. /// Can contain a nullptr in which case indicates absence of documentation diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 2a286cea..a4fab721 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -4542,7 +4542,6 @@ BOOST_AUTO_TEST_CASE(simple_constant_variables_test) BOOST_AUTO_TEST_CASE(constant_variables) { - //for now constant specifier is valid only for uint, bytesXX, string and enums char const* sourceCode = R"( contract Foo { uint constant x = 56; @@ -4553,6 +4552,58 @@ BOOST_AUTO_TEST_CASE(constant_variables) compileAndRun(sourceCode); } +BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_expression) +{ + char const* sourceCode = R"( + contract C { + uint constant x = 0x123 + 0x456; + function f() returns (uint) { return x + 1; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(0x123 + 0x456 + 1)); +} + +BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_keccak) +{ + char const* sourceCode = R"( + contract C { + bytes32 constant x = keccak256("abc"); + function f() returns (bytes32) { return x; } + } + )"; + compileAndRun(sourceCode); + BOOST_CHECK(callContractFunction("f()") == encodeArgs(dev::keccak256("abc"))); +} + +// Disabled until https://github.com/ethereum/solidity/issues/715 is implemented +//BOOST_AUTO_TEST_CASE(assignment_to_const_array_vars) +//{ +// char const* sourceCode = R"( +// contract C { +// uint[3] constant x = [uint(1), 2, 3]; +// uint constant y = x[0] + x[1] + x[2]; +// function f() returns (uint) { return y; } +// } +// )"; +// compileAndRun(sourceCode); +// BOOST_CHECK(callContractFunction("f()") == encodeArgs(1 + 2 + 3)); +//} + +// Disabled until https://github.com/ethereum/solidity/issues/715 is implemented +//BOOST_AUTO_TEST_CASE(constant_struct) +//{ +// char const* sourceCode = R"( +// contract C { +// struct S { uint x; uint[] y; } +// S constant x = S(5, new uint[](4)); +// function f() returns (uint) { return x.x; } +// } +// )"; +// compileAndRun(sourceCode); +// BOOST_CHECK(callContractFunction("f()") == encodeArgs(5)); +//} + BOOST_AUTO_TEST_CASE(packed_storage_structs_uint) { char const* sourceCode = R"( diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 865fd0c5..27791775 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2173,16 +2173,94 @@ BOOST_AUTO_TEST_CASE(assigning_value_to_const_variable) CHECK_ERROR(text, TypeError, ""); } -BOOST_AUTO_TEST_CASE(complex_const_variable) +BOOST_AUTO_TEST_CASE(assigning_state_to_const_variable) { - //for now constant specifier is valid only for uint bytesXX and enums char const* text = R"( - contract Foo { - mapping(uint => bool) x; - mapping(uint => bool) constant mapVar = x; + contract C { + address constant x = msg.sender; } )"; - CHECK_ERROR(text, TypeError, ""); + // Change to TypeError for 0.5.0. + CHECK_WARNING(text, "Initial value for constant variable has to be compile-time constant."); +} + +BOOST_AUTO_TEST_CASE(constant_string_literal_disallows_assignment) +{ + char const* text = R"( + contract Test { + string constant x = "abefghijklmnopqabcdefghijklmnopqabcdefghijklmnopqabca"; + function f() { + x[0] = "f"; + } + } + )"; + + // Even if this is made possible in the future, we should not allow assignment + // to elements of constant arrays. + CHECK_ERROR(text, TypeError, "Index access for string is not possible."); +} + +BOOST_AUTO_TEST_CASE(assign_constant_function_value_to_constant) +{ + char const* text = R"( + contract C { + function () constant returns (uint) x; + uint constant y = x(); + } + )"; + // Change to TypeError for 0.5.0. + CHECK_WARNING(text, "Initial value for constant variable has to be compile-time constant."); +} + +BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_conversion) +{ + char const* text = R"( + contract C { + C constant x = C(0x123); + } + )"; + CHECK_SUCCESS(text); +} + +BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_expression) +{ + char const* text = R"( + contract C { + uint constant x = 0x123 + 0x456; + } + )"; + CHECK_SUCCESS(text); +} + +BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_keccak) +{ + char const* text = R"( + contract C { + bytes32 constant x = keccak256("abc"); + } + )"; + CHECK_SUCCESS(text); +} + +BOOST_AUTO_TEST_CASE(assignment_to_const_array_vars) +{ + char const* text = R"( + contract C { + uint[3] constant x = [uint(1), 2, 3]; + } + )"; + CHECK_ERROR(text, TypeError, "implemented"); +} + +BOOST_AUTO_TEST_CASE(constant_struct) +{ + char const* text = R"( + contract C { + struct S { uint x; uint[] y; } + S constant x = S(5, new uint[](4)); + } + )"; + CHECK_ERROR(text, TypeError, "implemented"); } BOOST_AUTO_TEST_CASE(uninitialized_const_variable) |