Merge pull request #11456 from ethereum/ice-abstract-contract-mapping-constructor

Fix ICE related to mapping types in abstract contract constructor

Changelog.md

@@ -18,6 +18,7 @@ Bugfixes:
  * AST: Do not output value of Yul literal if it is not a valid UTF-8 string.
  * SMTChecker: Fix internal error on struct constructor with fixed bytes member initialized with string literal.
  * Standard JSON: Properly allow the ``inliner`` setting under ``settings.optimizer.details``.
+ * Type Checker: Fix internal compiler error related to having mapping types in constructor parameter for abstract contracts.
 
 
 AST Changes:

libsolidity/analysis/TypeChecker.cpp

@@ -588,7 +588,12 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
 		if (result)
 		{
 			bool isLibraryStorageParameter = (_variable.isLibraryFunctionParameter() && referenceType->location() == DataLocation::Storage);
-			bool callDataCheckRequired = ((_variable.isConstructorParameter() || _variable.isPublicCallableParameter()) && !isLibraryStorageParameter);
+			// We skip the calldata check for abstract contract constructors.
+			bool isAbstractConstructorParam = _variable.isConstructorParameter() && m_currentContract && m_currentContract->abstract();
+			bool callDataCheckRequired =
+				!isAbstractConstructorParam &&
+				(_variable.isConstructorParameter() || _variable.isPublicCallableParameter()) &&
+				!isLibraryStorageParameter;
 			if (callDataCheckRequired)
 			{
 				if (!referenceType->interfaceType(false))

test/libsolidity/semanticTests/types/array_mapping_abstract_constructor_param.sol

@@ -0,0 +1,19 @@
+abstract contract A {
+	constructor (mapping (uint => uint) [] storage m) {
+		m.push();
+		m[0][1] = 2;
+	}
+}
+
+contract C is A {
+	mapping(uint => mapping (uint => uint) []) public m;
+
+	constructor() A(m[1]) {
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// m(uint256,uint256,uint256): 0, 0, 0 -> FAILURE
+// m(uint256,uint256,uint256): 1, 0, 1 -> 2
+// m(uint256,uint256,uint256): 1, 0, 5 -> 0

test/libsolidity/semanticTests/types/mapping_abstract_constructor_param.sol

@@ -0,0 +1,17 @@
+abstract contract A {
+    constructor (mapping (uint => uint) storage m) {
+        m[5] = 20;
+    }
+}
+
+contract C is A {
+    mapping (uint => uint) public m;
+
+    constructor() A(m) {
+    }
+}
+// ====
+// compileViaYul: also
+// ----
+// m(uint256): 1 -> 0
+// m(uint256): 5 -> 20

test/libsolidity/semanticTests/types/struct_mapping_abstract_constructor_param.sol

@@ -0,0 +1,26 @@
+struct S {
+	mapping (uint => uint) m;
+}
+
+abstract contract A {
+	constructor (S storage s) {
+		s.m[5] = 16;
+	}
+}
+
+contract C is A {
+	mapping(uint => S) m;
+
+	constructor() A(m[1]) {
+	}
+
+	function getM(uint a, uint b) external returns (uint) {
+		return m[a].m[b];
+	}
+}
+// ====
+// compileViaYul: also
+// ----
+// getM(uint256,uint256): 0, 0 -> 0
+// getM(uint256,uint256): 1, 5 -> 0x10
+// getM(uint256,uint256): 1, 0 -> 0

test/libsolidity/syntaxTests/types/mapping/abstract_contructor_param.sol

@@ -0,0 +1,4 @@
+abstract contract A {
+    constructor (mapping (uint => uint) [] storage) { }
+}
+// ----

test/libsolidity/syntaxTests/types/mapping/constructor_parameter.sol

@@ -0,0 +1,5 @@
+contract A {
+    constructor (mapping (uint => uint) [] storage) { }
+}
+// ----
+// TypeError 3644: (30-63): This parameter has a type that can only be used internally. You can make the contract abstract to avoid this problem.