Merge pull request #12186 from ethereum/fix-userdefined-ice

Moved storage size assert to TypeChecker from DeclarationTypeChecker

Changelog.md

@@ -23,6 +23,8 @@ Bugfixes:
  * Commandline Interface: When linking only accept exact matches for library names passed to the ``--libraries`` option. Library names not prefixed with a file name used to match any library with that name.
  * SMTChecker: Fix internal error in magic type access (``block``, ``msg``, ``tx``).
  * TypeChecker: Fix internal error when using user defined value types in public library functions.
+ * TypeChecker: Fix internal error when using arrays and structs with user defined value types before declaration.
+ * TypeChecker: Improved error message for constant variables with (nested) mapping types.
  * Yul Assembler: Fix internal error when function names are not unique.
  * Yul IR Generator: Do not output empty switches/if-bodies for empty contracts.
 

libsolidity/analysis/DeclarationTypeChecker.cpp

@@ -100,7 +100,6 @@ bool DeclarationTypeChecker::visit(StructDefinition const& _struct)
 		m_recursiveStructSeen = false;
 		member->accept(*this);
 		solAssert(member->annotation().type, "");
-		solAssert(member->annotation().type->canBeStored(), "Type cannot be used in struct.");
 		if (m_recursiveStructSeen)
 			hasRecursiveChild = true;
 	}
@@ -289,7 +288,6 @@ void DeclarationTypeChecker::endVisit(ArrayTypeName const& _typeName)
 		return;
 	}
 
-	solAssert(baseType->storageBytes() != 0, "Illegal base type of storage size zero for array.");
 	if (Expression const* length = _typeName.length())
 	{
 		optional<rational> lengthValue;
@@ -439,14 +437,16 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable)
 		type = TypeProvider::withLocation(ref, typeLoc, isPointer);
 	}
 
-	if (_variable.isConstant() && !type->isValueType())
-	{
-		bool allowed = false;
-		if (auto arrayType = dynamic_cast<ArrayType const*>(type))
-			allowed = arrayType->isByteArray();
-		if (!allowed)
-			m_errorReporter.fatalDeclarationError(9259_error, _variable.location(), "Constants of non-value type not yet implemented.");
-	}
+	if (
+		_variable.isConstant() &&
+		!dynamic_cast<UserDefinedValueType const*>(type) &&
+		type->containsNestedMapping()
+	)
+		m_errorReporter.fatalDeclarationError(
+			3530_error,
+			_variable.location(),
+			"The type contains a (nested) mapping and therefore cannot be a constant."
+		);
 
 	_variable.annotation().type = type;
 }

libsolidity/analysis/TypeChecker.cpp

@@ -530,6 +530,15 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
 	}
 	if (_variable.isConstant())
 	{
+		if (!varType->isValueType())
+		{
+			bool allowed = false;
+			if (auto arrayType = dynamic_cast<ArrayType const*>(varType))
+				allowed = arrayType->isByteArray();
+			if (!allowed)
+				m_errorReporter.fatalTypeError(9259_error, _variable.location(), "Constants of non-value type not yet implemented.");
+		}
+
 		if (!_variable.value())
 			m_errorReporter.typeError(4266_error, _variable.location(), "Uninitialized \"constant\" variable.");
 		else if (!*_variable.value()->annotation().isPure)
@@ -621,6 +630,16 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
 	return false;
 }
 
+void TypeChecker::endVisit(StructDefinition const& _struct)
+{
+	for (auto const& member: _struct.members())
+		solAssert(
+			member->annotation().type &&
+			member->annotation().type->canBeStored(),
+			"Type cannot be used in struct."
+		);
+}
+
 void TypeChecker::visitManually(
 	ModifierInvocation const& _modifier,
 	vector<ContractDefinition const*> const& _bases
@@ -1220,6 +1239,14 @@ void TypeChecker::endVisit(RevertStatement const& _revert)
 		m_errorReporter.typeError(1885_error, errorCall.expression().location(), "Expression has to be an error.");
 }
 
+void TypeChecker::endVisit(ArrayTypeName const& _typeName)
+{
+	solAssert(
+		_typeName.baseType().annotation().type &&
+		_typeName.baseType().annotation().type->storageBytes() != 0,
+		"Illegal base type of storage size zero for array."
+	);
+}
 
 bool TypeChecker::visit(VariableDeclarationStatement const& _statement)
 {

libsolidity/analysis/TypeChecker.h

@@ -119,7 +119,9 @@ private:
 	void endVisit(InheritanceSpecifier const& _inheritance) override;
 	void endVisit(ModifierDefinition const& _modifier) override;
 	bool visit(FunctionDefinition const& _function) override;
+	void endVisit(ArrayTypeName const& _typeName) override;
 	bool visit(VariableDeclaration const& _variable) override;
+	void endVisit(StructDefinition const& _struct) override;
 	/// We need to do this manually because we want to pass the bases of the current contract in
 	/// case this is a base constructor call.
 	void visitManually(ModifierInvocation const& _modifier, std::vector<ContractDefinition const*> const& _bases);

libsolidity/ast/Types.h

@@ -1124,6 +1124,8 @@ public:
 	bool containsNestedMapping() const override
 	{
 		solAssert(nameable(), "Called for a non nameable type.");
+		// DeclarationTypeChecker::endVisit(VariableDeclaration const&)
+		// assumes that this will never be true.
 		solAssert(!underlyingType().containsNestedMapping(), "");
 		return false;
 	}

test/libsolidity/syntaxTests/constantEvaluator/type_reference.sol

@@ -1,4 +1,3 @@
 int[L] constant L = 6;
 // ----
 // TypeError 5462: (4-5): Invalid array length, expected integer literal or constant expression.
-// DeclarationError 9259: (0-21): Constants of non-value type not yet implemented.

test/libsolidity/syntaxTests/constantEvaluator/type_reference_in_contract.sol

@@ -3,4 +3,3 @@ contract C {
 }
 // ----
 // TypeError 5462: (21-22): Invalid array length, expected integer literal or constant expression.
-// DeclarationError 9259: (17-38): Constants of non-value type not yet implemented.

test/libsolidity/syntaxTests/constants/mapping_constant.sol

@@ -1,3 +1,3 @@
 mapping(uint => uint) constant b = b;
 // ----
-// DeclarationError 9259: (0-36): Constants of non-value type not yet implemented.
+// DeclarationError 3530: (0-36): The type contains a (nested) mapping and therefore cannot be a constant.

test/libsolidity/syntaxTests/constants/struct_constant.sol

@@ -1,4 +1,4 @@
 struct S { uint x; }
 S constant s;
 // ----
-// DeclarationError 9259: (21-33): Constants of non-value type not yet implemented.
+// TypeError 9259: (21-33): Constants of non-value type not yet implemented.

test/libsolidity/syntaxTests/iceRegressionTests/const_struct_with_mapping.sol

@@ -5,4 +5,4 @@ contract C {
     S public constant e = 0x1212121212121212121212121212121212121212;
 }
 // ----
-// DeclarationError 9259: (71-135): Constants of non-value type not yet implemented.
+// DeclarationError 3530: (71-135): The type contains a (nested) mapping and therefore cannot be a constant.

test/libsolidity/syntaxTests/nameAndTypeResolution/105_constant_input_parameter.sol

@@ -3,4 +3,3 @@ contract test {
 }
 // ----
 // DeclarationError 1788: (31-55): The "constant" keyword can only be used for state variables or variables at file level.
-// DeclarationError 9259: (31-55): Constants of non-value type not yet implemented.

test/libsolidity/syntaxTests/nameAndTypeResolution/171_assignment_to_const_array_vars.sol

@@ -2,4 +2,4 @@ contract C {
     uint[3] constant x = [uint(1), 2, 3];
 }
 // ----
-// DeclarationError 9259: (17-53): Constants of non-value type not yet implemented.
+// TypeError 9259: (17-53): Constants of non-value type not yet implemented.

test/libsolidity/syntaxTests/nameAndTypeResolution/173_constant_struct.sol

@@ -3,4 +3,4 @@ contract C {
     S constant x = S(5, new uint[](4));
 }
 // ----
-// DeclarationError 9259: (52-86): Constants of non-value type not yet implemented.
+// TypeError 9259: (52-86): Constants of non-value type not yet implemented.

test/libsolidity/syntaxTests/nameAndTypeResolution/constant_mapping.sol

@@ -1,7 +1,5 @@
 contract C {
-    // This should probably have a better error message at some point.
-    // Constant mappings should not be possible in general.
     mapping(uint => uint) constant x;
 }
 // ----
-// DeclarationError 9259: (148-180): Constants of non-value type not yet implemented.
+// DeclarationError 3530: (17-49): The type contains a (nested) mapping and therefore cannot be a constant.

test/libsolidity/syntaxTests/nameAndTypeResolution/constant_nested_mapping.sol

@@ -5,4 +5,4 @@ contract C {
     S public constant c;
 }
 // ----
-// DeclarationError 9259: (71-90): Constants of non-value type not yet implemented.
+// DeclarationError 3530: (71-90): The type contains a (nested) mapping and therefore cannot be a constant.

test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol

@@ -3,4 +3,3 @@ contract Foo {
 }
 // ----
 // DeclarationError 1788: (30-55): The "constant" keyword can only be used for state variables or variables at file level.
-// DeclarationError 9259: (30-55): Constants of non-value type not yet implemented.

test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_array.sol

@@ -0,0 +1,4 @@
+contract C {
+    Left[] pu1;
+}
+type Left is bytes2;

test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_constant_variable_declaration.sol

@@ -0,0 +1,2 @@
+MyInt constant x = MyInt.wrap(20);
+type MyInt is int;

test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_struct.sol

@@ -0,0 +1,3 @@
+struct S { U u; }
+contract C { S s; }
+type U is address;