From bde913f088c873cba75db30b2d463f50f9dfe862 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 1 Mar 2017 16:34:29 +0100 Subject: [PATCH 01/10] Some new tests for constant variables. --- .../SolidityNameAndTypeResolution.cpp | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 865fd0c5a..aef93e926 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2173,6 +2173,56 @@ BOOST_AUTO_TEST_CASE(assigning_value_to_const_variable) CHECK_ERROR(text, TypeError, ""); } +BOOST_AUTO_TEST_CASE(assigning_state_to_const_variable) +{ + char const* text = R"( + contract C { + address constant x = msg.sender; + } + )"; + CHECK_ERROR(text, TypeError, "Expression is not 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 + 9x456; + } + )"; + CHECK_SUCCESS(text); +} + +BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_keccak) +{ + char const* text = R"( + contract C { + bytes32 constant x = keccak("abc"); + } + )"; + CHECK_SUCCESS(text); +} + +BOOST_AUTO_TEST_CASE(assignment_to_const_array_vars) +{ + char const* text = R"( + contract C { + uint[3] memory constant x = [1, 2, 3]; + } + )"; + CHECK_SUCCESS(text); +} + BOOST_AUTO_TEST_CASE(complex_const_variable) { //for now constant specifier is valid only for uint bytesXX and enums From f39763e91c97b29bfe6d2c79cb1c1ebf80b2b9aa Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 1 Mar 2017 19:12:40 +0100 Subject: [PATCH 02/10] Type checking for pure expressions. --- libsolidity/analysis/TypeChecker.cpp | 68 ++++++++++++++----- libsolidity/ast/ASTAnnotations.h | 2 + libsolidity/ast/Types.cpp | 12 ++++ libsolidity/ast/Types.h | 4 ++ .../SolidityNameAndTypeResolution.cpp | 30 +++++--- 5 files changed, 88 insertions(+), 28 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index fbff68650..38da9b145 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -467,27 +467,20 @@ 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(_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()) - { - bool constImplemented = false; - if (auto arrayType = dynamic_cast(varType.get())) - constImplemented = arrayType->isByteArray(); - if (!constImplemented) - typeError( - _variable.location(), - "Illegal use of \"constant\" specifier. \"constant\" " - "is not yet implemented for this type." - ); - } + else if (!_variable.value()->annotation().isPure) + typeError( + _variable.value()->location(), + "Initial value for constant variable has to be compile-time constant." + ); } - if (_variable.value()) - expectType(*_variable.value(), *varType); if (!_variable.isStateVariable()) { if (varType->dataStoredIn(DataLocation::Memory) || varType->dataStoredIn(DataLocation::CallData)) @@ -928,6 +921,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 +1006,7 @@ bool TypeChecker::visit(TupleExpression const& _tuple) } else { + bool isPure = true; TypePointer inlineArrayType; for (size_t i = 0; i < components.size(); ++i) { @@ -1031,10 +1029,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 +1062,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 +1081,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 +1108,10 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) Token::isCompareOp(_operation.getOperator()) ? make_shared() : commonType; + _operation.annotation().isPure = + _operation.leftExpression().annotation().isPure && + _operation.rightExpression().annotation().isPure; + if (_operation.getOperator() == Token::Exp) { if ( @@ -1133,6 +1140,8 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) vector> arguments = _functionCall.arguments(); vector> const& argumentNames = _functionCall.names(); + bool isPure = true; + // We need to check arguments' type first as they will be needed for overload resolution. shared_ptr argumentTypes; if (isPositionalCall) @@ -1140,6 +1149,8 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) for (ASTPointer 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 +1188,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 +1205,16 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) auto const& structType = dynamic_cast(*t.actualType()); functionType = structType.constructorType(); membersRemovedForStructConstructor = structType.membersMissingInMemory(); + _functionCall.annotation().isPure = isPure; } else + { functionType = dynamic_pointer_cast(expressionType); + _functionCall.annotation().isPure = + isPure && + _functionCall.expression().annotation().isPure && + functionType->isPure(); + } if (!functionType) { @@ -1360,6 +1379,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 +1465,9 @@ 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 to false for now. + return false; } @@ -1454,6 +1477,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 +1559,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 +1616,22 @@ bool TypeChecker::visit(Identifier const& _identifier) !!annotation.referencedDeclaration, "Referenced declaration is null after overload resolution." ); - auto variableDeclaration = dynamic_cast(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(annotation.referencedDeclaration)) + annotation.isPure = annotation.isConstant = variableDeclaration->isConstant(); + else if (dynamic_cast(annotation.referencedDeclaration)) + if (auto functionType = dynamic_cast(annotation.type.get())) + annotation.isPure = functionType->isPure(); return false; } void TypeChecker::endVisit(ElementaryTypeNameExpression const& _expr) { _expr.annotation().type = make_shared(Type::fromElementaryTypeName(_expr.typeName())); + _expr.annotation().isPure = true; } void TypeChecker::endVisit(Literal const& _literal) @@ -1620,6 +1651,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 9c4c3ae8b..bd297f9eb 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 d2793b6d5..0e11c3ece 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 022b67c48..7c8fd4295 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -972,6 +972,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/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index aef93e926..90831ccdd 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2180,7 +2180,18 @@ BOOST_AUTO_TEST_CASE(assigning_state_to_const_variable) address constant x = msg.sender; } )"; - CHECK_ERROR(text, TypeError, "Expression is not compile-time constant."); + CHECK_ERROR(text, TypeError, "Initial value for constant variable has to be compile-time constant."); +} + +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(); + } + )"; + CHECK_ERROR(text, TypeError, "Initial value for constant variable has to be compile-time constant."); } BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_conversion) @@ -2197,7 +2208,7 @@ BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_expression) { char const* text = R"( contract C { - uint constant x = 0x123 + 9x456; + uint constant x = 0x123 + 0x456; } )"; CHECK_SUCCESS(text); @@ -2207,7 +2218,7 @@ BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_keccak) { char const* text = R"( contract C { - bytes32 constant x = keccak("abc"); + bytes32 constant x = keccak256("abc"); } )"; CHECK_SUCCESS(text); @@ -2217,22 +2228,21 @@ BOOST_AUTO_TEST_CASE(assignment_to_const_array_vars) { char const* text = R"( contract C { - uint[3] memory constant x = [1, 2, 3]; + uint[3] constant x = [uint(1), 2, 3]; } )"; CHECK_SUCCESS(text); } -BOOST_AUTO_TEST_CASE(complex_const_variable) +BOOST_AUTO_TEST_CASE(constant_struct) { - //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 { + struct S { uint x; uint[] y; } + S constant x = S(5, new uint[](4)); } )"; - CHECK_ERROR(text, TypeError, ""); + CHECK_SUCCESS(text); } BOOST_AUTO_TEST_CASE(uninitialized_const_variable) From 49cfacced291ea73d99ec1353a0c77bfe476729a Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 1 Mar 2017 19:49:02 +0100 Subject: [PATCH 03/10] End to end tests for constants. --- test/libsolidity/SolidityEndToEndTest.cpp | 50 +++++++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index baed3f1e0..f14108ca0 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -4553,6 +4553,56 @@ 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"))); +} + +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)); +} + +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"( From 14948e514d7b15eb92d2c08f018bb23ead71fa5a Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 1 Mar 2017 19:49:15 +0100 Subject: [PATCH 04/10] Allow enum values for constants. --- libsolidity/analysis/TypeChecker.cpp | 5 ++++- test/libsolidity/SolidityEndToEndTest.cpp | 1 - 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 38da9b145..755307396 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1466,7 +1466,10 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) } // TODO some members might be pure, but for example `address(0x123).balance` is not pure - // although every subexpression is, so leaving this to false for now. + // although every subexpression is, so leaving this limited for now. + if (auto tt = dynamic_cast(exprType.get())) + if (tt->actualType()->category() == Type::Category::Enum) + annotation.isPure = true; return false; } diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index f14108ca0..552352e4f 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; From fdc41f3b07c48438fcdd703a4dd1bd2f75736dc0 Mon Sep 17 00:00:00 2001 From: chriseth Date: Wed, 1 Mar 2017 19:53:20 +0100 Subject: [PATCH 05/10] Changelog entry. --- Changelog.md | 1 + 1 file changed, 1 insertion(+) diff --git a/Changelog.md b/Changelog.md index 89c0d883d..a80f23f4e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,6 +17,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: Only allow compile-time constants for constant state variables. * 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. From 4077e56a2fad49227e7fe4c1ed4ff81462fc6a9a Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 2 Mar 2017 14:41:51 +0100 Subject: [PATCH 06/10] Documentation. --- docs/contracts.rst | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index 1516ba0bb..52dd440a7 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -428,8 +428,10 @@ 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 a value or expression which is a constant at compile time. The compiler does +not reserve a storage slot for these variables and every occurrence is +replaced by their constant value (which might be computed by the optimizer). :: @@ -438,12 +440,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 From 592cec7e9074313e7ce5539769c065ecf5cbba12 Mon Sep 17 00:00:00 2001 From: chriseth Date: Fri, 3 Mar 2017 19:26:54 +0100 Subject: [PATCH 07/10] Disallow constants that are neither value types nor strings. --- docs/contracts.rst | 3 ++ libsolidity/analysis/TypeChecker.cpp | 8 +++ test/libsolidity/SolidityEndToEndTest.cpp | 50 ++++++++++--------- .../SolidityNameAndTypeResolution.cpp | 20 +++++++- 4 files changed, 55 insertions(+), 26 deletions(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index 52dd440a7..f15dc1228 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -433,6 +433,9 @@ assigned a value or expression which is a constant at compile time. The compiler not reserve a storage slot for these variables and every occurrence is replaced by their constant value (which might be computed by the optimizer). +Not all types for constants are implemented at this time. The only supported types are +value types and strings. + :: pragma solidity ^0.4.0; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 755307396..41636db75 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -473,6 +473,14 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) { if (!_variable.isStateVariable()) typeError(_variable.location(), "Illegal use of \"constant\" specifier."); + if (!_variable.type()->isValueType()) + { + bool allowed = false; + if (auto arrayType = dynamic_cast(_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) diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp index 552352e4f..831840e22 100644 --- a/test/libsolidity/SolidityEndToEndTest.cpp +++ b/test/libsolidity/SolidityEndToEndTest.cpp @@ -4576,31 +4576,33 @@ BOOST_AUTO_TEST_CASE(assignment_to_const_var_involving_keccak) BOOST_CHECK(callContractFunction("f()") == encodeArgs(dev::keccak256("abc"))); } -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(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)); +//} -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)); -} +// 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) { diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 90831ccdd..71fef32df 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2183,6 +2183,22 @@ BOOST_AUTO_TEST_CASE(assigning_state_to_const_variable) CHECK_ERROR(text, TypeError, "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"( @@ -2231,7 +2247,7 @@ BOOST_AUTO_TEST_CASE(assignment_to_const_array_vars) uint[3] constant x = [uint(1), 2, 3]; } )"; - CHECK_SUCCESS(text); + CHECK_ERROR(text, TypeError, "implemented"); } BOOST_AUTO_TEST_CASE(constant_struct) @@ -2242,7 +2258,7 @@ BOOST_AUTO_TEST_CASE(constant_struct) S constant x = S(5, new uint[](4)); } )"; - CHECK_SUCCESS(text); + CHECK_ERROR(text, TypeError, "implemented"); } BOOST_AUTO_TEST_CASE(uninitialized_const_variable) From bdbd3b158eb68d36a56e0220c7b02960bc71ccb1 Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 9 Mar 2017 14:39:30 +0100 Subject: [PATCH 08/10] Extend documentation for constant state variables. --- docs/contracts.rst | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index f15dc1228..890745cef 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -429,8 +429,15 @@ Constant State Variables ************************ State variables can be declared as constant. In this case, they have to be -assigned a value or expression which is a constant at compile time. The compiler does -not reserve a storage slot for these variables and every occurrence is +assigned a value or expression which is a constant at compile time. 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. This makes it possible +to create constant memory arrays as lookup-tables +(although this is not yet fully implemented). +Expressions that depend on blockchain data like `now`, `this.balance` or +`block.number` or perform any storage access are disallowed. + +The compiler does not reserve a storage slot for these variables and every occurrence is replaced by their constant value (which might be computed by the optimizer). Not all types for constants are implemented at this time. The only supported types are From c65d50681117edb96f6b1387f5a88de160811d38 Mon Sep 17 00:00:00 2001 From: chriseth Date: Mon, 13 Mar 2017 13:29:34 +0100 Subject: [PATCH 09/10] Documentation update. --- docs/contracts.rst | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index 890745cef..9145f0168 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -428,17 +428,22 @@ change by overriding). Constant State Variables ************************ -State variables can be declared as constant. In this case, they have to be -assigned a value or expression which is a constant at compile time. Expressions +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. This makes it possible -to create constant memory arrays as lookup-tables -(although this is not yet fully implemented). -Expressions that depend on blockchain data like `now`, `this.balance` or -`block.number` or perform any storage access are disallowed. +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 their constant value (which might be computed by the optimizer). +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. From 9f328ff749477106a569e679e5eeed5c7e78d29d Mon Sep 17 00:00:00 2001 From: chriseth Date: Tue, 14 Mar 2017 19:25:16 +0100 Subject: [PATCH 10/10] Turn non-constant constants error into warning. --- Changelog.md | 2 +- libsolidity/analysis/TypeChecker.cpp | 5 +++-- test/libsolidity/SolidityNameAndTypeResolution.cpp | 6 ++++-- 3 files changed, 8 insertions(+), 5 deletions(-) diff --git a/Changelog.md b/Changelog.md index a80f23f4e..ba802834e 100644 --- a/Changelog.md +++ b/Changelog.md @@ -17,7 +17,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: Only allow compile-time constants for constant state variables. + * 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/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 41636db75..8e7ec29b0 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -484,9 +484,10 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) if (!_variable.value()) typeError(_variable.location(), "Uninitialized \"constant\" variable."); else if (!_variable.value()->annotation().isPure) - typeError( + warning( _variable.value()->location(), - "Initial value for constant variable has to be compile-time constant." + "Initial value for constant variable has to be compile-time constant. " + "This will fail to compile with the next breaking version change." ); } if (!_variable.isStateVariable()) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 71fef32df..277917757 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -2180,7 +2180,8 @@ BOOST_AUTO_TEST_CASE(assigning_state_to_const_variable) address constant x = msg.sender; } )"; - CHECK_ERROR(text, TypeError, "Initial value for constant variable has to be compile-time constant."); + // 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) @@ -2207,7 +2208,8 @@ BOOST_AUTO_TEST_CASE(assign_constant_function_value_to_constant) uint constant y = x(); } )"; - CHECK_ERROR(text, TypeError, "Initial value for constant variable has to be compile-time constant."); + // 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)