From 6feb8aea73be36a18f3a618f1f6d7190f274de66 Mon Sep 17 00:00:00 2001 From: wechman Date: Fri, 5 Aug 2022 13:11:38 +0200 Subject: [PATCH] Rework error handlig of user type operators --- libsolidity/analysis/TypeChecker.cpp | 141 +++++++++++------- libsolidity/ast/Types.cpp | 10 +- .../custom/implicit_conversion_is_blocked.sol | 34 +++++ .../custom/operator_invalid_parameters.sol | 18 +-- .../operator_invalid_return_parameters.sol | 6 - ...rameters_with_different_data_locations.sol | 2 +- .../operator_with_calldata_parameters.sol | 9 +- ...ator_with_different_param_return_types.sol | 6 +- .../operator_with_storage_parameters.sol | 10 +- ...ator_attached_to_two_argument_function.sol | 2 +- 10 files changed, 152 insertions(+), 86 deletions(-) create mode 100644 test/libsolidity/syntaxTests/operators/custom/implicit_conversion_is_blocked.sol diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index fef0ed27a..267a772e1 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1750,37 +1750,26 @@ bool TypeChecker::visit(UnaryOperation const& _operation) solAssert(!builtinResult || !userDefinedOperatorResult); if (userDefinedOperatorResult) { - if (userDefinedFunctionType->returnParameterTypes().size() != 1) - { - m_errorReporter.typeError( - 3138_error, - _operation.location(), - "User defined operator " + string(TokenTraits::toString(_operation.getOperator())) + - " needs to return exactly one value." - ); - _operation.annotation().type = subExprType; - } - else if (*userDefinedFunctionType->returnParameterTypes().front() != *userDefinedFunctionType->parameterTypes().front()) + Type const* normalizedSubExprType = subExprType; + Type const* normalizedParameterType = userDefinedFunctionType->parameterTypes().front(); + + if (auto const* subExprReference = dynamic_cast(normalizedSubExprType)) + normalizedSubExprType = TypeProvider::withLocationIfReference(subExprReference->location(), normalizedSubExprType); + if (auto const* parameterReferenceType = dynamic_cast(normalizedParameterType)) + normalizedParameterType = TypeProvider::withLocationIfReference(parameterReferenceType->location(), normalizedParameterType); + + if (*normalizedSubExprType != *normalizedParameterType) { m_errorReporter.typeError( 7983_error, _operation.location(), "User defined operator " + string(TokenTraits::toString(_operation.getOperator())) + - " needs to return value of type " + + " needs a value of type " + userDefinedFunctionType->parameterTypes().front()->humanReadableName() + "." ); - _operation.annotation().type = subExprType; - } - else - { - solAssert(userDefinedFunctionType->parameterTypes().size() == 1); - solAssert( - *userDefinedFunctionType->parameterTypes().at(0) == - *userDefinedFunctionType->returnParameterTypes().at(0) - ); - _operation.annotation().type = userDefinedFunctionType->returnParameterTypes().at(0); } + _operation.annotation().type = subExprType; } else if (builtinResult) _operation.annotation().type = builtinResult; @@ -1859,13 +1848,32 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) commonType = builtinResult.get(); else if (userDefinedOperatorResult) { - if (userDefinedFunctionType->parameterTypes().size() != 2 || - *userDefinedFunctionType->parameterTypes().at(0) != *userDefinedFunctionType->parameterTypes().at(1)) + Type const* normalizedParameterType = userDefinedFunctionType->parameterTypes().at(0); + Type const* normalizedLeftType = leftType; + Type const* normalizedRightType = rightType; + + if (auto const* parameterReference = dynamic_cast(normalizedParameterType)) + normalizedParameterType = TypeProvider::withLocationIfReference(parameterReference->location(), normalizedParameterType); + if (auto const* leftReferenceType = dynamic_cast(normalizedLeftType)) + normalizedLeftType = TypeProvider::withLocationIfReference(leftReferenceType->location(), normalizedLeftType); + if (auto const* rightReferenceType = dynamic_cast(normalizedRightType)) + normalizedRightType = TypeProvider::withLocationIfReference(rightReferenceType->location(), normalizedRightType); + + if ( + userDefinedFunctionType->parameterTypes().size() != 2 || + *normalizedLeftType != *normalizedParameterType || + *normalizedRightType != *normalizedParameterType + ) m_errorReporter.typeError( 5653_error, _operation.location(), - "User defined operator " + string(TokenTraits::toString(_operation.getOperator())) + - " needs to have two parameters of equal type." + "User defined operator " + + string(TokenTraits::toString(_operation.getOperator())) + + " not compatible with types " + + leftType->humanReadableName() + + " and " + + rightType->humanReadableName() + + "." ); else if (userDefinedFunctionType->returnParameterTypes().size() == 1) commonType = userDefinedFunctionType->parameterTypes().at(0); @@ -1877,26 +1885,11 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) TypeProvider::boolean() : commonType; - if (userDefinedOperatorResult) - { - if (userDefinedFunctionType->returnParameterTypes().size() != 1) - m_errorReporter.typeError( - 1208_error, - _operation.location(), - "User defined operator " + string(TokenTraits::toString(_operation.getOperator())) + - " needs to return exactly one value." - ); - else if (*userDefinedFunctionType->returnParameterTypes().front() != *_operation.annotation().type) - m_errorReporter.typeError( - 3841_error, - _operation.location(), - "User defined operator " + string(TokenTraits::toString(_operation.getOperator())) + - " needs to return value of type " + - _operation.annotation().type->humanReadableName() + "." - ); - } - else if (_operation.getOperator() == Token::Exp || _operation.getOperator() == Token::SHL) + if ( + !userDefinedOperatorResult && + (_operation.getOperator() == Token::Exp || _operation.getOperator() == Token::SHL) + ) { string operation = _operation.getOperator() == Token::Exp ? "exponentiation" : "shift"; if ( @@ -3929,7 +3922,7 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor) BoolResult result = normalizedType->isImplicitlyConvertibleTo( *TypeProvider::withLocationIfReference(DataLocation::Storage, functionType->selfType()) ); - if (!result) + if (!result && !operator_) m_errorReporter.typeError( 3100_error, path->location(), @@ -3971,23 +3964,62 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor) 1884_error, path->location(), "The function \"" + joinHumanReadable(path->path(), ".") + "\" "+ - "needs to have two parameters of equal type to be used for the operator " + + "needs to have two parameters of type " + + _usingFor.typeName()->annotation().type->canonicalName() + + " and the same data location to be used for the operator " + TokenTraits::friendlyName(*operator_) + "." ); else if ( !TokenTraits::isBinaryOp(*operator_) && TokenTraits::isUnaryOp(*operator_) && - functionType->parameterTypesIncludingSelf().size() != 1 + ( + functionType->parameterTypesIncludingSelf().size() != 1 || + ( + *TypeProvider::withLocationIfReference(DataLocation::Storage, functionType->parameterTypesIncludingSelf().front()) != + *TypeProvider::withLocationIfReference(DataLocation::Storage, _usingFor.typeName()->annotation().type) + ) + ) ) m_errorReporter.typeError( 1147_error, path->location(), "The function \"" + joinHumanReadable(path->path(), ".") + "\" "+ - "needs to have exactly one parameter to be used for the operator " + + "needs to have exactly one parameter of type " + + _usingFor.typeName()->annotation().type->canonicalName() + + " to be used for the operator " + TokenTraits::friendlyName(*operator_) + "." ); + else if ( + ( + functionType->parameterTypesIncludingSelf().size() == 2 && + ( + (*functionType->parameterTypesIncludingSelf().at(0) != *functionType->parameterTypesIncludingSelf().at(1)) || + ( + *TypeProvider::withLocationIfReference(DataLocation::Storage, functionType->parameterTypesIncludingSelf().at(0)) != + *TypeProvider::withLocationIfReference(DataLocation::Storage, _usingFor.typeName()->annotation().type) + ) + ) + ) || + ( + functionType->parameterTypesIncludingSelf().size() == 1 && + ( + *TypeProvider::withLocationIfReference(DataLocation::Storage, functionType->parameterTypesIncludingSelf().at(0)) != + *TypeProvider::withLocationIfReference(DataLocation::Storage, _usingFor.typeName()->annotation().type) + ) + ) + ) + m_errorReporter.typeError( + 7617_error, + path->location(), + "The function \"" + joinHumanReadable(path->path(), ".") + "\" "+ + "needs to have one or two parameters of type " + + _usingFor.typeName()->annotation().type->canonicalName() + + " and the same data location to be used for the operator " + + TokenTraits::friendlyName(*operator_) + + "." + ); else if ( functionType->parameterTypesIncludingSelf().size() != 1 && functionType->parameterTypesIncludingSelf().size() != 2 @@ -3996,7 +4028,9 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor) 8112_error, path->location(), "The function \"" + joinHumanReadable(path->path(), ".") + "\" "+ - "needs to have one or two parameters to be used for the operator " + + "needs to have one or two parameters of type " + + _usingFor.typeName()->annotation().type->canonicalName() + + " and the same data location to be used for the operator " + TokenTraits::friendlyName(*operator_) + "." ); @@ -4018,7 +4052,10 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor) !TokenTraits::isCompareOp(*operator_) && ( functionType->returnParameterTypes().size() != 1 || - *functionType->returnParameterTypes().front() != *functionType->parameterTypesIncludingSelf().front() + ( + *TypeProvider::withLocationIfReference(DataLocation::Storage, functionType->returnParameterTypes().front()) != + *TypeProvider::withLocationIfReference(DataLocation::Storage, _usingFor.typeName()->annotation().type) + ) ) ) m_errorReporter.typeError( @@ -4026,7 +4063,7 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor) path->location(), "The function \"" + joinHumanReadable(path->path(), ".") + "\" "+ "needs to return exactly one value of type " + - functionType->parameterTypesIncludingSelf().front()->canonicalName() + + _usingFor.typeName()->annotation().type->canonicalName() + " to be used for the operator " + TokenTraits::friendlyName(*operator_) + "." diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index b26f3e4aa..e5ce3a825 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -404,8 +404,16 @@ Result Type::userDefinedOperator(Token _token, ASTNod ); solAssert(functionType && !functionType->parameterTypes().empty()); + Type const* normalizedType = this; + if (auto const* referenceType = dynamic_cast(normalizedType)) + normalizedType = TypeProvider::withLocationIfReference(referenceType->location(), normalizedType); + + Type const* normalizedParameterType = functionType->parameterTypes().front(); + if (auto const* referenceType = dynamic_cast(normalizedParameterType)) + normalizedParameterType = TypeProvider::withLocationIfReference(referenceType->location(), normalizedParameterType); + if ( - isImplicitlyConvertibleTo(*functionType->parameterTypes().front()) && + *normalizedType == *normalizedParameterType && ( (_unaryOperation && function.parameterList().parameters().size() == 1) || (!_unaryOperation && function.parameterList().parameters().size() == 2) diff --git a/test/libsolidity/syntaxTests/operators/custom/implicit_conversion_is_blocked.sol b/test/libsolidity/syntaxTests/operators/custom/implicit_conversion_is_blocked.sol new file mode 100644 index 000000000..f6a98e02b --- /dev/null +++ b/test/libsolidity/syntaxTests/operators/custom/implicit_conversion_is_blocked.sol @@ -0,0 +1,34 @@ +using {add as +, unsub as -} for S; + +struct S { + uint x; +} + +function add(S memory _a, S memory) returns (S memory) { + return _a; +} + +function unsub(S memory _a) returns (S memory) { + return _a; +} + +contract C { + S s; + + function test() public { + S memory sTmp; + s + s; + sTmp + true; + true + s; + -sTmp; + -s; + -true; + } +} + +// ---- +// TypeError 2271: (288-293): Operator + not compatible with types struct S storage ref and struct S storage ref. No matching user-defined operator found. +// TypeError 5653: (303-314): User defined operator + not compatible with types struct S memory and bool. +// TypeError 2271: (324-332): Operator + not compatible with types bool and struct S storage ref. +// TypeError 4907: (357-359): Unary operator - cannot be applied to type struct S storage ref. No matching user-defined operator found. +// TypeError 4907: (369-374): Unary operator - cannot be applied to type bool. diff --git a/test/libsolidity/syntaxTests/operators/custom/operator_invalid_parameters.sol b/test/libsolidity/syntaxTests/operators/custom/operator_invalid_parameters.sol index 1a84fc47a..9844a4871 100644 --- a/test/libsolidity/syntaxTests/operators/custom/operator_invalid_parameters.sol +++ b/test/libsolidity/syntaxTests/operators/custom/operator_invalid_parameters.sol @@ -1,7 +1,9 @@ type Int is int256; using { - add as +, sub as -, div as / + add as +, + sub as -, + div as / } for Int; function add(Int) pure returns (Int) { @@ -20,14 +22,12 @@ function f() pure { Int.wrap(0) + Int.wrap(1); Int.wrap(0) - Int.wrap(0); Int.wrap(0) / Int.wrap(0); - Int.wrap(0) * Int.wrap(0); } // ---- -// TypeError 1884: (33-36): The function "add" needs to have two parameters of equal type to be used for the operator +. -// TypeError 8112: (43-46): The function "sub" needs to have one or two parameters to be used for the operator -. -// TypeError 3100: (53-56): The function "div" cannot be bound to the type "Int" because the type cannot be implicitly converted to the first argument of the function ("int256"). -// TypeError 2271: (317-342): Operator + not compatible with types Int and Int. No matching user-defined operator found. -// TypeError 2271: (348-373): Operator - not compatible with types Int and Int. No matching user-defined operator found. -// TypeError 2271: (379-404): Operator / not compatible with types Int and Int. No matching user-defined operator found. -// TypeError 2271: (410-435): Operator * not compatible with types Int and Int. No matching user-defined operator found. +// TypeError 1884: (33-36): The function "add" needs to have two parameters of type Int and the same data location to be used for the operator +. +// TypeError 8112: (47-50): The function "sub" needs to have one or two parameters of type Int and the same data location to be used for the operator -. +// TypeError 7617: (61-64): The function "div" needs to have one or two parameters of type Int and the same data location to be used for the operator /. +// TypeError 2271: (325-350): Operator + not compatible with types Int and Int. No matching user-defined operator found. +// TypeError 2271: (356-381): Operator - not compatible with types Int and Int. No matching user-defined operator found. +// TypeError 2271: (387-412): Operator / not compatible with types Int and Int. No matching user-defined operator found. diff --git a/test/libsolidity/syntaxTests/operators/custom/operator_invalid_return_parameters.sol b/test/libsolidity/syntaxTests/operators/custom/operator_invalid_return_parameters.sol index 7373edba1..b06fcb003 100644 --- a/test/libsolidity/syntaxTests/operators/custom/operator_invalid_return_parameters.sol +++ b/test/libsolidity/syntaxTests/operators/custom/operator_invalid_return_parameters.sol @@ -49,9 +49,3 @@ function f() pure { // TypeError 7743: (77-83): The function "bitnot" needs to return exactly one value of type Int to be used for the operator ~. // TypeError 7995: (94-96): The function "gt" needs to return exactly one value of type bool to be used for the operator >. // TypeError 7995: (107-109): The function "lt" needs to return exactly one value of type bool to be used for the operator <. -// TypeError 3841: (571-596): User defined operator + needs to return value of type Int. -// TypeError 1208: (602-627): User defined operator / needs to return exactly one value. -// TypeError 3138: (633-645): User defined operator - needs to return exactly one value. -// TypeError 7983: (651-663): User defined operator ~ needs to return value of type Int. -// TypeError 1208: (669-694): User defined operator < needs to return exactly one value. -// TypeError 3841: (700-725): User defined operator > needs to return value of type bool. diff --git a/test/libsolidity/syntaxTests/operators/custom/operator_parameters_with_different_data_locations.sol b/test/libsolidity/syntaxTests/operators/custom/operator_parameters_with_different_data_locations.sol index 35a549e55..29a3f1f7f 100644 --- a/test/libsolidity/syntaxTests/operators/custom/operator_parameters_with_different_data_locations.sol +++ b/test/libsolidity/syntaxTests/operators/custom/operator_parameters_with_different_data_locations.sol @@ -7,4 +7,4 @@ function add(S memory, S storage) returns (S memory) { } // ---- -// TypeError 1884: (32-35): The function "add" needs to have two parameters of equal type to be used for the operator +. +// TypeError 1884: (32-35): The function "add" needs to have two parameters of type S and the same data location to be used for the operator +. diff --git a/test/libsolidity/syntaxTests/operators/custom/operator_with_calldata_parameters.sol b/test/libsolidity/syntaxTests/operators/custom/operator_with_calldata_parameters.sol index 811165f31..49ea805e4 100644 --- a/test/libsolidity/syntaxTests/operators/custom/operator_with_calldata_parameters.sol +++ b/test/libsolidity/syntaxTests/operators/custom/operator_with_calldata_parameters.sol @@ -57,14 +57,11 @@ function test(S calldata s) pure { } // ---- -// TypeError 1884: (40-43): The function "mul" needs to have two parameters of equal type to be used for the operator *. +// TypeError 7617: (26-29): The function "sub" needs to have one or two parameters of type S and the same data location to be used for the operator -. +// TypeError 1884: (40-43): The function "mul" needs to have two parameters of type S and the same data location to be used for the operator *. // TypeError 7743: (54-57): The function "div" needs to return exactly one value of type S to be used for the operator /. // TypeError 7743: (68-71): The function "mod" needs to return exactly one value of type S to be used for the operator %. -// TypeError 3100: (82-87): The function "unsub" cannot be bound to the type "struct S storage pointer" because the type cannot be implicitly converted to the first argument of the function ("uint256"). +// TypeError 7617: (82-87): The function "unsub" needs to have one or two parameters of type S and the same data location to be used for the operator -. // TypeError 7743: (98-104): The function "bitnot" needs to return exactly one value of type S to be used for the operator ~. -// TypeError 5653: (747-752): User defined operator - needs to have two parameters of equal type. // TypeError 2271: (758-763): Operator * not compatible with types struct S calldata and struct S calldata. No matching user-defined operator found. -// TypeError 3841: (769-774): User defined operator / needs to return value of type struct S calldata. -// TypeError 1208: (780-785): User defined operator % needs to return exactly one value. // TypeError 4907: (791-793): Unary operator - cannot be applied to type struct S calldata. No matching user-defined operator found. -// TypeError 3138: (799-801): User defined operator ~ needs to return exactly one value. diff --git a/test/libsolidity/syntaxTests/operators/custom/operator_with_different_param_return_types.sol b/test/libsolidity/syntaxTests/operators/custom/operator_with_different_param_return_types.sol index 4f80eb33c..3b0c6e22e 100644 --- a/test/libsolidity/syntaxTests/operators/custom/operator_with_different_param_return_types.sol +++ b/test/libsolidity/syntaxTests/operators/custom/operator_with_different_param_return_types.sol @@ -35,7 +35,7 @@ function bitor(S storage, S storage) pure returns (S memory) { // ---- -// TypeError 3100: (71-74): The function "sub" cannot be bound to the type "Int" because the type cannot be implicitly converted to the first argument of the function ("int128"). -// TypeError 3100: (85-88): The function "mul" cannot be bound to the type "Int" because the type cannot be implicitly converted to the first argument of the function ("int128"). +// TypeError 7617: (71-74): The function "sub" needs to have one or two parameters of type Int and the same data location to be used for the operator -. +// TypeError 7743: (71-74): The function "sub" needs to return exactly one value of type Int to be used for the operator -. +// TypeError 1884: (85-88): The function "mul" needs to have two parameters of type Int and the same data location to be used for the operator *. // TypeError 7743: (95-98): The function "div" needs to return exactly one value of type Int to be used for the operator /. -// TypeError 7743: (128-133): The function "bitor" needs to return exactly one value of type S to be used for the operator |. diff --git a/test/libsolidity/syntaxTests/operators/custom/operator_with_storage_parameters.sol b/test/libsolidity/syntaxTests/operators/custom/operator_with_storage_parameters.sol index c7d6b4616..8767f8105 100644 --- a/test/libsolidity/syntaxTests/operators/custom/operator_with_storage_parameters.sol +++ b/test/libsolidity/syntaxTests/operators/custom/operator_with_storage_parameters.sol @@ -53,15 +53,11 @@ contract C { } } // ---- -// TypeError 1884: (40-43): The function "mul" needs to have two parameters of equal type to be used for the operator *. +// TypeError 7617: (26-29): The function "sub" needs to have one or two parameters of type S and the same data location to be used for the operator -. +// TypeError 1884: (40-43): The function "mul" needs to have two parameters of type S and the same data location to be used for the operator *. // TypeError 7743: (54-57): The function "div" needs to return exactly one value of type S to be used for the operator /. // TypeError 7743: (68-71): The function "mod" needs to return exactly one value of type S to be used for the operator %. // TypeError 7743: (82-87): The function "unsub" needs to return exactly one value of type S to be used for the operator -. -// TypeError 1147: (98-104): The function "bitnot" needs to have exactly one parameter to be used for the operator ~. -// TypeError 5653: (707-712): User defined operator - needs to have two parameters of equal type. -// TypeError 3841: (707-712): User defined operator - needs to return value of type struct S storage ref. +// TypeError 1147: (98-104): The function "bitnot" needs to have exactly one parameter of type S to be used for the operator ~. // TypeError 2271: (722-727): Operator * not compatible with types struct S storage ref and struct S storage ref. No matching user-defined operator found. -// TypeError 3841: (737-742): User defined operator / needs to return value of type struct S storage pointer. -// TypeError 1208: (752-757): User defined operator % needs to return exactly one value. -// TypeError 3138: (767-769): User defined operator - needs to return exactly one value. // TypeError 4907: (779-781): Unary operator ~ cannot be applied to type struct S storage ref. No matching user-defined operator found. diff --git a/test/libsolidity/syntaxTests/operators/custom/unary_operator_attached_to_two_argument_function.sol b/test/libsolidity/syntaxTests/operators/custom/unary_operator_attached_to_two_argument_function.sol index beb04264c..5acebcf72 100644 --- a/test/libsolidity/syntaxTests/operators/custom/unary_operator_attached_to_two_argument_function.sol +++ b/test/libsolidity/syntaxTests/operators/custom/unary_operator_attached_to_two_argument_function.sol @@ -14,5 +14,5 @@ contract C { } // ---- -// TypeError 1147: (32-38): The function "bitnot" needs to have exactly one parameter to be used for the operator ~. +// TypeError 1147: (32-38): The function "bitnot" needs to have exactly one parameter of type Int to be used for the operator ~. // TypeError 4907: (186-198): Unary operator ~ cannot be applied to type Int. No matching user-defined operator found.