Merge pull request #7239 from ethereum/externalFunctionToAddress

[0.6.0] Replace casting of external functions to address by a member named "address".

Changelog.md

@@ -1,6 +1,7 @@
 ### 0.6.0 (unreleased)
 
 Breaking changes:
+ * General: Disallow explicit conversions from external function types to ``address`` and add a member called ``address`` to them as replacement.
 
 
 Language Features:

docs/060-breaking-changes.rst

@@ -9,6 +9,14 @@ For the full list check
 `the release changelog <https://github.com/ethereum/solidity/releases/tag/v0.6.0>`_.
 
 
+Syntactic Only Changes
+======================
+
+This section lists purely syntactic changes that do not affect the behavior of existing code.
+
+* Conversions from external function types to ``address`` are now disallowed. Instead external
+  function types have a member called ``address``, similar to the existing ``selector`` member.
+
 Semantic Only Changes
 =====================
 
@@ -27,6 +35,7 @@ How to update your code
 
 This section gives detailed instructions on how to update prior code for every breaking change.
 
+* Change ``address(f)`` to ``f.address`` for ``f`` being of external function type.
 
 Deprecated Elements
 ===================

docs/types/value-types.rst

@@ -582,9 +582,6 @@ do not have a default.
 
 Conversions:
 
-A value of external function type can be explicitly converted to ``address``
-resulting in the address of the contract of the function.
-
 A function type ``A`` is implicitly convertible to a function type ``B`` if and only if
 their parameter types are identical, their return types are identical,
 their internal/external property is identical and the state mutability of ``A``
@@ -616,8 +613,9 @@ just use ``f``, if you want to use its external form, use ``this.f``.
 
 Members:
 
-Public (or external) functions have the following members:
+External (or public) functions have the following members:
 
+* ``.address`` returns the address of the contract of the function.
 * ``.selector`` returns the :ref:`ABI function selector <abi_function_selector>`
 * ``.gas(uint)`` returns a callable function object which, when called, will send the specified amount of gas to the target function. See :ref:`External Function Calls <external-function-calls>` for more information.
 * ``.value(uint)`` returns a callable function object which, when called, will send the specified amount of wei to the target function. See :ref:`External Function Calls <external-function-calls>` for more information.
@@ -629,6 +627,7 @@ Example that shows how to use the members::
 
     contract Example {
         function f() public payable returns (bytes4) {
+            assert(this.f.address == address(this));
             return this.f.selector;
         }
 

libsolidity/analysis/TypeChecker.cpp

@@ -1478,12 +1478,28 @@ TypePointer TypeChecker::typeCheckTypeConversionAndRetrieveReturnType(
 							variableDeclaration->location()
 						);
 				m_errorReporter.typeError(
-					_functionCall.location(), ssl,
+					_functionCall.location(),
+					ssl,
 					"Explicit type conversion not allowed from non-payable \"address\" to \"" +
 					resultType->toString() +
 					"\", which has a payable fallback function."
 				);
 			}
+			else if (
+				auto const* functionType = dynamic_cast<FunctionType const*>(argType);
+				functionType &&
+				functionType->kind() == FunctionType::Kind::External &&
+				resultType->category() == Type::Category::Address
+			)
+				m_errorReporter.typeError(
+					_functionCall.location(),
+					"Explicit type conversion not allowed from \"" +
+					argType->toString() +
+					"\" to \"" +
+					resultType->toString() +
+					"\". To obtain the address of the contract of the function, " +
+					"you can use the .address member of the function."
+				);
 			else
 				m_errorReporter.typeError(
 					_functionCall.location(),

libsolidity/ast/Types.cpp

@@ -2726,8 +2726,6 @@ bool FunctionType::operator==(Type const& _other) const
 
 BoolResult FunctionType::isExplicitlyConvertibleTo(Type const& _convertTo) const
 {
-	if (m_kind == Kind::External && _convertTo == *TypeProvider::address())
-		return true;
 	return _convertTo.category() == category();
 }
 
@@ -2922,7 +2920,10 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con
 	{
 		MemberList::MemberMap members;
 		if (m_kind == Kind::External)
+		{
 			members.emplace_back("selector", TypeProvider::fixedBytes(4));
+			members.emplace_back("address", TypeProvider::address());
+		}
 		if (m_kind != Kind::BareDelegateCall)
 		{
 			if (isPayable())

libsolidity/codegen/CompilerUtils.cpp

@@ -1112,16 +1112,8 @@ void CompilerUtils::convertType(
 			m_context << Instruction::ISZERO << Instruction::ISZERO;
 		break;
 	default:
-		if (stackTypeCategory == Type::Category::Function && targetTypeCategory == Type::Category::Address)
+		// we used to allow conversions from function to address
-		{
+		solAssert(!(stackTypeCategory == Type::Category::Function && targetTypeCategory == Type::Category::Address), "");
-			FunctionType const& typeOnStack = dynamic_cast<FunctionType const&>(_typeOnStack);
-			solAssert(typeOnStack.kind() == FunctionType::Kind::External, "Only external function type can be converted.");
-
-			// stack: <address> <function_id>
-			m_context << Instruction::POP;
-		}
-		else
-		{
 		if (stackTypeCategory == Type::Category::Function && targetTypeCategory == Type::Category::Function)
 		{
 			FunctionType const& typeOnStack = dynamic_cast<FunctionType const&>(_typeOnStack);
@@ -1142,7 +1134,6 @@ void CompilerUtils::convertType(
 			m_context
 				<< ((u256(1) << (8 * _targetType.storageBytes())) - 1)
 				<< Instruction::AND;
-		}
 		break;
 	}
 

libsolidity/codegen/ExpressionCompiler.cpp

@@ -1338,6 +1338,13 @@ bool ExpressionCompiler::visit(MemberAccess const& _memberAccess)
 			/// need to store it as bytes4
 			utils().leftShiftNumberOnStack(224);
 		}
+		else if (member == "address")
+		{
+			auto const& functionType = dynamic_cast<FunctionType const&>(*_memberAccess.expression().annotation().type);
+			solAssert(functionType.kind() == FunctionType::Kind::External, "");
+			// stack: <address> <function_id>
+			m_context << Instruction::POP;
+		}
 		else
 			solAssert(
 				!!_memberAccess.expression().annotation().type->memberType(member),

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -718,6 +718,10 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 		{
 			solUnimplementedAssert(false, "");
 		}
+		else if (member == "address")
+		{
+			solUnimplementedAssert(false, "");
+		}
 		else
 			solAssert(
 				!!_memberAccess.expression().annotation().type->memberType(member),

libsolidity/parsing/Parser.cpp

@@ -1552,6 +1552,12 @@ ASTPointer<Expression> Parser::parseLeftHandSideExpression(
 		{
 			m_scanner->next();
 			nodeFactory.markEndPosition();
+			if (m_scanner->currentToken() == Token::Address)
+			{
+				expression = nodeFactory.createNode<MemberAccess>(expression, make_shared<ASTString>("address"));
+				m_scanner->next();
+			}
+			else
 				expression = nodeFactory.createNode<MemberAccess>(expression, expectIdentifierToken());
 			break;
 		}

test/libsolidity/SolidityEndToEndTest.cpp

@@ -11201,25 +11201,6 @@ BOOST_AUTO_TEST_CASE(function_array_cross_calls)
 	ABI_CHECK(callContractFunction("test()"), encodeArgs(u256(5), u256(6), u256(7)));
 }
 
-BOOST_AUTO_TEST_CASE(external_function_to_address)
-{
-	char const* sourceCode = R"(
-		contract C {
-			function f() public returns (bool) {
-				return address(this.f) == address(this);
-			}
-			function g(function() external cb) public returns (address) {
-				return address(cb);
-			}
-		}
-	)";
-
-	compileAndRun(sourceCode, 0, "C");
-	ABI_CHECK(callContractFunction("f()"), encodeArgs(true));
-	ABI_CHECK(callContractFunction("g(function)", fromHex("00000000000000000000000000000000000004226121ff00000000000000000")), encodeArgs(u160(0x42)));
-}
-
-
 BOOST_AUTO_TEST_CASE(copy_internal_function_array_to_storage)
 {
 	char const* sourceCode = R"(

test/libsolidity/semanticTests/types/external_function_to_address.sol

@@ -0,0 +1,11 @@
+contract C {
+    function f() public returns (bool) {
+        return this.f.address == address(this);
+    }
+    function g(function() external cb) public returns (address) {
+        return cb.address;
+    }
+}
+// ----
+// f() -> true
+// g(function): hex"00000000000000000000000000000000000004226121ff00000000000000000" -> 0x42

test/libsolidity/syntaxTests/functionTypes/conversion_to_address.sol

@@ -0,0 +1,7 @@
+contract C {
+    function f() public view returns (address) {
+        return address(this.f);
+    }
+}
+// ----
+// TypeError: (77-92): Explicit type conversion not allowed from "function () view external returns (address)" to "address". To obtain the address of the contract of the function, you can use the .address member of the function.

test/libsolidity/syntaxTests/functionTypes/external_function_type_to_address.sol

@@ -1,5 +1,5 @@
 contract C {
     function f() public view returns (address) {
-        return address(this.f);
+        return this.f.address;
     }
 }

test/libsolidity/syntaxTests/functionTypes/external_function_type_to_address_payable.sol

@@ -1,7 +1,7 @@
 contract C {
     function f() public view returns (address payable) {
-        return address(this.f);
+        return this.f.address;
     }
 }
 // ----
-// TypeError: (85-100): Return argument type address is not implicitly convertible to expected type (type of first return variable) address payable.
+// TypeError: (85-99): Return argument type address is not implicitly convertible to expected type (type of first return variable) address payable.

test/libsolidity/syntaxTests/structs/address_member_access.sol

@@ -0,0 +1,9 @@
+contract C {
+    struct S { uint a; }
+    function f() public pure returns(address) {
+        S memory s = S(42);
+        return s.address;
+    }
+}
+// ----
+// TypeError: (129-138): Member "address" not found or not visible after argument-dependent lookup in struct C.S memory.

test/libsolidity/syntaxTests/structs/address_member_declaration.sol

@@ -0,0 +1,5 @@
+contract C {
+    struct S { uint address; }
+}
+// ----
+// ParserError: (33-40): Expected identifier but got 'address'