Merge pull request #4176 from sifmelcara/add/calldata-keyword

Add a new keyword, "calldata", to allow explicitly specify data location in external function's argument list
This commit is contained in:
chriseth 2018-05-30 14:42:50 +02:00 committed by GitHub
commit 41965ca262
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
25 changed files with 155 additions and 25 deletions

View File

@ -7,7 +7,8 @@ Breaking Changes:
* General: ``continue`` in a ``do...while`` loop jumps to the condition (it used to jump to the loop body). Warning: this may silently change the semantics of existing code.
* Type Checker: Disallow arithmetic operations for Boolean variables.
Features:
Language Features:
* General: Allow appending ``calldata`` keyword to types, to explicitly specify data location for arguments of external functions.
Bugfixes:

View File

@ -57,7 +57,7 @@ Mapping = 'mapping' '(' ElementaryTypeName '=>' TypeName ')'
ArrayTypeName = TypeName '[' Expression? ']'
FunctionTypeName = 'function' FunctionTypeParameterList ( 'internal' | 'external' | StateMutability )*
( 'returns' FunctionTypeParameterList )?
StorageLocation = 'memory' | 'storage'
StorageLocation = 'memory' | 'storage' | 'calldata'
StateMutability = 'pure' | 'constant' | 'view' | 'payable'
Block = '{' Statement* '}'

View File

@ -327,7 +327,7 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
else
{
// force location of external function parameters (not return) to calldata
if (varLoc != Location::Default)
if (varLoc != Location::CallData && varLoc != Location::Default)
fatalTypeError(_variable.location(),
"Location has to be calldata for external functions "
"(remove the \"memory\" or \"storage\" keyword)."
@ -344,15 +344,22 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
*dynamic_cast<Declaration const&>(*_variable.scope()).scope()
);
// force locations of public or external function (return) parameters to memory
if (varLoc == Location::Storage && !contract.isLibrary())
if (varLoc != Location::Memory && varLoc != Location::Default && !contract.isLibrary())
fatalTypeError(_variable.location(),
"Location has to be memory for publicly visible functions "
"(remove the \"storage\" keyword)."
"(remove the \"storage\" or \"calldata\" keyword)."
);
if (varLoc == Location::Default || !contract.isLibrary())
typeLoc = DataLocation::Memory;
else
{
if (varLoc == Location::CallData)
fatalTypeError(_variable.location(),
"Location cannot be calldata for non-external functions "
"(remove the \"calldata\" keyword)."
);
typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage;
}
}
else
{
@ -361,7 +368,7 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
if (varLoc != Location::Default && varLoc != Location::Memory)
fatalTypeError(
_variable.location(),
"Storage location has to be \"memory\" (or unspecified) for constants."
"Data location has to be \"memory\" (or unspecified) for constants."
);
typeLoc = DataLocation::Memory;
}
@ -377,7 +384,7 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
if (_variable.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050))
typeError(
_variable.location(),
"Storage location must be specified as either \"memory\" or \"storage\"."
"Data location must be specified as either \"memory\" or \"storage\"."
);
else
m_errorReporter.warning(
@ -389,14 +396,31 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable)
}
}
else
typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage;
{
switch (varLoc)
{
case Location::Memory:
typeLoc = DataLocation::Memory;
break;
case Location::Storage:
typeLoc = DataLocation::Storage;
break;
case Location::CallData:
fatalTypeError(_variable.location(),
"Variable cannot be declared as \"calldata\" (remove the \"calldata\" keyword)."
);
break;
default:
solAssert(false, "Unknown data location");
}
}
isPointer = !_variable.isStateVariable();
}
type = ref->copyForLocation(typeLoc, isPointer);
}
else if (varLoc != Location::Default && !ref)
typeError(_variable.location(), "Storage location can only be given for array or struct types.");
typeError(_variable.location(), "Data location can only be given for array or struct types.");
_variable.annotation().type = type;
}

View File

@ -656,7 +656,7 @@ private:
class VariableDeclaration: public Declaration
{
public:
enum Location { Default, Storage, Memory };
enum Location { Default, Storage, Memory, CallData };
VariableDeclaration(
SourceLocation const& _sourceLocation,

View File

@ -751,6 +751,8 @@ string ASTJsonConverter::location(VariableDeclaration::Location _location)
return "storage";
case VariableDeclaration::Location::Memory:
return "memory";
case VariableDeclaration::Location::CallData:
return "calldata";
default:
solAssert(false, "Unknown declaration location.");
}

View File

@ -1469,12 +1469,20 @@ string ReferenceType::stringForReferencePart() const
string ReferenceType::identifierLocationSuffix() const
{
string id;
if (location() == DataLocation::Storage)
switch (location())
{
case DataLocation::Storage:
id += "_storage";
else if (location() == DataLocation::Memory)
break;
case DataLocation::Memory:
id += "_memory";
else
break;
case DataLocation::CallData:
id += "_calldata";
break;
default:
solAssert(false, "Unknown location returned by location()");
}
if (isPointer())
id += "_ptr";
return id;

View File

@ -592,11 +592,22 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
else if (!type)
parserError(string("Location specifier needs explicit type name."));
else
location = (
token == Token::Memory ?
VariableDeclaration::Location::Memory :
VariableDeclaration::Location::Storage
);
{
switch (token)
{
case Token::Storage:
location = VariableDeclaration::Location::Storage;
break;
case Token::Memory:
location = VariableDeclaration::Location::Memory;
break;
case Token::CallData:
location = VariableDeclaration::Location::CallData;
break;
default:
solAssert(false, "Unknown data location.");
}
}
}
else
break;

View File

@ -173,6 +173,7 @@ namespace solidity
K(Return, "return", 0) \
K(Returns, "returns", 0) \
K(Storage, "storage", 0) \
K(CallData, "calldata", 0) \
K(Struct, "struct", 0) \
K(Throw, "throw", 0) \
K(Using, "using", 0) \
@ -289,7 +290,7 @@ public:
static bool isShiftOp(Value op) { return (SHL <= op) && (op <= SHR); }
static bool isVisibilitySpecifier(Value op) { return isVariableVisibilitySpecifier(op) || op == External; }
static bool isVariableVisibilitySpecifier(Value op) { return op == Public || op == Private || op == Internal; }
static bool isLocationSpecifier(Value op) { return op == Memory || op == Storage; }
static bool isLocationSpecifier(Value op) { return op == Memory || op == Storage || op == CallData; }
static bool isStateMutabilitySpecifier(Value op) { return op == Pure || op == Constant || op == View || op == Payable; }
static bool isEtherSubdenomination(Value op) { return op == SubWei || op == SubSzabo || op == SubFinney || op == SubEther; }
static bool isTimeSubdenomination(Value op) { return op == SubSecond || op == SubMinute || op == SubHour || op == SubDay || op == SubWeek || op == SubYear; }

View File

@ -6280,7 +6280,7 @@ BOOST_AUTO_TEST_CASE(unspecified_storage_v050)
}
}
)";
CHECK_ERROR(text, TypeError, "Storage location must be specified as either \"memory\" or \"storage\".");
CHECK_ERROR(text, TypeError, "Data location must be specified as either \"memory\" or \"storage\".");
}
BOOST_AUTO_TEST_CASE(storage_location_non_array_or_struct_disallowed)
@ -6290,7 +6290,7 @@ BOOST_AUTO_TEST_CASE(storage_location_non_array_or_struct_disallowed)
function f(uint storage a) public { }
}
)";
CHECK_ERROR(text, TypeError, "Storage location can only be given for array or struct types.");
CHECK_ERROR(text, TypeError, "Data location can only be given for array or struct types.");
}
BOOST_AUTO_TEST_CASE(storage_location_non_array_or_struct_disallowed_is_not_fatal)
@ -6302,7 +6302,7 @@ BOOST_AUTO_TEST_CASE(storage_location_non_array_or_struct_disallowed_is_not_fata
}
}
)";
CHECK_ERROR_ALLOW_MULTI(text, TypeError, (std::vector<std::string>{"Storage location can only be given for array or struct types."}));
CHECK_ERROR_ALLOW_MULTI(text, TypeError, (std::vector<std::string>{"Data location can only be given for array or struct types."}));
}
BOOST_AUTO_TEST_CASE(implicit_conversion_disallowed)

View File

@ -151,7 +151,12 @@ BOOST_AUTO_TEST_CASE(type_identifiers)
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("bool")->identifier(), "t_bool");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("bytes")->identifier(), "t_bytes_storage_ptr");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("bytes memory")->identifier(), "t_bytes_memory_ptr");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("bytes storage")->identifier(), "t_bytes_storage_ptr");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("bytes calldata")->identifier(), "t_bytes_calldata_ptr");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("string")->identifier(), "t_string_storage_ptr");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("string memory")->identifier(), "t_string_memory_ptr");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("string storage")->identifier(), "t_string_storage_ptr");
BOOST_CHECK_EQUAL(Type::fromElementaryTypeName("string calldata")->identifier(), "t_string_calldata_ptr");
ArrayType largeintArray(DataLocation::Memory, Type::fromElementaryTypeName("int128"), u256("2535301200456458802993406410752"));
BOOST_CHECK_EQUAL(largeintArray.identifier(), "t_array$_t_int128_$2535301200456458802993406410752_memory_ptr");
TypePointer stringArray = make_shared<ArrayType>(DataLocation::Storage, Type::fromElementaryTypeName("string"), u256("20"));

View File

@ -0,0 +1,4 @@
contract test {
function f(bytes calldata) external;
}
// ----

View File

@ -0,0 +1,5 @@
contract test {
function f(bytes memory) external;
}
// ----
// TypeError: (31-36): Location has to be calldata for external functions (remove the "memory" or "storage" keyword).

View File

@ -0,0 +1,5 @@
contract test {
function f(bytes storage) external;
}
// ----
// TypeError: (31-36): Location has to be calldata for external functions (remove the "memory" or "storage" keyword).

View File

@ -0,0 +1,5 @@
library test {
function f(bytes calldata) public;
}
// ----
// TypeError: (30-35): Location cannot be calldata for non-external functions (remove the "calldata" keyword).

View File

@ -0,0 +1,5 @@
contract test {
function f(bytes4 memory) public;
}
// ----
// TypeError: (31-37): Data location can only be given for array or struct types.

View File

@ -0,0 +1,5 @@
contract test {
function f(bytes calldata) internal;
}
// ----
// TypeError: (31-36): Variable cannot be declared as "calldata" (remove the "calldata" keyword).

View File

@ -0,0 +1,4 @@
contract test {
function f(bytes memory) internal;
}
// ----

View File

@ -0,0 +1,4 @@
contract test {
function f(bytes storage) internal;
}
// ----

View File

@ -0,0 +1,5 @@
contract test {
function f(bytes calldata) public;
}
// ----
// TypeError: (31-36): Location has to be memory for publicly visible functions (remove the "storage" or "calldata" keyword).

View File

@ -0,0 +1,4 @@
contract test {
function f(bytes memory) public;
}
// ----

View File

@ -0,0 +1,5 @@
contract test {
function f(bytes storage) public;
}
// ----
// TypeError: (31-36): Location has to be memory for publicly visible functions (remove the "storage" or "calldata" keyword).

View File

@ -0,0 +1,13 @@
contract test {
function f() {
uint storage a1;
bytes16 storage b1;
uint memory a2;
bytes16 memory b2;
}
}
// ----
// TypeError: (41-56): Data location can only be given for array or struct types.
// TypeError: (64-82): Data location can only be given for array or struct types.
// TypeError: (90-104): Data location can only be given for array or struct types.
// TypeError: (112-129): Data location can only be given for array or struct types.

View File

@ -0,0 +1,14 @@
contract test {
uint[] a;
uint[] b;
function f() public {
uint[] storage s1 = a;
uint[] memory s2 = new uint[](42);
uint[] 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

@ -2,4 +2,4 @@ contract Foo {
function f(uint[] storage constant x, uint[] memory y) internal { }
}
// ----
// TypeError: (30-55): Storage location has to be "memory" (or unspecified) for constants.
// TypeError: (30-55): Data location has to be "memory" (or unspecified) for constants.