From 9428dbc94fe43c0132d1ed6c37cf755fe9855723 Mon Sep 17 00:00:00 2001 From: hrkrshnn Date: Mon, 25 Oct 2021 11:58:49 +0100 Subject: [PATCH 1/3] Moved storage size assert to TypeChecker from DeclarationTypeChecker --- Changelog.md | 2 ++ libsolidity/analysis/DeclarationTypeChecker.cpp | 1 - libsolidity/analysis/TypeChecker.cpp | 8 ++++++++ libsolidity/analysis/TypeChecker.h | 1 + .../userDefinedValueType/forward_reference_array.sol | 4 ++++ 5 files changed, 15 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_array.sol diff --git a/Changelog.md b/Changelog.md index c70922942..ff183e22b 100644 --- a/Changelog.md +++ b/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. diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index b015939e2..d531fd77f 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/libsolidity/analysis/DeclarationTypeChecker.cpp @@ -289,7 +289,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 lengthValue; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index f11756b25..0b5f97719 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1220,6 +1220,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) { diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index bf5039204..f0820d930 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -119,6 +119,7 @@ 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; /// 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. diff --git a/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_array.sol b/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_array.sol new file mode 100644 index 000000000..884fb2b5d --- /dev/null +++ b/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_array.sol @@ -0,0 +1,4 @@ +contract C { + Left[] pu1; +} +type Left is bytes2; From 51009c005d0934f6e32e77fa0277ad847defb69e Mon Sep 17 00:00:00 2001 From: hrkrshnn Date: Tue, 26 Oct 2021 18:31:13 +0200 Subject: [PATCH 2/3] Moved a canBeStored assert for struct members to TypeChecker This is to avoid a assert from failing for forward declared user defined value types. --- libsolidity/analysis/DeclarationTypeChecker.cpp | 1 - libsolidity/analysis/TypeChecker.cpp | 10 ++++++++++ libsolidity/analysis/TypeChecker.h | 1 + .../userDefinedValueType/forward_reference_struct.sol | 3 +++ 4 files changed, 14 insertions(+), 1 deletion(-) create mode 100644 test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_struct.sol diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index d531fd77f..d5392ef84 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/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; } diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 0b5f97719..83a9da55c 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -621,6 +621,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 const& _bases diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index f0820d930..ba445ffbb 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -121,6 +121,7 @@ private: 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 const& _bases); diff --git a/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_struct.sol b/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_struct.sol new file mode 100644 index 000000000..b99cff215 --- /dev/null +++ b/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_struct.sol @@ -0,0 +1,3 @@ +struct S { U u; } +contract C { S s; } +type U is address; From 8815d6f5f09a0e4bd54190ab148f5ca3659a7c02 Mon Sep 17 00:00:00 2001 From: hrkrshnn Date: Tue, 26 Oct 2021 18:29:46 +0200 Subject: [PATCH 3/3] Moved a check related to constants to TypeChecker And added a proper error message when constant types containing (nested) mapping types are used. --- .../analysis/DeclarationTypeChecker.cpp | 18 ++++++++++-------- libsolidity/analysis/TypeChecker.cpp | 9 +++++++++ libsolidity/ast/Types.h | 2 ++ .../constantEvaluator/type_reference.sol | 1 - .../type_reference_in_contract.sol | 1 - .../syntaxTests/constants/mapping_constant.sol | 2 +- .../syntaxTests/constants/struct_constant.sol | 2 +- .../const_struct_with_mapping.sol | 2 +- .../105_constant_input_parameter.sol | 1 - .../171_assignment_to_const_array_vars.sol | 2 +- .../173_constant_struct.sol | 2 +- .../nameAndTypeResolution/constant_mapping.sol | 4 +--- .../constant_nested_mapping.sol | 2 +- .../parsing/location_specifiers_for_params.sol | 1 - ...reference_constant_variable_declaration.sol | 2 ++ 15 files changed, 30 insertions(+), 21 deletions(-) create mode 100644 test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_constant_variable_declaration.sol diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index d5392ef84..076155366 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/libsolidity/analysis/DeclarationTypeChecker.cpp @@ -437,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(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(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; } diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 83a9da55c..05fdc489b 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/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(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) diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 334e69ba1..b08694820 100644 --- a/libsolidity/ast/Types.h +++ b/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; } diff --git a/test/libsolidity/syntaxTests/constantEvaluator/type_reference.sol b/test/libsolidity/syntaxTests/constantEvaluator/type_reference.sol index 4ad0a0c4b..585c005f6 100644 --- a/test/libsolidity/syntaxTests/constantEvaluator/type_reference.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/constantEvaluator/type_reference_in_contract.sol b/test/libsolidity/syntaxTests/constantEvaluator/type_reference_in_contract.sol index 7ce1a7ad3..9073f6ac1 100644 --- a/test/libsolidity/syntaxTests/constantEvaluator/type_reference_in_contract.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/constants/mapping_constant.sol b/test/libsolidity/syntaxTests/constants/mapping_constant.sol index 5821d0d56..4d80fb43e 100644 --- a/test/libsolidity/syntaxTests/constants/mapping_constant.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/constants/struct_constant.sol b/test/libsolidity/syntaxTests/constants/struct_constant.sol index 42f4584ed..8d750beb6 100644 --- a/test/libsolidity/syntaxTests/constants/struct_constant.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/iceRegressionTests/const_struct_with_mapping.sol b/test/libsolidity/syntaxTests/iceRegressionTests/const_struct_with_mapping.sol index 2ee03b09e..f8e0896c3 100644 --- a/test/libsolidity/syntaxTests/iceRegressionTests/const_struct_with_mapping.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/105_constant_input_parameter.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/105_constant_input_parameter.sol index cac87c3dd..aaf6f620a 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/105_constant_input_parameter.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/171_assignment_to_const_array_vars.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/171_assignment_to_const_array_vars.sol index cb39b004b..81b9eab8a 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/171_assignment_to_const_array_vars.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/173_constant_struct.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/173_constant_struct.sol index b9b798431..d32dd2768 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/173_constant_struct.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/constant_mapping.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/constant_mapping.sol index 12d2d9105..7539a99cb 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/constant_mapping.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/constant_nested_mapping.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/constant_nested_mapping.sol index fcc8a432b..81e6e6b15 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/constant_nested_mapping.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol b/test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol index 8e1ca503c..7d3dd5640 100644 --- a/test/libsolidity/syntaxTests/parsing/location_specifiers_for_params.sol +++ b/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. diff --git a/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_constant_variable_declaration.sol b/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_constant_variable_declaration.sol new file mode 100644 index 000000000..a459698a7 --- /dev/null +++ b/test/libsolidity/syntaxTests/userDefinedValueType/forward_reference_constant_variable_declaration.sol @@ -0,0 +1,2 @@ +MyInt constant x = MyInt.wrap(20); +type MyInt is int;