Update code to review comments

libsolidity/analysis/TypeChecker.cpp

@@ -1790,7 +1790,7 @@ void TypeChecker::endVisit(BinaryOperation const& _operation)
 	_operation.annotation().isConstant = false;
 
 	// Check if the operator is built-in or user-defined.
-	FunctionDefinitionResult userDefinedOperatorResult = leftType->userDefinedOperator(
+	Result<FunctionDefinition const*> userDefinedOperatorResult = leftType->userDefinedOperator(
 		_operation.getOperator(),
 		*currentDefinitionScope(),
 		false // _unaryOperation

libsolidity/ast/Types.cpp

@@ -59,6 +59,8 @@ using namespace solidity;
 using namespace solidity::langutil;
 using namespace solidity::frontend;
 
+using util::Result;
+
 namespace
 {
 
@@ -384,7 +386,7 @@ vector<UsingForDirective const*> usingForDirectivesForType(Type const& _type, AS
 
 }
 
-FunctionDefinitionResult Type::userDefinedOperator(Token _token, ASTNode const& _scope, bool _unaryOperation) const
+Result<FunctionDefinition const*> Type::userDefinedOperator(Token _token, ASTNode const& _scope, bool _unaryOperation) const
 {
 	if (!typeDefinition())
 		return nullptr;
@@ -412,12 +414,10 @@ FunctionDefinitionResult Type::userDefinedOperator(Token _token, ASTNode const&
 
 	if (seenFunctions.size() == 1)
 		return *seenFunctions.begin();
-	else if (!!typeDefinition() && seenFunctions.size() == 0)
-		return FunctionDefinitionResult::err("Operator has not been user-defined.");
-	else if (!!typeDefinition())
-		return FunctionDefinitionResult::err("Multiple user-defined functions provided for this operator.");
+	else if (seenFunctions.size() == 0)
+		return Result<FunctionDefinition const*>::err("No matching user-defined operator found.");
 	else
-		return nullptr;
+		return Result<FunctionDefinition const*>::err("Multiple user-defined functions provided for this operator.");
 }
 
 MemberList::MemberMap Type::boundFunctions(Type const& _type, ASTNode const& _scope)

libsolidity/ast/Types.h

@@ -54,7 +54,6 @@ using TypePointers = std::vector<Type const*>;
 using rational = boost::rational<bigint>;
 using TypeResult = util::Result<Type const*>;
 using BoolResult = util::Result<bool>;
-using FunctionDefinitionResult = util::Result<FunctionDefinition const*>;
 
 }
 
@@ -378,7 +377,7 @@ public:
 	/// Clears all internally cached values (if any).
 	virtual void clearCache() const;
 
-	FunctionDefinitionResult userDefinedOperator(Token _token, ASTNode const& _scope, bool _unaryOperation) const;
+	util::Result<FunctionDefinition const*> userDefinedOperator(Token _token, ASTNode const& _scope, bool _unaryOperation) const;
 
 private:
 	/// @returns a member list containing all members added to this type by `using for` directives.

libsolidity/codegen/ExpressionCompiler.cpp

@@ -416,7 +416,8 @@ bool ExpressionCompiler::visit(UnaryOperation const& _unaryOperation)
 		solAssert(function->isFree(), "Only free functions can be bound to a user type operator.");
 
 		FunctionType const* functionType = dynamic_cast<FunctionType const*>(
-			function->libraryFunction() ? function->typeViaContractName() : function->type());
+			function->libraryFunction() ? function->typeViaContractName() : function->type()
+		);
 		solAssert(functionType);
 
 		functionType = dynamic_cast<FunctionType const&>(*functionType).asBoundFunction();

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -694,21 +694,22 @@ bool IRGeneratorForStatements::visit(UnaryOperation const& _unaryOperation)
 			"Functions with parameters other than self parameter cannot be bound to a user type unary operator."
 		);
 
-		string parameter = expressionAsType(_unaryOperation.subExpression(), *functionType->selfType());
-		solAssert(!parameter.empty());
+		string argument = expressionAsType(_unaryOperation.subExpression(), *functionType->selfType());
+		solAssert(!argument.empty());
 		solAssert(function->isImplemented(), "");
 
 		solAssert(
 			function->returnParameters().size() == 1,
 			"A function bound to the user type operator is supposed to return exactly one value."
 		);
-		solAssert(*_unaryOperation.annotation().type == *function->returnParameters().at(0)->type(),
+		solAssert(
+			*_unaryOperation.annotation().type == *function->returnParameters().at(0)->type(),
 			"A return type of the bound function is supposed to be same as a operator type."
 		);
 
 		define(_unaryOperation) <<
 			m_context.enqueueFunctionForCodeGeneration(*function) <<
-			("(" + parameter + ")\n");
+			("(" + argument + ")\n");
 
 		return false;
 	}

test/libsolidity/syntaxTests/controlFlow/localStorageVariables/and_operator_err.sol

@@ -1,11 +1,22 @@
-struct S { bool f; }
+struct S {
+    uint v;
+}
+
+using {bitand as &} for S;
+
+function bitand(S storage, S storage) returns (S storage) {
+    S storage rTmp;
+    return rTmp;
+}
 
 contract C {
-    function f() public view {
-        S storage s;
-        s.f && true;
+    S s;
+    function f() public {
+        S storage sTmp;
+        sTmp & s;
     }
 }
 
 // ----
-// TypeError 3464: (95-96): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.
+// TypeError 3464: (145-149): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.
+// TypeError 3464: (234-238): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.

test/libsolidity/syntaxTests/controlFlow/localStorageVariables/or_operator_err.sol

@@ -1,11 +1,22 @@
-struct S { bool f; }
+struct S {
+    uint v;
+}
+
+using {bitor as |} for S;
+
+function bitor(S storage, S storage) returns (S storage) {
+    S storage rTmp;
+    return rTmp;
+}
 
 contract C {
-    function f() public view {
-        S storage s;
-        s.f || true;
+    S s;
+    function f() public {
+        S storage sTmp;
+        sTmp | s;
     }
 }
 
 // ----
-// TypeError 3464: (95-96): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.
+// TypeError 3464: (143-147): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.
+// TypeError 3464: (232-236): This variable is of storage pointer type and can be accessed without prior assignment, which would lead to undefined behaviour.

test/libsolidity/syntaxTests/nameAndTypeResolution/203_struct_reference_compare_operators.sol

@@ -7,4 +7,4 @@ contract test {
   }
 }
 // ----
-// TypeError 2271: (79-85): Operator == not compatible with types struct test.s storage ref and struct test.s storage ref. Operator has not been user-defined.
+// TypeError 2271: (79-85): Operator == not compatible with types struct test.s storage ref and struct test.s storage ref. No matching user-defined operator found.

test/libsolidity/syntaxTests/operators/custom/operator_with_different_param_return_types.sol

@@ -2,7 +2,7 @@ type Int is int128;
 using {
     add as +, sub as -,
     mul as *, div as /
-     } for Int;
+} for Int;
 
 function add(Int, Int) pure returns (Int) {
     return Int.wrap(1);

test/libsolidity/syntaxTests/operators/custom/operator_wrong_number_params_return.sol

@@ -4,7 +4,7 @@ using {
     add as +, sub as -,
     mul as *, div as /,
     gt as >, lt as <
-     } for Int;
+} for Int;
 
 function add(Int) pure returns (Int) {
     return Int.wrap(0);