Merge pull request #2754 from ethereum/isdeclaredconst

Use statemutability instead of isDeclaredConst
This commit is contained in:
Alex Beregszaszi 2017-08-16 19:51:09 +01:00 committed by GitHub
commit 4c6d476f7e
8 changed files with 42 additions and 39 deletions

View File

@ -84,8 +84,13 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
{ {
if (!function->returnParameters().empty()) if (!function->returnParameters().empty())
m_errorReporter.typeError(function->returnParameterList()->location(), "Non-empty \"returns\" directive for constructor."); m_errorReporter.typeError(function->returnParameterList()->location(), "Non-empty \"returns\" directive for constructor.");
if (function->isDeclaredConst()) if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable)
m_errorReporter.typeError(function->location(), "Constructor cannot be defined as constant."); m_errorReporter.typeError(
function->location(),
"Constructor must be payable or non-payable, but is \"" +
stateMutabilityToString(function->stateMutability()) +
"\"."
);
if (function->visibility() != FunctionDefinition::Visibility::Public && function->visibility() != FunctionDefinition::Visibility::Internal) if (function->visibility() != FunctionDefinition::Visibility::Public && function->visibility() != FunctionDefinition::Visibility::Internal)
m_errorReporter.typeError(function->location(), "Constructor must be public or internal."); m_errorReporter.typeError(function->location(), "Constructor must be public or internal.");
} }
@ -104,8 +109,13 @@ bool TypeChecker::visit(ContractDefinition const& _contract)
fallbackFunction = function; fallbackFunction = function;
if (_contract.isLibrary()) if (_contract.isLibrary())
m_errorReporter.typeError(fallbackFunction->location(), "Libraries cannot have fallback functions."); m_errorReporter.typeError(fallbackFunction->location(), "Libraries cannot have fallback functions.");
if (fallbackFunction->isDeclaredConst()) if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable)
m_errorReporter.typeError(fallbackFunction->location(), "Fallback function cannot be declared constant."); m_errorReporter.typeError(
function->location(),
"Fallback function must be payable or non-payable, but is \"" +
stateMutabilityToString(function->stateMutability()) +
"\"."
);
if (!fallbackFunction->parameters().empty()) if (!fallbackFunction->parameters().empty())
m_errorReporter.typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters."); m_errorReporter.typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters.");
if (!fallbackFunction->returnParameters().empty()) if (!fallbackFunction->returnParameters().empty())
@ -308,17 +318,16 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& function, Func
if (function.visibility() != super.visibility()) if (function.visibility() != super.visibility())
overrideError(function, super, "Overriding function visibility differs."); overrideError(function, super, "Overriding function visibility differs.");
else if (function.isDeclaredConst() && !super.isDeclaredConst()) else if (function.stateMutability() != super.stateMutability())
overrideError(function, super, "Overriding function should not be declared constant."); overrideError(
function,
else if (!function.isDeclaredConst() && super.isDeclaredConst()) super,
overrideError(function, super, "Overriding function should be declared constant."); "Overriding function changes state mutability from \"" +
stateMutabilityToString(super.stateMutability()) +
else if (function.isPayable() && !super.isPayable()) "\" to \"" +
overrideError(function, super, "Overriding function should not be declared payable."); stateMutabilityToString(function.stateMutability()) +
"\"."
else if (!function.isPayable() && super.isPayable()) );
overrideError(function, super, "Overriding function should be declared payable.");
else if (functionType != superType) else if (functionType != superType)
overrideError(function, super, "Overriding function return types differ."); overrideError(function, super, "Overriding function return types differ.");

View File

@ -608,7 +608,6 @@ public:
StateMutability stateMutability() const { return m_stateMutability; } StateMutability stateMutability() const { return m_stateMutability; }
bool isConstructor() const { return m_isConstructor; } bool isConstructor() const { return m_isConstructor; }
bool isFallback() const { return name().empty(); } bool isFallback() const { return name().empty(); }
bool isDeclaredConst() const { return m_stateMutability == StateMutability::View; }
bool isPayable() const { return m_stateMutability == StateMutability::Payable; } bool isPayable() const { return m_stateMutability == StateMutability::Payable; }
std::vector<ASTPointer<ModifierInvocation>> const& modifiers() const { return m_functionModifiers; } std::vector<ASTPointer<ModifierInvocation>> const& modifiers() const { return m_functionModifiers; }
std::vector<ASTPointer<VariableDeclaration>> const& returnParameters() const { return m_returnParameters->parameters(); } std::vector<ASTPointer<VariableDeclaration>> const& returnParameters() const { return m_returnParameters->parameters(); }
@ -913,7 +912,6 @@ public:
return m_visibility == Declaration::Visibility::Default ? Declaration::Visibility::Internal : m_visibility; return m_visibility == Declaration::Visibility::Default ? Declaration::Visibility::Internal : m_visibility;
} }
StateMutability stateMutability() const { return m_stateMutability; } StateMutability stateMutability() const { return m_stateMutability; }
bool isDeclaredConst() const { return m_stateMutability == StateMutability::View; }
bool isPayable() const { return m_stateMutability == StateMutability::Payable; } bool isPayable() const { return m_stateMutability == StateMutability::Payable; }
private: private:

View File

@ -323,8 +323,9 @@ bool ASTJsonConverter::visit(FunctionDefinition const& _node)
{ {
std::vector<pair<string, Json::Value>> attributes = { std::vector<pair<string, Json::Value>> attributes = {
make_pair("name", _node.name()), make_pair("name", _node.name()),
make_pair(m_legacy ? "constant" : "isDeclaredConst", _node.isDeclaredConst()), make_pair(m_legacy ? "constant" : "isDeclaredConst", _node.stateMutability() == StateMutability::View),
make_pair("payable", _node.isPayable()), make_pair("payable", _node.isPayable()),
make_pair("statemutability", stateMutabilityToString(_node.stateMutability())),
make_pair("visibility", Declaration::visibilityToString(_node.visibility())), make_pair("visibility", Declaration::visibilityToString(_node.visibility())),
make_pair("parameters", toJson(_node.parameterList())), make_pair("parameters", toJson(_node.parameterList())),
make_pair("isConstructor", _node.isConstructor()), make_pair("isConstructor", _node.isConstructor()),
@ -419,7 +420,8 @@ bool ASTJsonConverter::visit(FunctionTypeName const& _node)
setJsonNode(_node, "FunctionTypeName", { setJsonNode(_node, "FunctionTypeName", {
make_pair("payable", _node.isPayable()), make_pair("payable", _node.isPayable()),
make_pair("visibility", Declaration::visibilityToString(_node.visibility())), make_pair("visibility", Declaration::visibilityToString(_node.visibility())),
make_pair(m_legacy ? "constant" : "isDeclaredConst", _node.isDeclaredConst()), make_pair("statemutability", stateMutabilityToString(_node.stateMutability())),
make_pair(m_legacy ? "constant" : "isDeclaredConst", _node.stateMutability() == StateMutability::View),
make_pair("parameterTypes", toJson(*_node.parameterTypeList())), make_pair("parameterTypes", toJson(*_node.parameterTypeList())),
make_pair("returnParameterTypes", toJson(*_node.returnParameterTypeList())), make_pair("returnParameterTypes", toJson(*_node.returnParameterTypeList())),
make_pair("typeDescriptions", typePointerToJson(_node.annotation().type)) make_pair("typeDescriptions", typePointerToJson(_node.annotation().type))

View File

@ -105,7 +105,7 @@ bool ASTPrinter::visit(FunctionDefinition const& _node)
{ {
writeLine("FunctionDefinition \"" + _node.name() + "\"" + writeLine("FunctionDefinition \"" + _node.name() + "\"" +
(_node.isPublic() ? " - public" : "") + (_node.isPublic() ? " - public" : "") +
(_node.isDeclaredConst() ? " - const" : "")); (_node.stateMutability() == StateMutability::View ? " - const" : ""));
printSourcePart(_node); printSourcePart(_node);
return goDeeper(); return goDeeper();
} }

View File

@ -2215,10 +2215,7 @@ string FunctionType::identifier() const
case Kind::Require: id += "require";break; case Kind::Require: id += "require";break;
default: solAssert(false, "Unknown function location."); break; default: solAssert(false, "Unknown function location."); break;
} }
if (isConstant()) id += "_" + stateMutabilityToString(m_stateMutability);
id += "_constant";
if (isPayable())
id += "_payable";
id += identifierList(m_parameterTypes) + "returns" + identifierList(m_returnParameterTypes); id += identifierList(m_parameterTypes) + "returns" + identifierList(m_returnParameterTypes);
if (m_gasSet) if (m_gasSet)
id += "gas"; id += "gas";
@ -2237,8 +2234,7 @@ bool FunctionType::operator==(Type const& _other) const
FunctionType const& other = dynamic_cast<FunctionType const&>(_other); FunctionType const& other = dynamic_cast<FunctionType const&>(_other);
if ( if (
m_kind != other.m_kind || m_kind != other.m_kind ||
isConstant() != other.isConstant() || m_stateMutability != other.stateMutability() ||
isPayable() != other.isPayable() ||
m_parameterTypes.size() != other.m_parameterTypes.size() || m_parameterTypes.size() != other.m_parameterTypes.size() ||
m_returnParameterTypes.size() != other.m_returnParameterTypes.size() m_returnParameterTypes.size() != other.m_returnParameterTypes.size()
) )
@ -2300,10 +2296,8 @@ string FunctionType::toString(bool _short) const
for (auto it = m_parameterTypes.begin(); it != m_parameterTypes.end(); ++it) for (auto it = m_parameterTypes.begin(); it != m_parameterTypes.end(); ++it)
name += (*it)->toString(_short) + (it + 1 == m_parameterTypes.end() ? "" : ","); name += (*it)->toString(_short) + (it + 1 == m_parameterTypes.end() ? "" : ",");
name += ")"; name += ")";
if (isConstant()) if (m_stateMutability != StateMutability::NonPayable)
name += " constant"; name += " " + stateMutabilityToString(m_stateMutability);
if (isPayable())
name += " payable";
if (m_kind == Kind::External) if (m_kind == Kind::External)
name += " external"; name += " external";
if (!m_returnParameterTypes.empty()) if (!m_returnParameterTypes.empty())

View File

@ -221,7 +221,7 @@ BOOST_AUTO_TEST_CASE(function_type)
Json::Value retval = fun["children"][1]["children"][0]; Json::Value retval = fun["children"][1]["children"][0];
BOOST_CHECK_EQUAL(retval["name"], "VariableDeclaration"); BOOST_CHECK_EQUAL(retval["name"], "VariableDeclaration");
BOOST_CHECK_EQUAL(retval["attributes"]["name"], ""); BOOST_CHECK_EQUAL(retval["attributes"]["name"], "");
BOOST_CHECK_EQUAL(retval["attributes"]["type"], "function () constant external returns (uint256)"); BOOST_CHECK_EQUAL(retval["attributes"]["type"], "function () view external returns (uint256)");
funType = retval["children"][0]; funType = retval["children"][0];
BOOST_CHECK_EQUAL(funType["attributes"]["constant"], true); BOOST_CHECK_EQUAL(funType["attributes"]["constant"], true);
BOOST_CHECK_EQUAL(funType["attributes"]["payable"], false); BOOST_CHECK_EQUAL(funType["attributes"]["payable"], false);

View File

@ -933,7 +933,7 @@ BOOST_AUTO_TEST_CASE(illegal_override_remove_constness)
contract B { function f() constant {} } contract B { function f() constant {} }
contract C is B { function f() {} } contract C is B { function f() {} }
)"; )";
CHECK_ERROR(text, TypeError, "Overriding function should be declared constant."); CHECK_ERROR(text, TypeError, "Overriding function changes state mutability from \"view\" to \"nonpayable\".");
} }
BOOST_AUTO_TEST_CASE(illegal_override_add_constness) BOOST_AUTO_TEST_CASE(illegal_override_add_constness)
@ -942,7 +942,7 @@ BOOST_AUTO_TEST_CASE(illegal_override_add_constness)
contract B { function f() {} } contract B { function f() {} }
contract C is B { function f() constant {} } contract C is B { function f() constant {} }
)"; )";
CHECK_ERROR(text, TypeError, "Overriding function should not be declared constant."); CHECK_ERROR(text, TypeError, "Overriding function changes state mutability from \"nonpayable\" to \"view\".");
} }
BOOST_AUTO_TEST_CASE(complex_inheritance) BOOST_AUTO_TEST_CASE(complex_inheritance)
@ -1363,7 +1363,7 @@ BOOST_AUTO_TEST_CASE(fallback_function_with_constant_modifier)
function() constant { x = 2; } function() constant { x = 2; }
} }
)"; )";
CHECK_ERROR(text, TypeError, "Fallback function cannot be declared constant."); CHECK_ERROR(text, TypeError, "Fallback function must be payable or non-payable");
} }
BOOST_AUTO_TEST_CASE(fallback_function_twice) BOOST_AUTO_TEST_CASE(fallback_function_twice)
@ -4779,7 +4779,7 @@ BOOST_AUTO_TEST_CASE(illegal_override_payable)
contract B { function f() payable {} } contract B { function f() payable {} }
contract C is B { function f() {} } contract C is B { function f() {} }
)"; )";
CHECK_ERROR(text, TypeError, "Overriding function should be declared payable."); CHECK_ERROR(text, TypeError, "Overriding function changes state mutability from \"payable\" to \"nonpayable\".");
} }
BOOST_AUTO_TEST_CASE(illegal_override_payable_nonpayable) BOOST_AUTO_TEST_CASE(illegal_override_payable_nonpayable)
@ -4788,7 +4788,7 @@ BOOST_AUTO_TEST_CASE(illegal_override_payable_nonpayable)
contract B { function f() {} } contract B { function f() {} }
contract C is B { function f() payable {} } contract C is B { function f() payable {} }
)"; )";
CHECK_ERROR(text, TypeError, "Overriding function should not be declared payable."); CHECK_ERROR(text, TypeError, "Overriding function changes state mutability from \"nonpayable\" to \"payable\".");
} }
BOOST_AUTO_TEST_CASE(function_variable_mixin) BOOST_AUTO_TEST_CASE(function_variable_mixin)
@ -4873,7 +4873,7 @@ BOOST_AUTO_TEST_CASE(constant_constructor)
function test() constant {} function test() constant {}
} }
)"; )";
CHECK_ERROR(text, TypeError, "Constructor cannot be defined as constant."); CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable");
} }
BOOST_AUTO_TEST_CASE(external_constructor) BOOST_AUTO_TEST_CASE(external_constructor)

View File

@ -129,10 +129,10 @@ BOOST_AUTO_TEST_CASE(type_identifiers)
BOOST_CHECK_EQUAL(t.identifier(), "t_tuple$_t_type$_t_enum$_Enum_$4_$_$_t_type$_t_struct$_Struct_$3_storage_ptr_$_$_t_array$_t_string_storage_$20_storage_ptr_$__$"); BOOST_CHECK_EQUAL(t.identifier(), "t_tuple$_t_type$_t_enum$_Enum_$4_$_$_t_type$_t_struct$_Struct_$3_storage_ptr_$_$_t_array$_t_string_storage_$20_storage_ptr_$__$");
TypePointer sha3fun = make_shared<FunctionType>(strings{}, strings{}, FunctionType::Kind::SHA3); TypePointer sha3fun = make_shared<FunctionType>(strings{}, strings{}, FunctionType::Kind::SHA3);
BOOST_CHECK_EQUAL(sha3fun->identifier(), "t_function_sha3$__$returns$__$"); BOOST_CHECK_EQUAL(sha3fun->identifier(), "t_function_sha3_nonpayable$__$returns$__$");
FunctionType metaFun(TypePointers{sha3fun}, TypePointers{s.type()}); FunctionType metaFun(TypePointers{sha3fun}, TypePointers{s.type()});
BOOST_CHECK_EQUAL(metaFun.identifier(), "t_function_internal$_t_function_sha3$__$returns$__$_$returns$_t_type$_t_struct$_Struct_$3_storage_ptr_$_$"); BOOST_CHECK_EQUAL(metaFun.identifier(), "t_function_internal_nonpayable$_t_function_sha3_nonpayable$__$returns$__$_$returns$_t_type$_t_struct$_Struct_$3_storage_ptr_$_$");
TypePointer m = make_shared<MappingType>(Type::fromElementaryTypeName("bytes32"), s.type()); TypePointer m = make_shared<MappingType>(Type::fromElementaryTypeName("bytes32"), s.type());
MappingType m2(Type::fromElementaryTypeName("uint64"), m); MappingType m2(Type::fromElementaryTypeName("uint64"), m);