Merge pull request #9325 from ethereum/develop

Merge develop into breaking.

Changelog.md

@@ -44,7 +44,9 @@ Bugfixes:
  * NatSpec: Do not consider ``////`` and ``/***`` as NatSpec comments.
  * Type Checker: Fix internal error related to ``using for`` applied to non-libraries.
  * Type Checker: Do not disallow assigning to calldata variables.
+ * Type Checker: Disallow constructor parameters with ``calldata`` data location.
  * Wasm backend: Fix code generation for for-loops with pre statements.
+ * Wasm backend: Properly support both ``i32.drop`` and ``i64.drop``, and remove ``drop``.
  * Yul: Fix source location of variable multi-assignment.
 
 

libsolidity/analysis/StaticAnalyzer.cpp

@@ -163,7 +163,8 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
 			m_errorReporter.warning(
 				3408_error,
 				_variable.location(),
-				"Variable covers a large part of storage and thus makes collisions likely. "
+				"Variable " + util::escapeAndQuoteString(_variable.name()) +
+				" covers a large part of storage and thus makes collisions likely. "
 				"Either use mappings or dynamic arrays and allow their size to be increased only "
 				"in small quantities per transaction."
 			);
@@ -171,7 +172,8 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
 			m_errorReporter.warning(
 				7325_error,
 				_variable.location(),
-				"Type " + type->canonicalName() + " has large size and thus makes collisions likely. "
+				"Type " + util::escapeAndQuoteString(type->canonicalName()) +
+				" has large size and thus makes collisions likely. "
 				"Either use mappings or dynamic arrays and allow their size to be increased only "
 				"in small quantities per transaction."
 			);

libsolidity/analysis/TypeChecker.cpp

@@ -1343,7 +1343,7 @@ bool TypeChecker::visit(Conditional const& _conditional)
 					_conditional.location(),
 					"True expression's type " +
 					trueType->toString() +
-					" doesn't match false expression's type " +
+					" does not match false expression's type " +
 					falseType->toString() +
 					"."
 					);

libsolidity/ast/AST.cpp

@@ -588,6 +588,15 @@ bool VariableDeclaration::isInternalCallableParameter() const
 	return false;
 }
 
+bool VariableDeclaration::isConstructorParameter() const
+{
+	if (!isCallableOrCatchParameter())
+		return false;
+	if (auto const* function = dynamic_cast<FunctionDefinition const*>(scope()))
+		return function->isConstructor();
+	return false;
+}
+
 bool VariableDeclaration::isLibraryFunctionParameter() const
 {
 	if (!isCallableOrCatchParameter())
@@ -622,7 +631,7 @@ set<VariableDeclaration::Location> VariableDeclaration::allowedDataLocations() c
 		set<Location> locations{ Location::Memory };
 		if (isInternalCallableParameter() || isLibraryFunctionParameter() || isTryCatchParameter())
 			locations.insert(Location::Storage);
-		if (!isTryCatchParameter())
+		if (!isTryCatchParameter() && !isConstructorParameter())
 			locations.insert(Location::CallData);
 
 		return locations;

libsolidity/ast/AST.h

@@ -928,6 +928,7 @@ public:
 	/// @returns true if this variable is a parameter or return parameter of an internal function
 	/// or a function type of internal visibility.
 	bool isInternalCallableParameter() const;
+	bool isConstructorParameter() const;
 	/// @returns true iff this variable is a parameter(or return parameter of a library function
 	bool isLibraryFunctionParameter() const;
 	/// @returns true if the type of the variable does not need to be specified, i.e. it is declared

libyul/AsmAnalysis.cpp

@@ -558,6 +558,13 @@ bool AsmAnalyzer::validateInstructions(evmasm::Instruction _instr, SourceLocatio
 	// Similarly we assume bitwise shifting and create2 go together.
 	yulAssert(m_evmVersion.hasBitwiseShifting() == m_evmVersion.hasCreate2(), "");
 
+	// These instructions are disabled in the dialect.
+	yulAssert(
+		_instr != evmasm::Instruction::JUMP &&
+		_instr != evmasm::Instruction::JUMPI &&
+		_instr != evmasm::Instruction::JUMPDEST,
+	"");
+
 	auto errorForVM = [&](ErrorId _errorId, string const& vmKindMessage) {
 		m_errorReporter.typeError(
 			_errorId,
@@ -604,19 +611,6 @@ bool AsmAnalyzer::validateInstructions(evmasm::Instruction _instr, SourceLocatio
 		);
 	else if (_instr == evmasm::Instruction::SELFBALANCE && !m_evmVersion.hasSelfBalance())
 		errorForVM(3672_error, "only available for Istanbul-compatible");
-	else if (
-		_instr == evmasm::Instruction::JUMP ||
-		_instr == evmasm::Instruction::JUMPI ||
-		_instr == evmasm::Instruction::JUMPDEST
-	)
-		m_errorReporter.error(
-			4316_error,
-			Error::Type::SyntaxError,
-			_location,
-			"Jump instructions and labels are low-level EVM features that can lead to "
-			"incorrect stack access. Because of that they are disallowed in strict assembly. "
-			"Use functions, \"switch\", \"if\" or \"for\" statements instead."
-		);
 	else
 		return false;
 

libyul/backends/evm/EVMDialect.cpp

@@ -114,12 +114,15 @@ pair<YulString, BuiltinFunctionForEVM> createFunction(
 map<YulString, BuiltinFunctionForEVM> createBuiltins(langutil::EVMVersion _evmVersion, bool _objectAccess)
 {
 	map<YulString, BuiltinFunctionForEVM> builtins;
+	// NOTE: Parser::instructions() will filter JUMPDEST and PUSHnn too
 	for (auto const& instr: Parser::instructions())
 		if (
 			!evmasm::isDupInstruction(instr.second) &&
 			!evmasm::isSwapInstruction(instr.second) &&
+			!evmasm::isPushInstruction(instr.second) &&
 			instr.second != evmasm::Instruction::JUMP &&
 			instr.second != evmasm::Instruction::JUMPI &&
+			instr.second != evmasm::Instruction::JUMPDEST &&
 			_evmVersion.hasOpcode(instr.second)
 		)
 			builtins.emplace(createEVMFunction(instr.first, instr.second));

libyul/backends/wasm/BinaryTransform.cpp

@@ -390,7 +390,7 @@ bytes BinaryTransform::operator()(BuiltinCall const& _call)
 		return toBytes(Opcode::Unreachable);
 	else if (_call.functionName == "nop")
 		return toBytes(Opcode::Nop);
-	else if (_call.functionName == "drop")
+	else if (_call.functionName == "i32.drop" || _call.functionName == "i64.drop")
 		return toBytes(Opcode::Drop);
 	else
 	{

libyul/backends/wasm/TextTransform.cpp

@@ -94,7 +94,10 @@ string TextTransform::operator()(wasm::GlobalVariable const& _identifier)
 string TextTransform::operator()(wasm::BuiltinCall const& _builtinCall)
 {
 	string args = joinTransformed(_builtinCall.arguments);
-	return "(" + _builtinCall.functionName + (args.empty() ? "" : " " + args) + ")";
+	string funcName = _builtinCall.functionName;
+	if (funcName == "i32.drop" || funcName == "i64.drop")
+		funcName = "drop";
+	return "(" + funcName + (args.empty() ? "" : " " + args) + ")";
 }
 
 string TextTransform::operator()(wasm::FunctionCall const& _functionCall)

libyul/backends/wasm/WasmDialect.cpp

@@ -91,9 +91,9 @@ WasmDialect::WasmDialect()
 	m_functions["i64.load"_yulstring].sideEffects.sideEffectFreeIfNoMSize = true;
 
 	// Drop is actually overloaded for all types, but Yul does not support that.
-	// Because of that, we introduce "i32.drop".
+	// Because of that, we introduce "i32.drop" and "i64.drop".
-	addFunction("drop", {i64}, {});
 	addFunction("i32.drop", {i32}, {});
+	addFunction("i64.drop", {i64}, {});
 
 	addFunction("nop", {}, {});
 	addFunction("unreachable", {}, {}, false);
@@ -122,7 +122,7 @@ BuiltinFunction const* WasmDialect::discardFunction(YulString _type) const
 	if (_type == "i32"_yulstring)
 		return builtin("i32.drop"_yulstring);
 	yulAssert(_type == "i64"_yulstring, "");
-	return builtin("drop"_yulstring);
+	return builtin("i64.drop"_yulstring);
 }
 
 BuiltinFunction const* WasmDialect::equalityFunction(YulString _type) const

test/libsolidity/syntaxTests/constructor/calldata_constructor_args.sol

@@ -0,0 +1,5 @@
+contract C {
+    constructor(uint[] calldata) public {}
+}
+// ----
+// TypeError 6651: (29-44): Data location must be "memory" for parameter in function, but "calldata" was given.

test/libsolidity/syntaxTests/inlineAssembly/invalid/jumpi_disallowed.sol

@@ -0,0 +1,9 @@
+contract C {
+    function f() pure public {
+        assembly {
+            jumpi(2, 1)
+        }
+    }
+}
+// ----
+// DeclarationError 4619: (75-80): Function not found.

test/libsolidity/syntaxTests/largeTypes/large_storage_array_mapping.sol

@@ -2,4 +2,4 @@ contract C {
     mapping(uint => uint[2**100]) x;
 }
 // ----
-// Warning 7325: (17-48): Type uint256[1267650600228229401496703205376] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (17-48): Type "uint256[1267650600228229401496703205376]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol

@@ -2,4 +2,4 @@ contract C {
     uint[2**64] x;
 }
 // ----
-// Warning 3408: (17-30): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (17-30): Variable "x" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol

@@ -3,4 +3,4 @@ contract C {
     S[2**20] x;
 }
 // ----
-// Warning 3408: (64-74): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (64-74): Variable "x" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol

@@ -2,64 +2,66 @@ contract C {
     struct P { uint256[2**63] x; }
 
     struct S0 {
-        P[2**62] x;
+        P[101] x;
         P y;
     }
     S0 s0;
 
     struct S1 {
         P x;
-        P[2**62] y;
+        P[102] y;
     }
     S1 s1;
 
     struct S2 {
-        mapping(uint => P[2**62]) x;
+        mapping(uint => P[103]) x;
-        mapping(uint => P[2**62]) y;
+        mapping(uint => P[103]) y;
-        mapping(uint => S2) z;
+        mapping(uint => P[104]) z;
+        mapping(uint => S2) t;
     }
     S2 s2;
 
     struct Q0
     {
-        uint[1][][2**65] x;
+        uint[1][][10**20 + 4] x;
-        uint[2**65][][1] y;
+        uint[10**20 + 4][][1] y;
-        uint[][2**65] z;
+        uint[][10**20 + 4] z;
-        uint[2**65][] t;
+        uint[10**20 + 4][] t;
     }
     Q0 q0;
 
     struct Q1
     {
-        uint[1][][2**65] x;
+        uint[1][][10**20 + 5] x;
     }
     Q1 q1;
 
     struct Q2
     {
-        uint[2**65][][1] y;
+        uint[10**20 + 6][][1] y;
     }
     Q2 q2;
 
     struct Q3
     {
-        uint[][2**65] z;
+        uint[][10**20 + 7] x;
     }
     Q3 q3;
 
     struct Q4
     {
-        uint[2**65][] t;
+        uint[10**20 + 8][] y;
     }
     Q4 q4;
 }
 // ----
-// Warning 3408: (108-113): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (106-111): Variable "s0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 3408: (175-180): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (171-176): Variable "s1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 7325: (314-319): Type C.P[4611686018427387904] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (341-346): Type "C.P[103]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 3408: (458-463): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (341-346): Type "C.P[104]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 7325: (458-463): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (505-510): Variable "q0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 3408: (524-529): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (505-510): Type "uint256[100000000000000000004]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 7325: (590-595): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (576-581): Variable "q1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 3408: (653-658): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (647-652): Type "uint256[100000000000000000006]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
-// Warning 7325: (716-721): Type uint256[36893488147419103232] has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (715-720): Variable "q3" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 7325: (783-788): Type "uint256[100000000000000000008]" has large size and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/libsolidity/syntaxTests/nameAndTypeResolution/282_invalid_different_types_for_conditional_expression.sol

@@ -4,4 +4,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1080: (47-62): True expression's type bool doesn't match false expression's type uint8.
+// TypeError 1080: (47-62): True expression's type bool does not match false expression's type uint8.

test/libsolidity/syntaxTests/nameAndTypeResolution/284_conditional_expression_with_different_struct.sol

@@ -12,4 +12,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1080: (165-177): True expression's type struct C.s1 memory doesn't match false expression's type struct C.s2 memory.
+// TypeError 1080: (165-177): True expression's type struct C.s1 memory does not match false expression's type struct C.s2 memory.

test/libsolidity/syntaxTests/nameAndTypeResolution/285_conditional_expression_with_different_function_type.sol

@@ -7,4 +7,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1080: (106-118): True expression's type function (bool) doesn't match false expression's type function ().
+// TypeError 1080: (106-118): True expression's type function (bool) does not match false expression's type function ().

test/libsolidity/syntaxTests/nameAndTypeResolution/286_conditional_expression_with_different_enum.sol

@@ -10,4 +10,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1080: (139-151): True expression's type enum C.small doesn't match false expression's type enum C.big.
+// TypeError 1080: (139-151): True expression's type enum C.small does not match false expression's type enum C.big.

test/libsolidity/syntaxTests/nameAndTypeResolution/287_conditional_expression_with_different_mapping.sol

@@ -7,4 +7,4 @@ contract C {
     }
 }
 // ----
-// TypeError 1080: (121-143): True expression's type mapping(uint8 => uint8) doesn't match false expression's type mapping(uint32 => uint8).
+// TypeError 1080: (121-143): True expression's type mapping(uint8 => uint8) does not match false expression's type mapping(uint32 => uint8).

test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol

@@ -2,4 +2,4 @@ contract c {
     uint[2**253] data;
 }
 // ----
-// Warning 3408: (17-34): Variable covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.
+// Warning 3408: (17-34): Variable "data" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.

test/tools/yulInterpreter/EwasmBuiltinInterpreter.cpp

@@ -93,7 +93,7 @@ u256 EwasmBuiltinInterpreter::evalBuiltin(YulString _fun, vector<u256> const& _a
 			);
 		return 0;
 	}
-	else if (_fun == "drop"_yulstring || _fun == "nop"_yulstring)
+	else if (_fun == "i32.drop"_yulstring || _fun == "i64.drop"_yulstring || _fun == "nop"_yulstring)
 		return {};
 	else if (_fun == "i32.wrap_i64"_yulstring)
 		return arg.at(0) & uint32_t(-1);