mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Merge pull request #4671 from ethereum/mappingTupleAssignment
Disallow assignments to mappings within tuple assignments; allow for local variables.
This commit is contained in:
commit
9d03de1f25
@ -80,6 +80,8 @@ Bugfixes:
|
||||
* Fix NatSpec json output for `@notice` and `@dev` tags on contract definitions.
|
||||
* References Resolver: Enforce ``storage`` as data location for mappings.
|
||||
* References Resolver: Report error instead of assertion fail when FunctionType has an undeclared type as parameter.
|
||||
* Type Checker: Disallow assignments to mappings within tuple assignments as well.
|
||||
* Type Checker: Allow assignments to local variables of mapping types.
|
||||
* Type Checker: Consider fixed size arrays when checking for recursive structs.
|
||||
* Type Checker: Report error when using structs in events without experimental ABIEncoderV2. This used to crash or log the wrong values.
|
||||
* Type System: Allow arbitrary exponents for literals with a mantissa of zero.
|
||||
|
@ -1330,11 +1330,41 @@ bool TypeChecker::visit(Conditional const& _conditional)
|
||||
return false;
|
||||
}
|
||||
|
||||
void TypeChecker::checkExpressionAssignment(Type const& _type, Expression const& _expression)
|
||||
{
|
||||
if (auto const* tupleExpression = dynamic_cast<TupleExpression const*>(&_expression))
|
||||
{
|
||||
auto const* tupleType = dynamic_cast<TupleType const*>(&_type);
|
||||
auto const& types = tupleType ? tupleType->components() : vector<TypePointer> { _type.shared_from_this() };
|
||||
|
||||
solAssert(tupleExpression->components().size() == types.size(), "");
|
||||
for (size_t i = 0; i < types.size(); i++)
|
||||
if (types[i])
|
||||
{
|
||||
solAssert(!!tupleExpression->components()[i], "");
|
||||
checkExpressionAssignment(*types[i], *tupleExpression->components()[i]);
|
||||
}
|
||||
}
|
||||
else if (_type.category() == Type::Category::Mapping)
|
||||
{
|
||||
bool isLocalOrReturn = false;
|
||||
if (auto const* identifier = dynamic_cast<Identifier const*>(&_expression))
|
||||
if (auto const *variableDeclaration = dynamic_cast<VariableDeclaration const*>(identifier->annotation().referencedDeclaration))
|
||||
if (variableDeclaration->isLocalOrReturn())
|
||||
isLocalOrReturn = true;
|
||||
if (!isLocalOrReturn)
|
||||
m_errorReporter.typeError(_expression.location(), "Mappings cannot be assigned to.");
|
||||
}
|
||||
}
|
||||
|
||||
bool TypeChecker::visit(Assignment const& _assignment)
|
||||
{
|
||||
requireLValue(_assignment.leftHandSide());
|
||||
TypePointer t = type(_assignment.leftHandSide());
|
||||
_assignment.annotation().type = t;
|
||||
|
||||
checkExpressionAssignment(*t, _assignment.leftHandSide());
|
||||
|
||||
if (TupleType const* tupleType = dynamic_cast<TupleType const*>(t.get()))
|
||||
{
|
||||
if (_assignment.assignmentOperator() != Token::Assign)
|
||||
@ -1351,11 +1381,6 @@ bool TypeChecker::visit(Assignment const& _assignment)
|
||||
if (dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get()))
|
||||
checkDoubleStorageAssignment(_assignment);
|
||||
}
|
||||
else if (t->category() == Type::Category::Mapping)
|
||||
{
|
||||
m_errorReporter.typeError(_assignment.location(), "Mappings cannot be assigned to.");
|
||||
_assignment.rightHandSide().accept(*this);
|
||||
}
|
||||
else if (_assignment.assignmentOperator() == Token::Assign)
|
||||
expectType(_assignment.rightHandSide(), *t);
|
||||
else
|
||||
|
@ -87,6 +87,9 @@ private:
|
||||
/// Checks (and warns) if a tuple assignment might cause unexpected overwrites in storage.
|
||||
/// Should only be called if the left hand side is tuple-typed.
|
||||
void checkDoubleStorageAssignment(Assignment const& _assignment);
|
||||
// Checks whether the expression @arg _expression can be assigned from type @arg _type
|
||||
// and reports an error, if not.
|
||||
void checkExpressionAssignment(Type const& _type, Expression const& _expression);
|
||||
|
||||
virtual void endVisit(InheritanceSpecifier const& _inheritance) override;
|
||||
virtual void endVisit(UsingForDirective const& _usingFor) override;
|
||||
|
@ -1478,6 +1478,73 @@ BOOST_AUTO_TEST_CASE(multi_level_mapping)
|
||||
testContractAgainstCpp("f(uint256,uint256,uint256)", f, u256(5), u256(4), u256(0));
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(mapping_local_assignment)
|
||||
{
|
||||
char const* sourceCode = R"(
|
||||
contract test {
|
||||
mapping(uint8 => uint8) m1;
|
||||
mapping(uint8 => uint8) m2;
|
||||
function f() public returns (uint8, uint8, uint8, uint8) {
|
||||
mapping(uint8 => uint8) storage m = m1;
|
||||
m[1] = 42;
|
||||
|
||||
m = m2;
|
||||
m[2] = 21;
|
||||
|
||||
return (m1[1], m1[2], m2[1], m2[2]);
|
||||
}
|
||||
}
|
||||
)";
|
||||
compileAndRun(sourceCode);
|
||||
|
||||
ABI_CHECK(callContractFunction("f()"), encodeArgs(byte(42), byte(0), byte(0), byte(21)));
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(mapping_local_tuple_assignment)
|
||||
{
|
||||
char const* sourceCode = R"(
|
||||
contract test {
|
||||
mapping(uint8 => uint8) m1;
|
||||
mapping(uint8 => uint8) m2;
|
||||
function f() public returns (uint8, uint8, uint8, uint8) {
|
||||
mapping(uint8 => uint8) storage m = m1;
|
||||
m[1] = 42;
|
||||
|
||||
uint8 v;
|
||||
(m, v) = (m2, 21);
|
||||
m[2] = v;
|
||||
|
||||
return (m1[1], m1[2], m2[1], m2[2]);
|
||||
}
|
||||
}
|
||||
)";
|
||||
compileAndRun(sourceCode);
|
||||
|
||||
ABI_CHECK(callContractFunction("f()"), encodeArgs(byte(42), byte(0), byte(0), byte(21)));
|
||||
}
|
||||
|
||||
BOOST_AUTO_TEST_CASE(mapping_local_compound_assignment)
|
||||
{
|
||||
char const* sourceCode = R"(
|
||||
contract test {
|
||||
mapping(uint8 => uint8) m1;
|
||||
mapping(uint8 => uint8) m2;
|
||||
function f() public returns (uint8, uint8, uint8, uint8) {
|
||||
mapping(uint8 => uint8) storage m = m1;
|
||||
m[1] = 42;
|
||||
|
||||
(m = m2)[2] = 21;
|
||||
|
||||
return (m1[1], m1[2], m2[1], m2[2]);
|
||||
}
|
||||
}
|
||||
)";
|
||||
compileAndRun(sourceCode);
|
||||
|
||||
ABI_CHECK(callContractFunction("f()"), encodeArgs(byte(42), byte(0), byte(0), byte(21)));
|
||||
}
|
||||
|
||||
|
||||
BOOST_AUTO_TEST_CASE(structs)
|
||||
{
|
||||
char const* sourceCode = R"(
|
||||
|
@ -1,12 +0,0 @@
|
||||
contract test {
|
||||
struct str {
|
||||
mapping(uint=>uint) map;
|
||||
}
|
||||
str data;
|
||||
function fun() public {
|
||||
mapping(uint=>uint) storage a = data.map;
|
||||
data.map = a;
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (172-184): Mappings cannot be assigned to.
|
@ -0,0 +1,11 @@
|
||||
contract test {
|
||||
mapping(uint=>uint) map;
|
||||
function fun() public view {
|
||||
mapping(uint=>uint) storage a = map;
|
||||
mapping(uint=>uint) storage b = map;
|
||||
b = a;
|
||||
(b) = a;
|
||||
(b, b) = (a, a);
|
||||
}
|
||||
}
|
||||
// ----
|
@ -0,0 +1,14 @@
|
||||
contract test {
|
||||
mapping(uint=>uint) map;
|
||||
function fun() public {
|
||||
mapping(uint=>uint) storage a = map;
|
||||
map = a;
|
||||
(map) = a;
|
||||
(map, map) = (a, a);
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (126-129): Mappings cannot be assigned to.
|
||||
// TypeError: (144-147): Mappings cannot be assigned to.
|
||||
// TypeError: (163-166): Mappings cannot be assigned to.
|
||||
// TypeError: (168-171): Mappings cannot be assigned to.
|
@ -0,0 +1,17 @@
|
||||
contract test {
|
||||
struct str {
|
||||
mapping(uint=>uint) map;
|
||||
}
|
||||
str data;
|
||||
function fun() public {
|
||||
mapping(uint=>uint) storage a = data.map;
|
||||
data.map = a;
|
||||
(data.map) = a;
|
||||
(data.map, data.map) = (a, a);
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (172-180): Mappings cannot be assigned to.
|
||||
// TypeError: (195-203): Mappings cannot be assigned to.
|
||||
// TypeError: (219-227): Mappings cannot be assigned to.
|
||||
// TypeError: (229-237): Mappings cannot be assigned to.
|
@ -0,0 +1,7 @@
|
||||
contract C {
|
||||
function f() external pure returns (mapping(uint=>uint) storage m) {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (53-82): Type is required to live outside storage.
|
||||
// TypeError: (53-82): Internal or recursive type is not allowed for public or external functions.
|
@ -0,0 +1,21 @@
|
||||
// This should be allowed in a future release.
|
||||
contract C {
|
||||
mapping(uint=>uint) m;
|
||||
function f() internal view returns (mapping(uint=>uint) storage) {
|
||||
return m;
|
||||
}
|
||||
function g() private view returns (mapping(uint=>uint) storage) {
|
||||
return m;
|
||||
}
|
||||
function h() internal view returns (mapping(uint=>uint) storage r) {
|
||||
r = m;
|
||||
}
|
||||
function i() private view returns (mapping(uint=>uint) storage r) {
|
||||
(r,r) = (m,m);
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (127-146): Type is required to live outside storage.
|
||||
// TypeError: (221-240): Type is required to live outside storage.
|
||||
// TypeError: (316-345): Type is required to live outside storage.
|
||||
// TypeError: (409-438): Type is required to live outside storage.
|
@ -0,0 +1,7 @@
|
||||
contract C {
|
||||
function f() public pure returns (mapping(uint=>uint) storage m) {
|
||||
}
|
||||
}
|
||||
// ----
|
||||
// TypeError: (51-80): Type is required to live outside storage.
|
||||
// TypeError: (51-80): Internal or recursive type is not allowed for public or external functions.
|
Loading…
Reference in New Issue
Block a user