Merge pull request #4478 from ethereum/requireStorageLocation

Turn missing storage locations into an error.
This commit is contained in:
chriseth 2018-07-12 18:00:05 +02:00 committed by GitHub
commit 81271801b4
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
23 changed files with 65 additions and 84 deletions

View File

@ -4,6 +4,7 @@ How to update your code:
* Change every ``.call()`` to a ``.call("")`` and every ``.call(signature, a, b, c)`` to use ``.call(abi.encodeWithSignature(signature, a, b, c))`` (the last one only works for value types).
* Change every ``keccak256(a, b, c)`` to ``keccak256(abi.encodePacked(a, b, c))``.
* Make your fallback functions ``external``.
* Explicitly state the storage location for local variables of struct and array types, e.g. change ``uint[] x = m_x`` to ``uint[] storage x = m_x``.
Breaking Changes:
@ -43,6 +44,7 @@ Breaking Changes:
* Type Checker: Only accept a single ``bytes`` type for ``.call()`` (and family), ``keccak256()``, ``sha256()`` and ``ripemd160()``.
* Type Checker: Fallback function must be external. This was already the case in the experimental 0.5.0 mode.
* Remove obsolete ``std`` directory from the Solidity repository. This means accessing ``https://github.com/ethereum/soldity/blob/develop/std/*.sol`` (or ``https://github.com/ethereum/solidity/std/*.sol`` in Remix) will not be possible.
* References Resolver: Turn missing storage locations into an error. This was already the case in the experimental 0.5.0 mode.
* Syntax Checker: Named return values in function types are an error.
* Syntax Checker: Disallow unary ``+``. This was already the case in the experimental 0.5.0 mode.
* View Pure Checker: Strictly enfore state mutability. This was already the case in the experimental 0.5.0 mode.

View File

@ -273,9 +273,11 @@ of variable it concerns:
* state variables are always in storage
* function arguments are in memory by default
* local variables of struct, array or mapping type reference storage by default
* local variables of mapping type reference storage by default
* local variables of value type (i.e. neither array, nor struct nor mapping) are stored in the stack
For local variables of struct or array type the storage location has to be stated explicitly.
Example::
pragma solidity ^0.4.0;
@ -309,8 +311,9 @@ carry back to ``data1`` or ``data2``.
.. warning::
Prior to version 0.5.0, a common mistake was to declare a local variable and assume that it will
be created in memory, although it will be created in storage. Using such a variable without initializing
it could lead to unexpected behavior. Starting from 0.5.0, however, storage variables have to be initialized,
which should prevent these kinds of mistakes.
could lead to unexpected behavior. Starting from 0.5.0, however, the storage location for local variables
has to be specified explicitly and local storage variables have to be initialized, which should prevent
these kinds of mistakes.
******************
Advanced Questions

View File

@ -377,19 +377,10 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
{
typeLoc = DataLocation::Storage;
if (_variable.isLocalVariable())
{
if (_variable.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050))
typeError(
_variable.location(),
"Data location must be specified as either \"memory\" or \"storage\"."
);
else
m_errorReporter.warning(
_variable.location(),
"Variable is declared as a storage pointer. "
"Use an explicit \"storage\" keyword to silence this warning."
);
}
typeError(
_variable.location(),
"Data location must be specified as either \"memory\" or \"storage\"."
);
}
}
else

View File

@ -225,7 +225,7 @@ contract MultiSigWallet {
notExecuted(transactionId)
{
if (isConfirmed(transactionId)) {
Transaction tx = transactions[transactionId];
Transaction storage tx = transactions[transactionId];
tx.executed = true;
if (tx.destination.call.value(tx.value)(tx.data))
emit Execution(transactionId);

View File

@ -42,7 +42,7 @@ contract MultiSigWalletWithDailyLimit is MultiSigWallet {
public
notExecuted(transactionId)
{
Transaction tx = transactions[transactionId];
Transaction storage tx = transactions[transactionId];
bool confirmed = isConfirmed(transactionId);
if (confirmed || tx.data.length == 0 && isUnderLimit(tx.value)) {
tx.executed = true;

View File

@ -227,7 +227,7 @@ contract MilestoneTracker {
RLP.RLPItem memory itmProposal = itrProposals.next();
Milestone milestone = milestones[milestones.length ++];
Milestone storage milestone = milestones[milestones.length ++];
if (!itmProposal.isList()) throw;
@ -259,7 +259,7 @@ contract MilestoneTracker {
public campaignNotCanceled notChanging
{
if (_idMilestone >= milestones.length) throw;
Milestone milestone = milestones[_idMilestone];
Milestone storage milestone = milestones[_idMilestone];
if ( (msg.sender != milestone.milestoneLeadLink)
&&(msg.sender != recipient))
throw;
@ -277,7 +277,7 @@ contract MilestoneTracker {
public campaignNotCanceled notChanging
{
if (_idMilestone >= milestones.length) throw;
Milestone milestone = milestones[_idMilestone];
Milestone storage milestone = milestones[_idMilestone];
if ((msg.sender != milestone.reviewer) ||
(milestone.status != MilestoneStatus.Completed)) throw;
@ -292,7 +292,7 @@ contract MilestoneTracker {
public campaignNotCanceled notChanging
{
if (_idMilestone >= milestones.length) throw;
Milestone milestone = milestones[_idMilestone];
Milestone storage milestone = milestones[_idMilestone];
if ((msg.sender != milestone.reviewer) ||
(milestone.status != MilestoneStatus.Completed)) throw;
@ -307,7 +307,7 @@ contract MilestoneTracker {
function requestMilestonePayment(uint _idMilestone
) public campaignNotCanceled notChanging {
if (_idMilestone >= milestones.length) throw;
Milestone milestone = milestones[_idMilestone];
Milestone storage milestone = milestones[_idMilestone];
if ( (msg.sender != milestone.milestoneLeadLink)
&&(msg.sender != recipient))
throw;
@ -324,7 +324,7 @@ contract MilestoneTracker {
public onlyRecipient campaignNotCanceled notChanging
{
if (_idMilestone >= milestones.length) throw;
Milestone milestone = milestones[_idMilestone];
Milestone storage milestone = milestones[_idMilestone];
if ((milestone.status != MilestoneStatus.AcceptedAndInProgress) &&
(milestone.status != MilestoneStatus.Completed))
throw;
@ -339,7 +339,7 @@ contract MilestoneTracker {
function arbitrateApproveMilestone(uint _idMilestone
) public onlyArbitrator campaignNotCanceled notChanging {
if (_idMilestone >= milestones.length) throw;
Milestone milestone = milestones[_idMilestone];
Milestone storage milestone = milestones[_idMilestone];
if ((milestone.status != MilestoneStatus.AcceptedAndInProgress) &&
(milestone.status != MilestoneStatus.Completed))
throw;
@ -356,7 +356,7 @@ contract MilestoneTracker {
// @dev This internal function is executed when the milestone is paid out
function authorizePayment(uint _idMilestone) internal {
if (_idMilestone >= milestones.length) throw;
Milestone milestone = milestones[_idMilestone];
Milestone storage milestone = milestones[_idMilestone];
// Recheck again to not pay twice
if (milestone.status == MilestoneStatus.AuthorizedForPayment) throw;
milestone.status = MilestoneStatus.AuthorizedForPayment;

View File

@ -74,7 +74,7 @@ contract VestedToken is StandardToken, LimitedTransferToken {
* @param _grantId The id of the token grant.
*/
function revokeTokenGrant(address _holder, uint256 _grantId) public {
TokenGrant grant = grants[_holder][_grantId];
TokenGrant storage grant = grants[_holder][_grantId];
if (!grant.revokable) { // Check if grant was revokable
throw;
@ -193,7 +193,7 @@ contract VestedToken is StandardToken, LimitedTransferToken {
* revokability, burnsOnRevoke, and vesting) plus the vested value at the current time.
*/
function tokenGrant(address _holder, uint256 _grantId) public view returns (address granter, uint256 value, uint256 vested, uint64 start, uint64 cliff, uint64 vesting, bool revokable, bool burnsOnRevoke) {
TokenGrant grant = grants[_holder][_grantId];
TokenGrant storage grant = grants[_holder][_grantId];
granter = grant.granter;
value = grant.value;

View File

@ -142,7 +142,7 @@ contract GlobalRegistrar is Registrar, AuctionSystem {
throw;
bid(_name, msg.sender, msg.value);
} else {
Record record = m_toRecord[_name];
Record storage record = m_toRecord[_name];
if (record.owner != 0x0000000000000000000000000000000000000000)
throw;
m_toRecord[_name].owner = msg.sender;

View File

@ -75,7 +75,7 @@ contract FixedFeeRegistrar is Registrar {
modifier onlyrecordowner(string _name) { if (m_record(_name).owner == msg.sender) _; }
function reserve(string _name) payable {
Record rec = m_record(_name);
Record storage rec = m_record(_name);
if (rec.owner == 0x0000000000000000000000000000000000000000 && msg.value >= c_fee) {
rec.owner = msg.sender;
emit Changed(_name);
@ -105,7 +105,7 @@ contract FixedFeeRegistrar is Registrar {
}
function record(string _name) view returns (address o_addr, address o_subRegistrar, bytes32 o_content, address o_owner) {
Record rec = m_record(_name);
Record storage rec = m_record(_name);
o_addr = rec.addr;
o_subRegistrar = rec.subRegistrar;
o_content = rec.content;

View File

@ -119,7 +119,7 @@ contract multiowned {
// make sure they're an owner
if (ownerIndex == 0) return;
uint ownerIndexBit = 2**ownerIndex;
PendingState pending = m_pending[_operation];
PendingState storage pending = m_pending[_operation];
if (pending.ownersDone & ownerIndexBit > 0) {
pending.yetNeeded++;
pending.ownersDone -= ownerIndexBit;
@ -178,7 +178,7 @@ contract multiowned {
}
function hasConfirmed(bytes32 _operation, address _owner) view returns (bool) {
PendingState pending = m_pending[_operation];
PendingState storage pending = m_pending[_operation];
uint ownerIndex = m_ownerIndex[uint(_owner)];
// make sure they're an owner
@ -201,7 +201,7 @@ contract multiowned {
// make sure they're an owner
if (ownerIndex == 0) return;
PendingState pending = m_pending[_operation];
PendingState storage pending = m_pending[_operation];
// if we're not yet working on this operation, switch over and reset the confirmation status.
if (pending.yetNeeded == 0) {
// reset count of confirmations needed.

View File

@ -88,7 +88,7 @@ BOOST_AUTO_TEST_CASE(long_type_name_binary_operation)
BOOST_AUTO_TEST_CASE(long_type_name_identifier)
{
CompilerStack c;
c.addSource("a", "contract c { uint[] a; function f() public { uint[] b = a; } }");
c.addSource("a", "contract c { uint[] a; function f() public { uint[] storage b = a; } }");
c.setEVMVersion(dev::test::Options::get().evmVersion());
c.parseAndAnalyze();
map<string, unsigned> sourceIndices;

View File

@ -1535,7 +1535,7 @@ BOOST_AUTO_TEST_CASE(struct_reference)
function set() public {
data.z = 2;
mapping(uint8 => s2) map = data.recursive;
s2 inner = map[0];
s2 storage inner = map[0];
inner.z = 3;
inner.recursive[0].z = inner.recursive[1].z + 1;
}
@ -6374,7 +6374,7 @@ BOOST_AUTO_TEST_CASE(struct_assign_reference_to_struct)
}
function assign() public returns (uint ret_local, uint ret_global, uint ret_global3, uint ret_global1)
{
testStruct x = data1; //x is a reference data1.m_value == 2 as well as x.m_value = 2
testStruct storage x = data1; //x is a reference data1.m_value == 2 as well as x.m_value = 2
data2 = data1; // should copy data. data2.m_value == 2
ret_local = x.m_value; // = 2
@ -6406,7 +6406,7 @@ BOOST_AUTO_TEST_CASE(struct_delete_member)
}
function deleteMember() public returns (uint ret_value)
{
testStruct x = data1; //should not copy the data. data1.m_value == 2 but x.m_value = 0
testStruct storage x = data1; //should not copy the data. data1.m_value == 2 but x.m_value = 0
x.m_value = 4;
delete x.m_value;
ret_value = data1.m_value;
@ -8873,7 +8873,7 @@ BOOST_AUTO_TEST_CASE(inline_assembly_storage_access_via_pointer)
Data public a;
uint public separator2;
function f() public returns (bool) {
Data x = a;
Data storage x = a;
uint off;
assembly {
sstore(x_slot, 7)

View File

@ -208,7 +208,7 @@ BOOST_AUTO_TEST_CASE(external_structs)
struct X { bytes32 x; Test t; Simple[] s; }
function f(ActionChoices, uint, Simple) external {}
function g(Test, Nested) external {}
function h(function(Nested) external returns (uint)[]) external {}
function h(function(Nested memory) external returns (uint)[]) external {}
function i(Nested[]) external {}
}
)";
@ -236,7 +236,7 @@ BOOST_AUTO_TEST_CASE(external_structs_in_libraries)
struct X { bytes32 x; Test t; Simple[] s; }
function f(ActionChoices, uint, Simple) external {}
function g(Test, Nested) external {}
function h(function(Nested) external returns (uint)[]) external {}
function h(function(Nested memory) external returns (uint)[]) external {}
function i(Nested[]) external {}
}
)";

View File

@ -518,8 +518,8 @@ BOOST_AUTO_TEST_CASE(inconsistency)
// Called with params: containerIndex=0, valueIndex=0
function levelIII(uint containerIndex, uint valueIndex) private {
Container container = containers[containerIndex];
Value value = container.values[valueIndex];
Container storage container = containers[containerIndex];
Value storage value = container.values[valueIndex];
debug = container.valueIndices[value.number];
}
function levelII() private {
@ -530,7 +530,7 @@ BOOST_AUTO_TEST_CASE(inconsistency)
function trigger() public returns (uint) {
containers.length++;
Container container = containers[0];
Container storage container = containers[0];
container.values.push(Value({
badnum: 9000,

View File

@ -4,11 +4,10 @@ contract test {
function f() public {
uint[] storage s1 = a;
uint[] memory s2 = new uint[](42);
uint[] s3 = b;
uint[] storage s3 = b;
s1.push(42);
s2[3] = 12;
s3.push(42);
}
}
// ----
// Warning: (147-156): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.

View File

@ -1,6 +1,5 @@
contract C {
function f() public { string x = "abc"; }
function f() public { string storage x = "abc"; }
}
// ----
// Warning: (39-47): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
// TypeError: (39-55): Type literal_string "abc" is not implicitly convertible to expected type string storage pointer.
// TypeError: (39-63): Type literal_string "abc" is not implicitly convertible to expected type string storage pointer.

View File

@ -3,15 +3,14 @@ contract C {
struct S { uint a; uint b; uint[20][20][20] c; R d; }
S data;
function f() public {
C.S x = data;
C.S storage x = data;
C.S memory y;
C.S[10] memory z;
C.S[10];
y.a = 2;
x.c[1][2][3] = 9;
x.d.y[2][2] = 3;
z;
}
}
// ----
// Warning: (150-155): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
// Warning: (194-210): Unused local variable.

View File

@ -1,8 +1,7 @@
contract C {
function f() public {
uint[3] x = [45, 'foo', true];
uint[3] memory x = [45, 'foo', true];
}
}
// ----
// Warning: (47-56): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
// TypeError: (59-76): Unable to deduce common type for array elements.
// TypeError: (66-83): Unable to deduce common type for array elements.

View File

@ -0,0 +1,13 @@
contract C {
struct S { uint a; }
S m_x;
uint[] m_y;
function f() view public {
S x = m_x;
uint[] y = m_y;
x; y;
}
}
// ----
// TypeError: (104-107): Data location must be specified as either "memory" or "storage".
// TypeError: (123-131): Data location must be specified as either "memory" or "storage".

View File

@ -1,10 +0,0 @@
contract C {
struct S { uint a; }
S x;
function f() view public {
S y = x;
y;
}
}
// ----
// Warning: (86-89): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.

View File

@ -1,11 +0,0 @@
pragma experimental "v0.5.0";
contract C {
struct S { uint a; }
S x;
function f() view public {
S y = x;
y;
}
}
// ----
// TypeError: (116-119): Data location must be specified as either "memory" or "storage".

View File

@ -1,8 +1,7 @@
contract C {
function f() pure public {
string x = "abc";
string storage x = "abc";
}
}
// ----
// Warning: (52-60): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
// TypeError: (52-68): Type literal_string "abc" is not implicitly convertible to expected type string storage pointer.
// TypeError: (52-76): Type literal_string "abc" is not implicitly convertible to expected type string storage pointer.

View File

@ -1,8 +1,6 @@
contract c {
function f() public { c[10] a = 7; uint8[10 * 2] x; }
function f() public { c[10] storage a = 7; uint8[10 * 2] storage x; }
}
// ----
// Warning: (39-46): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
// Warning: (52-67): Variable is declared as a storage pointer. Use an explicit "storage" keyword to silence this warning.
// TypeError: (39-50): Type int_const 7 is not implicitly convertible to expected type contract c[10] storage pointer.
// DeclarationError: (52-67): Uninitialized storage pointer. Did you mean '<type> memory x'?
// TypeError: (39-58): Type int_const 7 is not implicitly convertible to expected type contract c[10] storage pointer.
// DeclarationError: (60-83): Uninitialized storage pointer.