Merge pull request #4431 from ethereum/tupleDeclaration

Disallow multi variable declarations with mismatching number of values.
This commit is contained in:
chriseth 2018-07-13 01:23:28 +02:00 committed by GitHub
commit 052f19c6b0
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
30 changed files with 205 additions and 203 deletions

View File

@ -38,6 +38,7 @@ Breaking Changes:
* Type Checker: Disallow tight packing of literals. This was already the case in the experimental 0.5.0 mode.
* Type Checker: Disallow conversions between ``bytesX`` and ``uintY`` of different size.
* Type Checker: Disallow empty tuple components. This was partly already the case in the experimental 0.5.0 mode.
* Type Checker: Disallow multi-variable declarations with mismatching number of values. This was already the case in the experimental 0.5.0 mode.
* Type Checker: Disallow specifying base constructor arguments multiple times in the same inheritance hierarchy. This was already the case in the experimental 0.5.0 mode.
* Type Checker: Disallow calling constructor with wrong argument count. This was already the case in the experimental 0.5.0 mode.
* Type Checker: Disallow uninitialized storage variables. This was already the case in the experimental 0.5.0 mode.

View File

@ -297,9 +297,9 @@ These can then either be assigned to newly declared variables or to pre-existing
}
.. note::
Prior to version 0.4.24 it was possible to assign to tuples of smaller size, either
Prior to version 0.5.0 it was possible to assign to tuples of smaller size, either
filling up on the left or on the right side (which ever was empty). This is
now deprecated, both sides have to have the same number of components.
now disallowed, so both sides have to have the same number of components.
Complications for Arrays and Structs
------------------------------------

View File

@ -1043,10 +1043,10 @@ namespace
* @returns a suggested left-hand-side of a multi-variable declaration contairing
* the variable declarations given in @a _decls.
*/
string createTupleDecl(vector<VariableDeclaration const*> const& _decls)
string createTupleDecl(vector<ASTPointer<VariableDeclaration>> const& _decls)
{
vector<string> components;
for (VariableDeclaration const* decl: _decls)
for (ASTPointer<VariableDeclaration> const& decl: _decls)
if (decl)
components.emplace_back(decl->annotation().type->toString(false) + " " + decl->name());
else
@ -1058,9 +1058,9 @@ string createTupleDecl(vector<VariableDeclaration const*> const& _decls)
return "(" + boost::algorithm::join(components, ", ") + ")";
}
bool typeCanBeExpressed(vector<VariableDeclaration const*> const& decls)
bool typeCanBeExpressed(vector<ASTPointer<VariableDeclaration>> const& decls)
{
for (VariableDeclaration const* decl: decls)
for (ASTPointer<VariableDeclaration> const& decl: decls)
{
// skip empty tuples (they can be expressed of course)
if (!decl)
@ -1080,7 +1080,6 @@ bool typeCanBeExpressed(vector<VariableDeclaration const*> const& decls)
bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
{
bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
if (!_statement.initialValue())
{
// No initial value is only permitted for single variables with specified type.
@ -1119,82 +1118,27 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
else
valueTypes = TypePointers{type(*_statement.initialValue())};
// Determine which component is assigned to which variable.
// If numbers do not match, fill up if variables begin or end empty (not both).
vector<VariableDeclaration const*>& assignments = _statement.annotation().assignments;
assignments.resize(valueTypes.size(), nullptr);
vector<ASTPointer<VariableDeclaration>> const& variables = _statement.declarations();
if (variables.empty())
{
if (!valueTypes.empty())
m_errorReporter.fatalTypeError(
_statement.location(),
"Too many components (" +
toString(valueTypes.size()) +
") in value for variable assignment (0) needed"
);
}
// We already have an error for this in the SyntaxChecker.
solAssert(m_errorReporter.hasErrors(), "");
else if (valueTypes.size() != variables.size())
{
if (v050)
m_errorReporter.fatalTypeError(
_statement.location(),
"Different number of components on the left hand side (" +
toString(variables.size()) +
") than on the right hand side (" +
toString(valueTypes.size()) +
")."
);
else if (!variables.front() && !variables.back())
m_errorReporter.fatalTypeError(
_statement.location(),
"Wildcard both at beginning and end of variable declaration list is only allowed "
"if the number of components is equal."
);
else
m_errorReporter.warning(
_statement.location(),
"Different number of components on the left hand side (" +
toString(variables.size()) +
") than on the right hand side (" +
toString(valueTypes.size()) +
")."
);
}
size_t minNumValues = variables.size();
if (!variables.empty() && (!variables.back() || !variables.front()))
--minNumValues;
if (valueTypes.size() < minNumValues)
m_errorReporter.fatalTypeError(
m_errorReporter.typeError(
_statement.location(),
"Not enough components (" +
"Different number of components on the left hand side (" +
toString(variables.size()) +
") than on the right hand side (" +
toString(valueTypes.size()) +
") in value to assign all variables (" +
toString(minNumValues) + ")."
")."
);
if (valueTypes.size() > variables.size() && variables.front() && variables.back())
m_errorReporter.fatalTypeError(
_statement.location(),
"Too many components (" +
toString(valueTypes.size()) +
") in value for variable assignment (" +
toString(minNumValues) +
" needed)."
);
bool fillRight = !variables.empty() && (!variables.back() || variables.front());
for (size_t i = 0; i < min(variables.size(), valueTypes.size()); ++i)
if (fillRight)
assignments[i] = variables[i].get();
else
assignments[assignments.size() - i - 1] = variables[variables.size() - i - 1].get();
bool autoTypeDeductionNeeded = false;
for (size_t i = 0; i < assignments.size(); ++i)
for (size_t i = 0; i < min(variables.size(), valueTypes.size()); ++i)
{
if (!assignments[i])
if (!variables[i])
continue;
VariableDeclaration const& var = *assignments[i];
VariableDeclaration const& var = *variables[i];
solAssert(!var.value(), "Value has to be tied to statement.");
TypePointer const& valueComponentType = valueTypes[i];
solAssert(!!valueComponentType, "");
@ -1284,7 +1228,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
if (autoTypeDeductionNeeded)
{
if (!typeCanBeExpressed(assignments))
if (!typeCanBeExpressed(variables))
m_errorReporter.syntaxError(
_statement.location(),
"Use of the \"var\" keyword is disallowed. "
@ -1294,7 +1238,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
m_errorReporter.syntaxError(
_statement.location(),
"Use of the \"var\" keyword is disallowed. "
"Use explicit declaration `" + createTupleDecl(assignments) + " = ...´ instead."
"Use explicit declaration `" + createTupleDecl(variables) + " = ...´ instead."
);
}

View File

@ -535,13 +535,6 @@ ReturnAnnotation& Return::annotation() const
return dynamic_cast<ReturnAnnotation&>(*m_annotation);
}
VariableDeclarationStatementAnnotation& VariableDeclarationStatement::annotation() const
{
if (!m_annotation)
m_annotation = new VariableDeclarationStatementAnnotation();
return dynamic_cast<VariableDeclarationStatementAnnotation&>(*m_annotation);
}
ExpressionAnnotation& Expression::annotation() const
{
if (!m_annotation)

View File

@ -1270,8 +1270,6 @@ public:
virtual void accept(ASTVisitor& _visitor) override;
virtual void accept(ASTConstVisitor& _visitor) const override;
VariableDeclarationStatementAnnotation& annotation() const override;
std::vector<ASTPointer<VariableDeclaration>> const& declarations() const { return m_variables; }
Expression const* initialValue() const { return m_initialValue.get(); }

View File

@ -164,13 +164,6 @@ struct UserDefinedTypeNameAnnotation: TypeNameAnnotation
ContractDefinition const* contractScope = nullptr;
};
struct VariableDeclarationStatementAnnotation: StatementAnnotation
{
/// Information about which component of the value is assigned to which variable.
/// The pointer can be null to signify that the component is discarded.
std::vector<VariableDeclaration const*> assignments;
};
struct ExpressionAnnotation: ASTAnnotation
{
/// Inferred type of the expression.

View File

@ -555,8 +555,8 @@ bool ASTJsonConverter::visit(EmitStatement const& _node)
bool ASTJsonConverter::visit(VariableDeclarationStatement const& _node)
{
Json::Value varDecs(Json::arrayValue);
for (auto const& v: _node.annotation().assignments)
appendMove(varDecs, idOrNull(v));
for (auto const& v: _node.declarations())
appendMove(varDecs, idOrNull(v.get()));
setJsonNode(_node, "VariableDeclarationStatement", {
make_pair("assignments", std::move(varDecs)),
make_pair("declarations", toJson(_node.declarations())),

View File

@ -833,20 +833,19 @@ bool ContractCompiler::visit(VariableDeclarationStatement const& _variableDeclar
valueTypes = tupleType->components();
else
valueTypes = TypePointers{expression->annotation().type};
auto const& assignments = _variableDeclarationStatement.annotation().assignments;
solAssert(assignments.size() == valueTypes.size(), "");
for (size_t i = 0; i < assignments.size(); ++i)
auto const& declarations = _variableDeclarationStatement.declarations();
solAssert(declarations.size() == valueTypes.size(), "");
for (size_t i = 0; i < declarations.size(); ++i)
{
size_t j = assignments.size() - i - 1;
size_t j = declarations.size() - i - 1;
solAssert(!!valueTypes[j], "");
VariableDeclaration const* varDecl = assignments[j];
if (!varDecl)
utils.popStackElement(*valueTypes[j]);
else
if (VariableDeclaration const* varDecl = declarations[j].get())
{
utils.convertType(*valueTypes[j], *varDecl->annotation().type);
utils.moveToStackVariable(*varDecl);
}
else
utils.popStackElement(*valueTypes[j]);
}
}
checker.check();

View File

@ -8091,17 +8091,36 @@ BOOST_AUTO_TEST_CASE(multi_variable_declaration)
function g() public returns (uint a, uint b, uint c) {
a = 1; b = 2; c = 3;
}
function f() public returns (bool) {
function h() public returns (uint a, uint b, uint c, uint d) {
a = 1; b = 2; c = 3; d = 4;
}
function f1() public returns (bool) {
(uint x, uint y, uint z) = g();
if (x != 1 || y != 2 || z != 3) return false;
(, uint a,) = g();
if (a != 2) return false;
(uint b,) = g();
(uint b, , ) = g();
if (b != 1) return false;
(, uint c) = g();
(, , uint c) = g();
if (c != 3) return false;
return true;
}
function f2() public returns (bool) {
(uint a1, , uint a3, ) = h();
if (a1 != 1 || a3 != 3) return false;
(uint b1, uint b2, , ) = h();
if (b1 != 1 || b2 != 2) return false;
(, uint c2, uint c3, ) = h();
if (c2 != 2 || c3 != 3) return false;
(, , uint d3, uint d4) = h();
if (d3 != 3 || d4 != 4) return false;
(uint e1, , uint e3, uint e4) = h();
if (e1 != 1 || e3 != 3 || e4 != 4) return false;
return true;
}
function f() public returns (bool) {
return f1() && f2();
}
}
)";
compileAndRun(sourceCode);

View File

@ -0,0 +1,25 @@
contract C {
function f() public {
uint a = (1,2);
uint b = (1,2,3);
uint c = (1,2,3,4);
}
function g() public {
(uint a1, uint b1, uint c1, uint d1) = 1;
(uint a2, uint b2, uint c2) = 1;
(uint a3, uint b3) = 1;
}
function h() public {
(uint a1, uint b1, uint c1, uint d1) = (1,2,3);
(uint a2, uint b2, uint c2) = (1,2,3,4);
}
}
// ----
// TypeError: (47-61): Different number of components on the left hand side (1) than on the right hand side (2).
// TypeError: (71-87): Different number of components on the left hand side (1) than on the right hand side (3).
// TypeError: (97-115): Different number of components on the left hand side (1) than on the right hand side (4).
// TypeError: (157-197): Different number of components on the left hand side (4) than on the right hand side (1).
// TypeError: (207-238): Different number of components on the left hand side (3) than on the right hand side (1).
// TypeError: (248-270): Different number of components on the left hand side (2) than on the right hand side (1).
// TypeError: (312-358): Different number of components on the left hand side (4) than on the right hand side (3).
// TypeError: (368-407): Different number of components on the left hand side (3) than on the right hand side (4).

View File

@ -0,0 +1,29 @@
contract C {
function f() public {
uint a = two();
uint b = three();
uint c = four();
}
function g() public {
(uint a1, uint b1, uint c1, uint d1) = one();
(uint a2, uint b2, uint c2) = one();
(uint a3, uint b3) = one();
}
function h() public {
(uint a1, uint b1, uint c1, uint d1) = three();
(uint a2, uint b2, uint c2) = four();
}
function one() public pure returns (uint);
function two() public pure returns (uint, uint);
function three() public pure returns (uint, uint, uint);
function four() public pure returns (uint, uint, uint, uint);
}
// ----
// TypeError: (47-61): Different number of components on the left hand side (1) than on the right hand side (2).
// TypeError: (71-87): Different number of components on the left hand side (1) than on the right hand side (3).
// TypeError: (97-112): Different number of components on the left hand side (1) than on the right hand side (4).
// TypeError: (154-198): Different number of components on the left hand side (4) than on the right hand side (1).
// TypeError: (208-243): Different number of components on the left hand side (3) than on the right hand side (1).
// TypeError: (253-279): Different number of components on the left hand side (2) than on the right hand side (1).
// TypeError: (321-367): Different number of components on the left hand side (4) than on the right hand side (3).
// TypeError: (377-413): Different number of components on the left hand side (3) than on the right hand side (4).

View File

@ -0,0 +1,24 @@
contract C {
function fn() public pure {
(uint a,) = (1,2,3);
(,uint b) = (1,2,3);
(,uint c,) = (1,2,3,4,5);
(uint d, uint e,) = (1,2,3,4);
(,uint f, uint g) = (1,2,3,4);
(,uint h, uint i,) = (1,2,3);
(uint j,) = 1;
(,uint k) = 1;
(,uint l,) = 1;
a;b;c;d;e;f;g;h;i;j;k;l;
}
}
// ----
// TypeError: (53-72): Different number of components on the left hand side (2) than on the right hand side (3).
// TypeError: (82-101): Different number of components on the left hand side (2) than on the right hand side (3).
// TypeError: (111-135): Different number of components on the left hand side (3) than on the right hand side (5).
// TypeError: (145-174): Different number of components on the left hand side (3) than on the right hand side (4).
// TypeError: (184-213): Different number of components on the left hand side (3) than on the right hand side (4).
// TypeError: (223-251): Different number of components on the left hand side (4) than on the right hand side (3).
// TypeError: (261-274): Different number of components on the left hand side (2) than on the right hand side (1).
// TypeError: (284-297): Different number of components on the left hand side (2) than on the right hand side (1).
// TypeError: (307-321): Different number of components on the left hand side (3) than on the right hand side (1).

View File

@ -0,0 +1,31 @@
contract C {
function fn() public pure {
(uint a,) = three();
(,uint b) = three();
(,uint c,) = five();
(uint d, uint e,) = four();
(,uint f, uint g) = four();
(,uint h, uint i,) = three();
(uint j,) = one();
(,uint k) = one();
(,uint l,) = one();
(,uint m, uint n,) = five();
a;b;c;d;e;f;g;h;i;j;k;l;m;n;
}
function one() public pure returns (uint);
function two() public pure returns (uint, uint);
function three() public pure returns (uint, uint, uint);
function four() public pure returns (uint, uint, uint, uint);
function five() public pure returns (uint, uint, uint, uint, uint);
}
// ----
// TypeError: (53-72): Different number of components on the left hand side (2) than on the right hand side (3).
// TypeError: (82-101): Different number of components on the left hand side (2) than on the right hand side (3).
// TypeError: (111-130): Different number of components on the left hand side (3) than on the right hand side (5).
// TypeError: (140-166): Different number of components on the left hand side (3) than on the right hand side (4).
// TypeError: (176-202): Different number of components on the left hand side (3) than on the right hand side (4).
// TypeError: (212-240): Different number of components on the left hand side (4) than on the right hand side (3).
// TypeError: (250-267): Different number of components on the left hand side (2) than on the right hand side (1).
// TypeError: (277-294): Different number of components on the left hand side (2) than on the right hand side (1).
// TypeError: (304-322): Different number of components on the left hand side (3) than on the right hand side (1).
// TypeError: (332-359): Different number of components on the left hand side (4) than on the right hand side (5).

View File

@ -2,10 +2,10 @@ contract D {
struct S { uint a; uint b; }
}
contract C {
function f() internal returns (uint, uint, uint, D.S[20] storage, uint) {
(,,,D.S[10*2] storage x,) = f();
function f() internal pure {
(,,,D.S[10*2] storage x,) = g();
x;
}
}
function g() internal pure returns (uint, uint, uint, D.S[20] storage x, uint) { x = x; }
}
// ----
// Warning: (110-117): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,11 @@
contract C {
function f() public pure {
(uint a, uint b) = f();
(uint c) = f();
uint d = f();
}
}
// ----
// TypeError: (52-74): Different number of components on the left hand side (2) than on the right hand side (0).
// TypeError: (84-98): Different number of components on the left hand side (1) than on the right hand side (0).
// TypeError: (108-120): Different number of components on the left hand side (1) than on the right hand side (0).

View File

@ -1,12 +1,12 @@
contract C {
function f() internal returns (uint, uint, uint, uint) {
function f() internal pure returns (uint, uint, uint, uint) {
(uint a, uint b,,) = f();
a; b;
}
function g() internal returns (bytes memory, string storage) {
(bytes memory a, string storage b) = g();
function g() internal pure {
(bytes memory a, string storage b) = h();
a; b;
}
}
function h() internal pure returns (bytes memory, string storage s) { s = s; }
}
// ----
// Warning: (163-169): This variable is of storage pointer type and might be returned without assignment. This can cause storage corruption. Assign the variable (potentially from itself) to remove this warning.

View File

@ -0,0 +1,8 @@
contract C {
function f() public {
(uint a,) = (1,);
a;
}
}
// ----
// TypeError: (59-63): Tuple component cannot be empty.

View File

@ -0,0 +1,9 @@
contract C {
function f() public pure {
(uint a1, uint b1, uint c1, uint d1) = (1,2,3,4);
(uint a2, uint b2, uint c2) = (1,2,3);
(uint a3, uint b3) = (1,2);
a1; b1; c1; d1; a2; b2; c2; a3; b3;
}
}
// ----

View File

@ -1,21 +0,0 @@
contract C {
function three() public returns (uint, uint, uint);
function two() public returns (uint, uint);
function none() public;
function f() public {
(uint a,) = three();
(uint b, uint c,) = two();
(,uint d) = three();
(,uint e, uint g) = two();
var (,,) = three();
var () = none();
a;b;c;d;e;g;
}
}
// ----
// SyntaxError: (307-325): The use of the "var" keyword is disallowed. The declaration part of the statement can be removed, since it is empty.
// SyntaxError: (335-350): The use of the "var" keyword is disallowed. The declaration part of the statement can be removed, since it is empty.
// Warning: (179-198): Different number of components on the left hand side (2) than on the right hand side (3).
// Warning: (208-233): Different number of components on the left hand side (3) than on the right hand side (2).
// Warning: (243-262): Different number of components on the left hand side (2) than on the right hand side (3).
// Warning: (272-297): Different number of components on the left hand side (3) than on the right hand side (2).

View File

@ -1,7 +0,0 @@
contract C {
function one() public returns (uint);
function f() public { (uint a, uint b, ) = one(); }
}
// ----
// Warning: (81-107): Different number of components on the left hand side (3) than on the right hand side (1).
// TypeError: (81-107): Not enough components (1) in value to assign all variables (2).

View File

@ -1,7 +0,0 @@
contract C {
function one() public returns (uint);
function f() public { (uint a, , ) = one(); }
}
// ----
// Warning: (81-101): Different number of components on the left hand side (3) than on the right hand side (1).
// TypeError: (81-101): Not enough components (1) in value to assign all variables (2).

View File

@ -1,7 +0,0 @@
contract C {
function one() public returns (uint);
function f() public { (, , uint a) = one(); }
}
// ----
// Warning: (81-101): Different number of components on the left hand side (3) than on the right hand side (1).
// TypeError: (81-101): Not enough components (1) in value to assign all variables (2).

View File

@ -1,7 +0,0 @@
contract C {
function one() public returns (uint);
function f() public { (, uint a, uint b) = one(); }
}
// ----
// Warning: (81-107): Different number of components on the left hand side (3) than on the right hand side (1).
// TypeError: (81-107): Not enough components (1) in value to assign all variables (2).

View File

@ -1,13 +1,10 @@
contract C {
function f() public {
function f() public pure {
uint a = (1);
(uint b,) = uint8(1);
(uint b,) = (uint8(1),2);
(uint c, uint d) = (uint32(1), 2 + a);
(uint e,) = (uint64(1), 2, b);
(uint e, ,) = (uint64(1), 2, b);
a;b;c;d;e;
}
}
// ----
// Warning: (69-89): Different number of components on the left hand side (2) than on the right hand side (1).
// Warning: (146-175): Different number of components on the left hand side (2) than on the right hand side (3).
// Warning: (17-201): Function state mutability can be restricted to pure

View File

@ -1,7 +0,0 @@
contract C {
function one() public returns (uint);
function f() public { var (,) = one(); }
}
// ----
// SyntaxError: (81-96): The use of the "var" keyword is disallowed. The declaration part of the statement can be removed, since it is empty.
// TypeError: (81-96): Wildcard both at beginning and end of variable declaration list is only allowed if the number of components is equal.

View File

@ -1,7 +0,0 @@
contract C {
function two() public returns (uint, uint);
function f() public { (uint a, uint b, uint c) = two(); }
}
// ----
// Warning: (87-119): Different number of components on the left hand side (3) than on the right hand side (2).
// TypeError: (87-119): Not enough components (2) in value to assign all variables (3).

View File

@ -2,8 +2,8 @@ contract C {
function f() pure public {
(uint a, uint b, uint c) = g();
(uint d) = 2;
(, uint e) = 3;
(uint h,) = 4;
(, uint e) = (3,4);
(uint h,) = (4,5);
(uint x,,) = g();
(, uint y,) = g();
a; b; c; d; e; h; x; y;
@ -11,5 +11,3 @@ contract C {
function g() pure public returns (uint, uint, uint) {}
}
// ----
// Warning: (93-107): Different number of components on the left hand side (2) than on the right hand side (1).
// Warning: (111-124): Different number of components on the left hand side (2) than on the right hand side (1).

View File

@ -1,16 +1,11 @@
contract C {
function f() public {
function f() public pure {
uint a = (1);
(uint b,) = 1;
(uint b,) = (1,2);
(uint c, uint d) = (1, 2 + a);
(uint e,) = (1, 2, b);
(uint e,) = (1, b);
(a) = 3;
a;b;c;d;e;
}
}
// ----
// Warning: (54-67): Different number of components on the left hand side (2) than on the right hand side (1).
// Warning: (104-125): Different number of components on the left hand side (2) than on the right hand side (3).
// Warning: (72-78): Unused local variable.
// Warning: (80-86): Unused local variable.
// Warning: (105-111): Unused local variable.
// Warning: (14-140): Function state mutability can be restricted to pure

View File

@ -1,8 +0,0 @@
contract C {
function f() public pure returns (uint, uint, uint, uint) {
(uint a, uint b,) = f();
a; b;
}
}
// ----
// Warning: (76-99): Different number of components on the left hand side (3) than on the right hand side (4).

View File

@ -16,4 +16,3 @@ contract C {
// SyntaxError: (249-261): The use of the "var" keyword is disallowed. The declaration part of the statement can be removed, since it is empty.
// SyntaxError: (271-283): The use of the "var" keyword is disallowed. The declaration part of the statement can be removed, since it is empty.
// SyntaxError: (293-306): The use of the "var" keyword is disallowed. The declaration part of the statement can be removed, since it is empty.
// TypeError: (271-283): Too many components (1) in value for variable assignment (0) needed