From 9e1ba093d5334c3f63530b79f236446f68e06357 Mon Sep 17 00:00:00 2001 From: wechman Date: Tue, 2 Aug 2022 08:25:01 +0200 Subject: [PATCH] Minor changes after code review --- libsolidity/ast/AST.h | 23 +++++++++-------- libsolidity/ast/ASTJsonExporter.cpp | 6 ++--- libsolidity/ast/AST_accept.h | 4 +-- libsolidity/ast/Types.cpp | 25 ++++++++++--------- .../operators/custom/all_operators.sol | 2 -- .../operators/custom/fixedpoint.sol | 2 -- .../custom/operator_bound_for_two_types.sol | 2 -- .../operator_bound_inside_contracts.sol | 2 -- ...ound_to_function_imported_transitively.sol | 7 +----- ...d_operator_local_storage_variable_err.sol} | 0 ...r_operator_local_storage_variable_err.sol} | 0 ...ol => operator_not_user_implementable.sol} | 0 12 files changed, 32 insertions(+), 41 deletions(-) rename test/libsolidity/syntaxTests/controlFlow/localStorageVariables/{and_operator_err.sol => custom_bitand_operator_local_storage_variable_err.sol} (100%) rename test/libsolidity/syntaxTests/controlFlow/localStorageVariables/{or_operator_err.sol => custom_bitor_operator_local_storage_variable_err.sol} (100%) rename test/libsolidity/syntaxTests/operators/custom/{operator_not_user_implemented.sol => operator_not_user_implementable.sol} (100%) diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index c6d4de92a..2911a0a0c 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -671,13 +671,13 @@ public: bool _global ): ASTNode(_id, _location), - m_functions(std::move(_functions)), + m_functionsOrLibrary(std::move(_functions)), m_operators(std::move(_operators)), m_usesBraces(_usesBraces), m_typeName(std::move(_typeName)), m_global{_global} { - solAssert(m_functions.size() == m_operators.size()); + solAssert(m_functionsOrLibrary.size() == m_operators.size()); } void accept(ASTVisitor& _visitor) override; @@ -687,19 +687,22 @@ public: TypeName const* typeName() const { return m_typeName.get(); } /// @returns a list of functions or the single library. - std::vector> const& functionsOrLibrary() const { return m_functions; } - auto functionsAndOperators() const { return ranges::zip_view(m_functions, m_operators); } + std::vector> const& functionsOrLibrary() const { return m_functionsOrLibrary; } + auto functionsAndOperators() const { return ranges::zip_view(m_functionsOrLibrary, m_operators); } bool usesBraces() const { return m_usesBraces; } bool global() const { return m_global; } private: /// Either the single library or a list of functions. - std::vector> m_functions; - /// Operators, the functions are applied to. - std::vector> m_operators; - bool m_usesBraces; - ASTPointer m_typeName; - bool m_global = false; + std::vector> const m_functionsOrLibrary; + /// Operators, the functions from @a m_functionsOrLibrary implement. + /// A token if the corresponding element in m_functionsOrLibrary + /// defines an operator, nullptr otherwise. + /// Note that this vector size must be equal to m_functionsOrLibrary size. + std::vector> const m_operators; + bool const m_usesBraces; + ASTPointer const m_typeName; + bool const m_global; }; class StructDefinition: public Declaration, public ScopeOpener diff --git a/libsolidity/ast/ASTJsonExporter.cpp b/libsolidity/ast/ASTJsonExporter.cpp index f2ac33325..f2b6ff48e 100644 --- a/libsolidity/ast/ASTJsonExporter.cpp +++ b/libsolidity/ast/ASTJsonExporter.cpp @@ -337,7 +337,7 @@ bool ASTJsonExporter::visit(UsingForDirective const& _node) { Json::Value functionNode; functionNode["function"] = toJson(*function); - if (op) + if (op.has_value()) functionNode["operator"] = string(TokenTraits::toString(*op)); functionList.append(move(functionNode)); } @@ -828,8 +828,8 @@ bool ASTJsonExporter::visit(UnaryOperation const& _node) make_pair("operator", TokenTraits::toString(_node.getOperator())), make_pair("subExpression", toJson(_node.subExpression())) }; - if (FunctionDefinition const* function = _node.annotation().userDefinedFunction) - attributes.emplace_back("function", nodeId(*function)); + if (FunctionDefinition const* referencedDeclaration = _node.annotation().userDefinedFunction) + attributes.emplace_back("function", nodeId(*referencedDeclaration)); appendExpressionAttributes(attributes, _node.annotation()); setJsonNode(_node, "UnaryOperation", std::move(attributes)); return false; diff --git a/libsolidity/ast/AST_accept.h b/libsolidity/ast/AST_accept.h index 64d785e05..78dc0e5dd 100644 --- a/libsolidity/ast/AST_accept.h +++ b/libsolidity/ast/AST_accept.h @@ -194,7 +194,7 @@ void UsingForDirective::accept(ASTVisitor& _visitor) { if (_visitor.visit(*this)) { - listAccept(m_functions, _visitor); + listAccept(m_functionsOrLibrary, _visitor); if (m_typeName) m_typeName->accept(_visitor); } @@ -205,7 +205,7 @@ void UsingForDirective::accept(ASTConstVisitor& _visitor) const { if (_visitor.visit(*this)) { - listAccept(m_functions, _visitor); + listAccept(m_functionsOrLibrary, _visitor); if (m_typeName) m_typeName->accept(_visitor); } diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 4841240c0..6bf311bab 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -369,10 +369,9 @@ vector usingForDirectivesForType(Type const& _type, AS typeLocation = refType->location(); return usingForDirectives | ranges::views::filter([&](UsingForDirective const* _directive) -> bool { - // Convert both types to pointers for comparison to see if the `using for` - // directive applies. - // Further down, we check more detailed for each function if `_type` is - // convertible to the function parameter type. + // Convert both types to pointers for comparison to see if the `using for` directive applies. + // Note that at this point we don't yet know if the functions are actually usable with the type. + // `_type` may not be convertible to the function parameter type. return !_directive->typeName() || *TypeProvider::withLocationIfReference(typeLocation, &_type, true) == @@ -391,14 +390,14 @@ Result Type::userDefinedOperator(Token _token, ASTNod if (!typeDefinition()) return nullptr; - set seenFunctions; + set matchingDefinitions; for (UsingForDirective const* ufd: usingForDirectivesForType(*this, _scope)) - for (auto const& [pathPointer, operator_]: ufd->functionsAndOperators()) + for (auto const& [identifierPath, operator_]: ufd->functionsAndOperators()) { if (operator_ != _token) continue; FunctionDefinition const& function = dynamic_cast( - *pathPointer->annotation().referencedDeclaration + *identifierPath->annotation().referencedDeclaration ); FunctionType const* functionType = dynamic_cast( function.libraryFunction() ? function.typeViaContractName() : function.type() @@ -418,12 +417,12 @@ Result Type::userDefinedOperator(Token _token, ASTNod (!_unaryOperation && function.parameterList().parameters().size() == 2) ) ) - seenFunctions.insert(&function); + matchingDefinitions.insert(&function); } - if (seenFunctions.size() == 1) - return *seenFunctions.begin(); - else if (seenFunctions.size() == 0) + if (matchingDefinitions.size() == 1) + return *matchingDefinitions.begin(); + else if (matchingDefinitions.size() == 0) return Result::err("No matching user-defined operator found."); else return Result::err("Multiple user-defined functions provided for this operator."); @@ -453,7 +452,9 @@ MemberList::MemberMap Type::boundFunctions(Type const& _type, ASTNode const& _sc for (UsingForDirective const* ufd: usingForDirectivesForType(_type, _scope)) for (auto const& [pathPointer, operator_]: ufd->functionsAndOperators()) { - if (operator_) + if (operator_.has_value()) + // Functions used to define operators are not bound to the type. + // I.e. `using {f} for T` allows `T x; x.f()` but `using {f as +} for T` does not. continue; solAssert(pathPointer); diff --git a/test/libsolidity/semanticTests/operators/custom/all_operators.sol b/test/libsolidity/semanticTests/operators/custom/all_operators.sol index ea651773f..b6aff5447 100644 --- a/test/libsolidity/semanticTests/operators/custom/all_operators.sol +++ b/test/libsolidity/semanticTests/operators/custom/all_operators.sol @@ -79,8 +79,6 @@ contract C { function test_geq(int128 x) public pure returns (bool) { return w(x) >= w(2); } } -// ==== -// compileViaYul: also // ---- // test_bitor() -> 10 // test_bitand() -> 10 diff --git a/test/libsolidity/semanticTests/operators/custom/fixedpoint.sol b/test/libsolidity/semanticTests/operators/custom/fixedpoint.sol index fa119abf2..11f2a5e47 100644 --- a/test/libsolidity/semanticTests/operators/custom/fixedpoint.sol +++ b/test/libsolidity/semanticTests/operators/custom/fixedpoint.sol @@ -18,7 +18,5 @@ contract C { return value + value * percentage; } } -// ==== -// compileViaYul: also // ---- // applyInterest(int128,int128): 500000000000000000000, 100000000000000000 -> 550000000000000000000 diff --git a/test/libsolidity/semanticTests/operators/custom/operator_bound_for_two_types.sol b/test/libsolidity/semanticTests/operators/custom/operator_bound_for_two_types.sol index f933ceb08..40a954481 100644 --- a/test/libsolidity/semanticTests/operators/custom/operator_bound_for_two_types.sol +++ b/test/libsolidity/semanticTests/operators/custom/operator_bound_for_two_types.sol @@ -22,8 +22,6 @@ contract C { } } -// ==== -// compileViaYul: also // ---- // f() -> 1 // g() -> 2 diff --git a/test/libsolidity/semanticTests/operators/custom/operator_bound_inside_contracts.sol b/test/libsolidity/semanticTests/operators/custom/operator_bound_inside_contracts.sol index 3c32d0a4a..b0cb99806 100644 --- a/test/libsolidity/semanticTests/operators/custom/operator_bound_inside_contracts.sol +++ b/test/libsolidity/semanticTests/operators/custom/operator_bound_inside_contracts.sol @@ -36,8 +36,6 @@ contract C { } } -// ==== -// compileViaYul: also // ---- // test1() -> 1 // test2() -> 2 diff --git a/test/libsolidity/semanticTests/operators/custom/operator_bound_to_function_imported_transitively.sol b/test/libsolidity/semanticTests/operators/custom/operator_bound_to_function_imported_transitively.sol index e4c3f5b22..725583eb9 100644 --- a/test/libsolidity/semanticTests/operators/custom/operator_bound_to_function_imported_transitively.sol +++ b/test/libsolidity/semanticTests/operators/custom/operator_bound_to_function_imported_transitively.sol @@ -1,5 +1,4 @@ ==== Source: a.sol ==== -pragma abicoder v2; library L { type Int is int128; @@ -8,21 +7,17 @@ library L { } } ==== Source: b.sol ==== -pragma abicoder v2; import "a.sol" as a; ==== Source: c.sol ==== -pragma abicoder v2; import "b.sol" as b; contract C { using {b.a.L.add as +} for b.a.L.Int; function f() pure public returns (b.a.L.Int) { - return b.a.L.Int.wrap(0) + b.a.L.Int.wrap(0); + return b.a.L.Int.wrap(0) + b.a.L.Int.wrap(0); } } -// ==== -// compileViaYul: also // ---- // f() -> 7 diff --git a/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/and_operator_err.sol b/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/custom_bitand_operator_local_storage_variable_err.sol similarity index 100% rename from test/libsolidity/syntaxTests/controlFlow/localStorageVariables/and_operator_err.sol rename to test/libsolidity/syntaxTests/controlFlow/localStorageVariables/custom_bitand_operator_local_storage_variable_err.sol diff --git a/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/or_operator_err.sol b/test/libsolidity/syntaxTests/controlFlow/localStorageVariables/custom_bitor_operator_local_storage_variable_err.sol similarity index 100% rename from test/libsolidity/syntaxTests/controlFlow/localStorageVariables/or_operator_err.sol rename to test/libsolidity/syntaxTests/controlFlow/localStorageVariables/custom_bitor_operator_local_storage_variable_err.sol diff --git a/test/libsolidity/syntaxTests/operators/custom/operator_not_user_implemented.sol b/test/libsolidity/syntaxTests/operators/custom/operator_not_user_implementable.sol similarity index 100% rename from test/libsolidity/syntaxTests/operators/custom/operator_not_user_implemented.sol rename to test/libsolidity/syntaxTests/operators/custom/operator_not_user_implementable.sol