Minor changes after code review

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<ASTPointer<IdentifierPath>> const& functionsOrLibrary() const { return m_functions; }
+	std::vector<ASTPointer<IdentifierPath>> const& functionsOrLibrary() const { return m_functionsOrLibrary; }
-	auto functionsAndOperators() const { return ranges::zip_view(m_functions, m_operators); }
+	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<ASTPointer<IdentifierPath>> m_functions;
+	std::vector<ASTPointer<IdentifierPath>> const m_functionsOrLibrary;
-	/// Operators, the functions are applied to.
+	/// Operators, the functions from @a m_functionsOrLibrary implement.
-	std::vector<std::optional<Token>> m_operators;
+	/// A token if the corresponding element in m_functionsOrLibrary
-	bool m_usesBraces;
+	/// defines an operator, nullptr otherwise.
-	ASTPointer<TypeName> m_typeName;
+	/// Note that this vector size must be equal to m_functionsOrLibrary size.
-	bool m_global = false;
+	std::vector<std::optional<Token>> const m_operators;
+	bool const m_usesBraces;
+	ASTPointer<TypeName> const m_typeName;
+	bool const m_global;
 };
 
 class StructDefinition: public Declaration, public ScopeOpener

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)
+	if (FunctionDefinition const* referencedDeclaration = _node.annotation().userDefinedFunction)
-		attributes.emplace_back("function", nodeId(*function));
+		attributes.emplace_back("function", nodeId(*referencedDeclaration));
 	appendExpressionAttributes(attributes, _node.annotation());
 	setJsonNode(_node, "UnaryOperation", std::move(attributes));
 	return false;

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);
 	}

libsolidity/ast/Types.cpp

@@ -369,10 +369,9 @@ vector<UsingForDirective const*> 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`
+		// Convert both types to pointers for comparison to see if the `using for` directive applies.
-		// directive applies.
+		// Note that at this point we don't yet know if the functions are actually usable with the type.
-		// Further down, we check more detailed for each function if `_type` is
+		// `_type` may not be convertible to the function parameter type.
-		// convertible to the function parameter type.
 		return
 			!_directive->typeName() ||
 			*TypeProvider::withLocationIfReference(typeLocation, &_type, true) ==
@@ -391,14 +390,14 @@ Result<FunctionDefinition const*> Type::userDefinedOperator(Token _token, ASTNod
 	if (!typeDefinition())
 		return nullptr;
 
-	set<FunctionDefinition const*> seenFunctions;
+	set<FunctionDefinition const*> 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<FunctionDefinition const&>(
-				*pathPointer->annotation().referencedDeclaration
+				*identifierPath->annotation().referencedDeclaration
 			);
 			FunctionType const* functionType = dynamic_cast<FunctionType const*>(
 				function.libraryFunction() ? function.typeViaContractName() : function.type()
@@ -418,12 +417,12 @@ Result<FunctionDefinition const*> Type::userDefinedOperator(Token _token, ASTNod
 					(!_unaryOperation && function.parameterList().parameters().size() == 2)
 				)
 			)
-				seenFunctions.insert(&function);
+				matchingDefinitions.insert(&function);
 		}
 
-	if (seenFunctions.size() == 1)
+	if (matchingDefinitions.size() == 1)
-		return *seenFunctions.begin();
+		return *matchingDefinitions.begin();
-	else if (seenFunctions.size() == 0)
+	else if (matchingDefinitions.size() == 0)
 		return Result<FunctionDefinition const*>::err("No matching user-defined operator found.");
 	else
 		return Result<FunctionDefinition const*>::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);

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

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

test/libsolidity/semanticTests/operators/custom/operator_bound_for_two_types.sol

@@ -22,8 +22,6 @@ contract C {
     }
 }
 
-// ====
-// compileViaYul: also
 // ----
 // f() -> 1
 // g() -> 2

test/libsolidity/semanticTests/operators/custom/operator_bound_inside_contracts.sol

@@ -36,8 +36,6 @@ contract C {
     }
 }
 
-// ====
-// compileViaYul: also
 // ----
 // test1() -> 1
 // test2() -> 2

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