mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Merge pull request #4056 from ethereum/safetupleassignment
Deprecate wildcard assignments.
This commit is contained in:
commit
10792dbc90
@ -6,6 +6,7 @@ Features:
|
|||||||
* Optimizer: Remove unnecessary masking of the result of known short instructions (``ADDRESS``, ``CALLER``, ``ORIGIN`` and ``COINBASE``).
|
* Optimizer: Remove unnecessary masking of the result of known short instructions (``ADDRESS``, ``CALLER``, ``ORIGIN`` and ``COINBASE``).
|
||||||
* Type Checker: Deprecate the ``years`` unit denomination and raise a warning for it (or an error as experimental 0.5.0 feature).
|
* Type Checker: Deprecate the ``years`` unit denomination and raise a warning for it (or an error as experimental 0.5.0 feature).
|
||||||
* Type Checker: Make literals (without explicit type casting) an error for tight packing as experimental 0.5.0 feature.
|
* Type Checker: Make literals (without explicit type casting) an error for tight packing as experimental 0.5.0 feature.
|
||||||
|
* Type Checker: Warn about wildcard tuple assignments (this will turn into an error with version 0.5.0).
|
||||||
|
|
||||||
Bugfixes:
|
Bugfixes:
|
||||||
* Type Checker: Show proper error when trying to ``emit`` a non-event.
|
* Type Checker: Show proper error when trying to ``emit`` a non-event.
|
||||||
|
@ -293,12 +293,7 @@ Solidity internally allows tuple types, i.e. a list of objects of potentially di
|
|||||||
// Common trick to swap values -- does not work for non-value storage types.
|
// Common trick to swap values -- does not work for non-value storage types.
|
||||||
(x, y) = (y, x);
|
(x, y) = (y, x);
|
||||||
// Components can be left out (also for variable declarations).
|
// Components can be left out (also for variable declarations).
|
||||||
// If the tuple ends in an empty component,
|
(data.length,,) = f(); // Sets the length to 7
|
||||||
// the rest of the values are discarded.
|
|
||||||
(data.length,) = f(); // Sets the length to 7
|
|
||||||
// The same can be done on the left side.
|
|
||||||
// If the tuple begins in an empty component, the beginning values are discarded.
|
|
||||||
(,data[3]) = f(); // Sets data[3] to 2
|
|
||||||
// Components can only be left out at the left-hand-side of assignments, with
|
// Components can only be left out at the left-hand-side of assignments, with
|
||||||
// one exception:
|
// one exception:
|
||||||
(x,) = (1,);
|
(x,) = (1,);
|
||||||
@ -307,6 +302,11 @@ Solidity internally allows tuple types, i.e. a list of objects of potentially di
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
.. note::
|
||||||
|
Prior to version 0.4.24 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.
|
||||||
|
|
||||||
Complications for Arrays and Structs
|
Complications for Arrays and Structs
|
||||||
------------------------------------
|
------------------------------------
|
||||||
|
|
||||||
|
@ -1076,6 +1076,7 @@ void TypeChecker::endVisit(EmitStatement const& _emit)
|
|||||||
|
|
||||||
bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
|
bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
|
||||||
{
|
{
|
||||||
|
bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
|
||||||
if (!_statement.initialValue())
|
if (!_statement.initialValue())
|
||||||
{
|
{
|
||||||
// No initial value is only permitted for single variables with specified type.
|
// No initial value is only permitted for single variables with specified type.
|
||||||
@ -1092,7 +1093,7 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
|
|||||||
if (varDecl.referenceLocation() == VariableDeclaration::Location::Default)
|
if (varDecl.referenceLocation() == VariableDeclaration::Location::Default)
|
||||||
errorText += " Did you mean '<type> memory " + varDecl.name() + "'?";
|
errorText += " Did you mean '<type> memory " + varDecl.name() + "'?";
|
||||||
solAssert(m_scope, "");
|
solAssert(m_scope, "");
|
||||||
if (m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050))
|
if (v050)
|
||||||
m_errorReporter.declarationError(varDecl.location(), errorText);
|
m_errorReporter.declarationError(varDecl.location(), errorText);
|
||||||
else
|
else
|
||||||
m_errorReporter.warning(varDecl.location(), errorText);
|
m_errorReporter.warning(varDecl.location(), errorText);
|
||||||
@ -1132,12 +1133,33 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
|
|||||||
") in value for variable assignment (0) needed"
|
") in value for variable assignment (0) needed"
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
else if (valueTypes.size() != variables.size() && !variables.front() && !variables.back())
|
else if (valueTypes.size() != variables.size())
|
||||||
m_errorReporter.fatalTypeError(
|
{
|
||||||
_statement.location(),
|
if (v050)
|
||||||
"Wildcard both at beginning and end of variable declaration list is only allowed "
|
m_errorReporter.fatalTypeError(
|
||||||
"if the number of components is equal."
|
_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();
|
size_t minNumValues = variables.size();
|
||||||
if (!variables.empty() && (!variables.back() || !variables.front()))
|
if (!variables.empty() && (!variables.back() || !variables.front()))
|
||||||
--minNumValues;
|
--minNumValues;
|
||||||
@ -1335,6 +1357,7 @@ bool TypeChecker::visit(Conditional const& _conditional)
|
|||||||
|
|
||||||
bool TypeChecker::visit(Assignment const& _assignment)
|
bool TypeChecker::visit(Assignment const& _assignment)
|
||||||
{
|
{
|
||||||
|
bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
|
||||||
requireLValue(_assignment.leftHandSide());
|
requireLValue(_assignment.leftHandSide());
|
||||||
TypePointer t = type(_assignment.leftHandSide());
|
TypePointer t = type(_assignment.leftHandSide());
|
||||||
_assignment.annotation().type = t;
|
_assignment.annotation().type = t;
|
||||||
@ -1347,11 +1370,29 @@ bool TypeChecker::visit(Assignment const& _assignment)
|
|||||||
);
|
);
|
||||||
// Sequenced assignments of tuples is not valid, make the result a "void" type.
|
// Sequenced assignments of tuples is not valid, make the result a "void" type.
|
||||||
_assignment.annotation().type = make_shared<TupleType>();
|
_assignment.annotation().type = make_shared<TupleType>();
|
||||||
|
|
||||||
expectType(_assignment.rightHandSide(), *tupleType);
|
expectType(_assignment.rightHandSide(), *tupleType);
|
||||||
|
|
||||||
// expectType does not cause fatal errors, so we have to check again here.
|
// expectType does not cause fatal errors, so we have to check again here.
|
||||||
if (dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get()))
|
if (TupleType const* rhsType = dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get()))
|
||||||
|
{
|
||||||
checkDoubleStorageAssignment(_assignment);
|
checkDoubleStorageAssignment(_assignment);
|
||||||
|
// @todo For 0.5.0, this code shoud move to TupleType::isImplicitlyConvertibleTo,
|
||||||
|
// but we cannot do it right now.
|
||||||
|
if (rhsType->components().size() != tupleType->components().size())
|
||||||
|
{
|
||||||
|
string message =
|
||||||
|
"Different number of components on the left hand side (" +
|
||||||
|
toString(tupleType->components().size()) +
|
||||||
|
") than on the right hand side (" +
|
||||||
|
toString(rhsType->components().size()) +
|
||||||
|
").";
|
||||||
|
if (v050)
|
||||||
|
m_errorReporter.typeError(_assignment.location(), message);
|
||||||
|
else
|
||||||
|
m_errorReporter.warning(_assignment.location(), message);
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
else if (t->category() == Type::Category::Mapping)
|
else if (t->category() == Type::Category::Mapping)
|
||||||
{
|
{
|
||||||
|
@ -2894,7 +2894,7 @@ BOOST_AUTO_TEST_CASE(dynamic_return_types_not_possible)
|
|||||||
contract C {
|
contract C {
|
||||||
function f(uint) public returns (string);
|
function f(uint) public returns (string);
|
||||||
function g() public {
|
function g() public {
|
||||||
var (x,) = this.f(2);
|
var x = this.f(2);
|
||||||
// we can assign to x but it is not usable.
|
// we can assign to x but it is not usable.
|
||||||
bytes(x).length;
|
bytes(x).length;
|
||||||
}
|
}
|
||||||
@ -5833,80 +5833,6 @@ BOOST_AUTO_TEST_CASE(pure_statement_check_for_regular_for_loop)
|
|||||||
CHECK_SUCCESS(text);
|
CHECK_SUCCESS(text);
|
||||||
}
|
}
|
||||||
|
|
||||||
BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies)
|
|
||||||
{
|
|
||||||
char const* text = R"(
|
|
||||||
contract C {
|
|
||||||
struct S { uint a; uint b; }
|
|
||||||
S x; S y;
|
|
||||||
function f() public {
|
|
||||||
(x, y) = (y, x);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
)";
|
|
||||||
CHECK_WARNING(text, "This assignment performs two copies to storage.");
|
|
||||||
}
|
|
||||||
|
|
||||||
BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies_fill_right)
|
|
||||||
{
|
|
||||||
char const* text = R"(
|
|
||||||
contract C {
|
|
||||||
struct S { uint a; uint b; }
|
|
||||||
S x; S y;
|
|
||||||
function f() public {
|
|
||||||
(x, y, ) = (y, x, 1, 2);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
)";
|
|
||||||
CHECK_WARNING(text, "This assignment performs two copies to storage.");
|
|
||||||
}
|
|
||||||
|
|
||||||
BOOST_AUTO_TEST_CASE(warn_multiple_storage_storage_copies_fill_left)
|
|
||||||
{
|
|
||||||
char const* text = R"(
|
|
||||||
contract C {
|
|
||||||
struct S { uint a; uint b; }
|
|
||||||
S x; S y;
|
|
||||||
function f() public {
|
|
||||||
(,x, y) = (1, 2, y, x);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
)";
|
|
||||||
CHECK_WARNING(text, "This assignment performs two copies to storage.");
|
|
||||||
}
|
|
||||||
|
|
||||||
BOOST_AUTO_TEST_CASE(nowarn_swap_memory)
|
|
||||||
{
|
|
||||||
char const* text = R"(
|
|
||||||
contract C {
|
|
||||||
struct S { uint a; uint b; }
|
|
||||||
function f() pure public {
|
|
||||||
S memory x;
|
|
||||||
S memory y;
|
|
||||||
(x, y) = (y, x);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
)";
|
|
||||||
CHECK_SUCCESS_NO_WARNINGS(text);
|
|
||||||
}
|
|
||||||
|
|
||||||
BOOST_AUTO_TEST_CASE(nowarn_swap_storage_pointers)
|
|
||||||
{
|
|
||||||
char const* text = R"(
|
|
||||||
contract C {
|
|
||||||
struct S { uint a; uint b; }
|
|
||||||
S x; S y;
|
|
||||||
function f() public {
|
|
||||||
S storage x_local = x;
|
|
||||||
S storage y_local = y;
|
|
||||||
S storage z_local = x;
|
|
||||||
(x, y_local, x_local, z_local) = (y, x_local, y_local, y);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
)";
|
|
||||||
CHECK_SUCCESS_NO_WARNINGS(text);
|
|
||||||
}
|
|
||||||
|
|
||||||
BOOST_AUTO_TEST_CASE(warn_unused_local)
|
BOOST_AUTO_TEST_CASE(warn_unused_local)
|
||||||
{
|
{
|
||||||
char const* text = R"(
|
char const* text = R"(
|
||||||
|
12
test/libsolidity/syntaxTests/tupleAssignments/error_fill.sol
Normal file
12
test/libsolidity/syntaxTests/tupleAssignments/error_fill.sol
Normal file
@ -0,0 +1,12 @@
|
|||||||
|
pragma experimental "v0.5.0";
|
||||||
|
contract C {
|
||||||
|
function f() public pure returns (uint, uint, bytes32) {
|
||||||
|
uint a;
|
||||||
|
bytes32 b;
|
||||||
|
(a,) = f();
|
||||||
|
(,b) = f();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
||||||
|
// TypeError: (126-136): Different number of components on the left hand side (2) than on the right hand side (3).
|
||||||
|
// TypeError: (140-150): Different number of components on the left hand side (2) than on the right hand side (3).
|
@ -0,0 +1,24 @@
|
|||||||
|
pragma experimental "v0.5.0";
|
||||||
|
contract C {
|
||||||
|
function g() public pure returns (
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint,
|
||||||
|
uint
|
||||||
|
) { }
|
||||||
|
function f() public pure returns (uint, uint, bytes32) {
|
||||||
|
uint a;
|
||||||
|
uint b;
|
||||||
|
(,,,,a,,,,b,,,,) = g();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
@ -0,0 +1,8 @@
|
|||||||
|
contract C {
|
||||||
|
function f() public pure {
|
||||||
|
uint a;
|
||||||
|
(a,) = (uint(1),);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
||||||
|
// Warning: (53-70): Different number of components on the left hand side (2) than on the right hand side (1).
|
@ -0,0 +1,8 @@
|
|||||||
|
contract C {
|
||||||
|
struct S { uint a; uint b; }
|
||||||
|
function f() pure public {
|
||||||
|
S memory x;
|
||||||
|
S memory y;
|
||||||
|
(x, y) = (y, x);
|
||||||
|
}
|
||||||
|
}
|
@ -0,0 +1,10 @@
|
|||||||
|
contract C {
|
||||||
|
struct S { uint a; uint b; }
|
||||||
|
S x; S y;
|
||||||
|
function f() public {
|
||||||
|
S storage x_local = x;
|
||||||
|
S storage y_local = y;
|
||||||
|
S storage z_local = x;
|
||||||
|
(x, y_local, x_local, z_local) = (y, x_local, y_local, y);
|
||||||
|
}
|
||||||
|
}
|
@ -0,0 +1,11 @@
|
|||||||
|
contract C {
|
||||||
|
function f() public pure returns (uint, uint, bytes32) {
|
||||||
|
uint a;
|
||||||
|
bytes32 b;
|
||||||
|
(a,) = f();
|
||||||
|
(,b) = f();
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
||||||
|
// Warning: (96-106): Different number of components on the left hand side (2) than on the right hand side (3).
|
||||||
|
// Warning: (110-120): Different number of components on the left hand side (2) than on the right hand side (3).
|
@ -0,0 +1,11 @@
|
|||||||
|
contract C {
|
||||||
|
function f() public pure returns (uint, uint, uint, uint) {
|
||||||
|
// Can later be replaced by (uint a, uint b,) = f();
|
||||||
|
var (a,b,) = f();
|
||||||
|
a; b;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
||||||
|
// Warning: (136-137): Use of the "var" keyword is deprecated.
|
||||||
|
// Warning: (138-139): Use of the "var" keyword is deprecated.
|
||||||
|
// Warning: (131-147): Different number of components on the left hand side (3) than on the right hand side (4).
|
@ -0,0 +1,9 @@
|
|||||||
|
contract C {
|
||||||
|
struct S { uint a; uint b; }
|
||||||
|
S x; S y;
|
||||||
|
function f() public {
|
||||||
|
(x, y) = (y, x);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
||||||
|
// Warning: (79-94): This assignment performs two copies to storage. Since storage copies do not first copy to a temporary location, one of them might be overwritten before the second is executed and thus may have unexpected effects. It is safer to perform the copies separately or assign to storage pointers first.
|
@ -0,0 +1,10 @@
|
|||||||
|
contract C {
|
||||||
|
struct S { uint a; uint b; }
|
||||||
|
S x; S y;
|
||||||
|
function f() public {
|
||||||
|
(,x, y) = (1, 2, y, x);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
||||||
|
// Warning: (79-101): This assignment performs two copies to storage. Since storage copies do not first copy to a temporary location, one of them might be overwritten before the second is executed and thus may have unexpected effects. It is safer to perform the copies separately or assign to storage pointers first.
|
||||||
|
// Warning: (79-101): Different number of components on the left hand side (3) than on the right hand side (4).
|
@ -0,0 +1,10 @@
|
|||||||
|
contract C {
|
||||||
|
struct S { uint a; uint b; }
|
||||||
|
S x; S y;
|
||||||
|
function f() public {
|
||||||
|
(x, y, ) = (y, x, 1, 2);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// ----
|
||||||
|
// Warning: (79-102): This assignment performs two copies to storage. Since storage copies do not first copy to a temporary location, one of them might be overwritten before the second is executed and thus may have unexpected effects. It is safer to perform the copies separately or assign to storage pointers first.
|
||||||
|
// Warning: (79-102): Different number of components on the left hand side (3) than on the right hand side (4).
|
Loading…
Reference in New Issue
Block a user