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

25 files changed, 155 insertions, 25 deletions

diff --git a/Changelog.md b/Changelog.md
index c31aced2..3c4e5a7e 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -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:
 
diff --git a/docs/grammar.txt b/docs/grammar.txt
index 0dda4f49..5d977827 100644
--- a/docs/grammar.txt
+++ b/docs/grammar.txt
@@ -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* '}'
diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp
index f91eaf6e..a051d7f9 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/libsolidity/analysis/ReferencesResolver.cpp
@@ -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;
 	}
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 30302908..60cde33c 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -351,7 +351,7 @@ void TypeChecker::annotateBaseConstructorArguments(
 
 		SourceLocation const* mainLocation = nullptr;
 		SecondarySourceLocation ssl;
-	
+
 		if (
 			_currentContract.location().contains(previousNode->location()) ||
 			_currentContract.location().contains(_argumentNode->location())
diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h
index fa0d6921..d703ae53 100644
--- a/libsolidity/ast/AST.h
+++ b/libsolidity/ast/AST.h
@@ -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,
diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp
index b8e00b60..b7855668 100644
--- a/libsolidity/ast/ASTJsonConverter.cpp
+++ b/libsolidity/ast/ASTJsonConverter.cpp
@@ -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.");
 	}
diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp
index 59668e1d..8620f283 100644
--- a/libsolidity/ast/Types.cpp
+++ b/libsolidity/ast/Types.cpp
@@ -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;
diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp
index e2e1eebc..aec9ebbb 100644
--- a/libsolidity/parsing/Parser.cpp
+++ b/libsolidity/parsing/Parser.cpp
@@ -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;
diff --git a/libsolidity/parsing/Token.h b/libsolidity/parsing/Token.h
index 4d7a7bc6..4d456550 100644
--- a/libsolidity/parsing/Token.h
+++ b/libsolidity/parsing/Token.h
@@ -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; }
@@ -348,7 +349,7 @@ public:
 	unsigned int secondNumber() const { return m_secondNumber; }
 	Token::Value token() const { return m_token; }
 	///if tokValue is set to true, then returns the actual token type name, otherwise, returns full type
-	std::string toString(bool const& tokenValue = false) const 
+	std::string toString(bool const& tokenValue = false) const
 	{
 		std::string name = Token::toString(m_token);
 		if (tokenValue || (firstNumber() == 0 && secondNumber() == 0))
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 3cb7d1eb..260cb35c 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -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)
diff --git a/test/libsolidity/SolidityTypes.cpp b/test/libsolidity/SolidityTypes.cpp
index e6928bb8..ba0e9d4f 100644
--- a/test/libsolidity/SolidityTypes.cpp
+++ b/test/libsolidity/SolidityTypes.cpp
@@ -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"));
diff --git a/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_calldata.sol b/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_calldata.sol
new file mode 100644
index 00000000..781c645a
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_calldata.sol
@@ -0,0 +1,4 @@
+contract test {
+    function f(bytes calldata) external;
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_memory.sol b/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_memory.sol
new file mode 100644
index 00000000..807cc064
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_memory.sol
@@ -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).
diff --git a/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_storage.sol b/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_storage.sol
new file mode 100644
index 00000000..2664dbab
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/externalFunction/function_argument_location_specifier_test_external_storage.sol
@@ -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).
diff --git a/test/libsolidity/syntaxTests/dataLocations/function_argument_location_specifier_test_library.sol b/test/libsolidity/syntaxTests/dataLocations/function_argument_location_specifier_test_library.sol
new file mode 100644
index 00000000..4348482a
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/function_argument_location_specifier_test_library.sol
@@ -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).
diff --git a/test/libsolidity/syntaxTests/dataLocations/function_argument_location_specifier_test_non_reference_type.sol b/test/libsolidity/syntaxTests/dataLocations/function_argument_location_specifier_test_non_reference_type.sol
new file mode 100644
index 00000000..70f6c5eb
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/function_argument_location_specifier_test_non_reference_type.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_calldata.sol b/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_calldata.sol
new file mode 100644
index 00000000..f2740946
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_calldata.sol
@@ -0,0 +1,5 @@
+contract test {
+    function f(bytes calldata) internal;
+}
+// ----
+// TypeError: (31-36): Variable cannot be declared as "calldata" (remove the "calldata" keyword).
diff --git a/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_memory.sol b/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_memory.sol
new file mode 100644
index 00000000..1e5971c4
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_memory.sol
@@ -0,0 +1,4 @@
+contract test {
+    function f(bytes memory) internal;
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_storage.sol b/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_storage.sol
new file mode 100644
index 00000000..56f0fe99
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/internalFunction/function_argument_location_specifier_test_internal_storage.sol
@@ -0,0 +1,4 @@
+contract test {
+    function f(bytes storage) internal;
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_calldata.sol b/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_calldata.sol
new file mode 100644
index 00000000..cb00199f
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_calldata.sol
@@ -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).
diff --git a/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_memory.sol b/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_memory.sol
new file mode 100644
index 00000000..4eebf016
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_memory.sol
@@ -0,0 +1,4 @@
+contract test {
+    function f(bytes memory) public;
+}
+// ----
diff --git a/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_storage.sol b/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_storage.sol
new file mode 100644
index 00000000..9380d9df
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/publicFunction/function_argument_location_specifier_test_public_storage.sol
@@ -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).
diff --git a/test/libsolidity/syntaxTests/dataLocations/variable_declaration_location_specifier_test_non_reference_type.sol b/test/libsolidity/syntaxTests/dataLocations/variable_declaration_location_specifier_test_non_reference_type.sol
new file mode 100644
index 00000000..876d6fd6
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/variable_declaration_location_specifier_test_non_reference_type.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/dataLocations/variable_declaration_location_specifier_test_reference_type.sol b/test/libsolidity/syntaxTests/dataLocations/variable_declaration_location_specifier_test_reference_type.sol
new file mode 100644
index 00000000..bd011f2d
--- /dev/null
+++ b/test/libsolidity/syntaxTests/dataLocations/variable_declaration_location_specifier_test_reference_type.sol
@@ -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.
diff --git a/test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol b/test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol
index e021182a..72b6ce84 100644
--- a/test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol
+++ b/test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol
@@ -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.