mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Merge pull request #1729 from ethereum/constantvariables
Only allow pure expressions for constant state variables.
This commit is contained in:
commit
d134fda0c0
@ -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.
|
||||
|
@ -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
|
||||
|
@ -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.isConstant())
|
||||
{
|
||||
if (!dynamic_cast<ContractDefinition const*>(_variable.scope()))
|
||||
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<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."
|
||||
);
|
||||
}
|
||||
}
|
||||
if (_variable.value())
|
||||
expectType(*_variable.value(), *varType);
|
||||
if (_variable.isConstant())
|
||||
{
|
||||
if (!_variable.isStateVariable())
|
||||
typeError(_variable.location(), "Illegal use of \"constant\" specifier.");
|
||||
if (!_variable.type()->isValueType())
|
||||
{
|
||||
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.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.");
|
||||
}
|
||||
|
@ -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.
|
||||
|
@ -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;
|
||||
|
@ -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
|
||||
|
@ -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"(
|
||||
|
@ -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)
|
||||
|
Loading…
Reference in New Issue
Block a user