Merge pull request #10384 from ethereum/called_directly_feature

Use annotation.calledDirectly to simplify IR codegen

libsolidity/analysis/TypeChecker.cpp

@@ -2303,11 +2303,19 @@ bool TypeChecker::visit(FunctionCall const& _functionCall)
 		functionType = dynamic_cast<FunctionType const*>(expressionType);
 		funcCallAnno.kind = FunctionCallKind::FunctionCall;
 
+		if (auto memberAccess = dynamic_cast<MemberAccess const*>(&_functionCall.expression()))
+		{
+			if (dynamic_cast<FunctionDefinition const*>(memberAccess->annotation().referencedDeclaration))
+				_functionCall.expression().annotation().calledDirectly = true;
+		}
+		else if (auto identifier = dynamic_cast<Identifier const*>(&_functionCall.expression()))
+			if (dynamic_cast<FunctionDefinition const*>(identifier->annotation().referencedDeclaration))
+				_functionCall.expression().annotation().calledDirectly = true;
+
 		// Purity for function calls also depends upon the callee and its FunctionType
 		funcCallAnno.isPure =
 			argumentsArePure &&
 			*_functionCall.expression().annotation().isPure &&
-			functionType &&
 			functionType->isPure();
 
 		if (

libsolidity/ast/ASTAnnotations.h

@@ -260,6 +260,15 @@ struct ExpressionAnnotation: ASTAnnotation
 	/// Types and - if given - names of arguments if the expr. is a function
 	/// that is called, used for overload resolution
 	std::optional<FuncCallArguments> arguments;
+
+	/// True if the expression consists solely of the name of the function and the function is called immediately
+	/// instead of being stored or processed. The name may be qualified with the name of a contract, library
+	/// module, etc., that clarifies the scope. For example: `m.L.f()`, where `m` is a module, `L` is a library
+	/// and `f` is a function is a direct call. This means that the function to be called is known at compilation
+	/// time and it's not necessary to rely on any runtime dispatch mechanism to resolve it.
+	/// Note that even the simplest expressions, like `(f)()`, result in an indirect call even if they consist of
+	/// values known at compilation time.
+	bool calledDirectly = false;
 };
 
 struct IdentifierAnnotation: ExpressionAnnotation

libsolidity/codegen/ir/IRGenerationContext.cpp

@@ -137,38 +137,20 @@ void IRGenerationContext::initializeInternalDispatch(InternalDispatchMap _intern
 
 InternalDispatchMap IRGenerationContext::consumeInternalDispatchMap()
 {
-	m_directInternalFunctionCalls.clear();
-
 	InternalDispatchMap internalDispatch = move(m_internalDispatchMap);
 	m_internalDispatchMap.clear();
 	return internalDispatch;
 }
 
-void IRGenerationContext::internalFunctionCalledDirectly(Expression const& _expression)
+void IRGenerationContext::addToInternalDispatch(FunctionDefinition const& _function)
 {
-	solAssert(m_directInternalFunctionCalls.count(&_expression) == 0, "");
+	FunctionType const* functionType = TypeProvider::function(_function, FunctionType::Kind::Internal);
+	solAssert(functionType, "");
 
-	m_directInternalFunctionCalls.insert(&_expression);
+	m_internalDispatchMap[YulArity::fromType(*functionType)].insert(&_function);
+	enqueueFunctionForCodeGeneration(_function);
 }
 
-void IRGenerationContext::internalFunctionAccessed(Expression const& _expression, FunctionDefinition const& _function)
-{
-	solAssert(
-		IRHelpers::referencedFunctionDeclaration(_expression) &&
-		_function.resolveVirtual(mostDerivedContract()) ==
-		IRHelpers::referencedFunctionDeclaration(_expression)->resolveVirtual(mostDerivedContract()),
-		"Function definition does not match the expression"
-	);
-
-	if (m_directInternalFunctionCalls.count(&_expression) == 0)
-	{
-		FunctionType const* functionType = TypeProvider::function(_function, FunctionType::Kind::Internal);
-		solAssert(functionType, "");
-
-		m_internalDispatchMap[YulArity::fromType(*functionType)].insert(&_function);
-		enqueueFunctionForCodeGeneration(_function);
-	}
-}
 
 void IRGenerationContext::internalFunctionCalledThroughDispatch(YulArity const& _arity)
 {

libsolidity/codegen/ir/IRGenerationContext.h

@@ -108,7 +108,7 @@ public:
 
 	void initializeInternalDispatch(InternalDispatchMap _internalDispatchMap);
 	InternalDispatchMap consumeInternalDispatchMap();
-	bool internalDispatchClean() const { return m_internalDispatchMap.empty() && m_directInternalFunctionCalls.empty(); }
+	bool internalDispatchClean() const { return m_internalDispatchMap.empty(); }
 
 	/// Notifies the context that a function call that needs to go through internal dispatch was
 	/// encountered while visiting the AST. This ensures that the corresponding dispatch function
@@ -116,16 +116,8 @@ public:
 	/// the code contains a call to an uninitialized function variable).
 	void internalFunctionCalledThroughDispatch(YulArity const& _arity);
 
-	/// Notifies the context that a direct function call (i.e. not through internal dispatch) was
+	/// Adds a function to the internal dispatch.
-	/// encountered while visiting the AST. This lets the context know that the function should
+	void addToInternalDispatch(FunctionDefinition const& _function);
-	/// not be added to the dispatch (unless there are also indirect calls to it elsewhere else).
-	void internalFunctionCalledDirectly(Expression const& _expression);
-
-	/// Notifies the context that a name representing an internal function has been found while
-	/// visiting the AST. If the name has not been reported as a direct call using
-	/// @a internalFunctionCalledDirectly(), it's assumed to represent function variable access
-	/// and the function gets added to internal dispatch.
-	void internalFunctionAccessed(Expression const& _expression, FunctionDefinition const& _function);
 
 	/// @returns a new copy of the utility function generator (but using the same function set).
 	YulUtilFunctions utils();
@@ -179,7 +171,6 @@ private:
 	/// the code contains a call via a pointer even though a specific function is never assigned to it.
 	/// It will fail at runtime but the code must still compile.
 	InternalDispatchMap m_internalDispatchMap;
-	std::set<Expression const*> m_directInternalFunctionCalls;
 
 	std::set<ContractDefinition const*, ASTNode::CompareByID> m_subObjects;
 };

libsolidity/codegen/ir/IRGeneratorForStatements.cpp

@@ -810,20 +810,6 @@ bool IRGeneratorForStatements::visit(BinaryOperation const& _binOp)
 	return false;
 }
 
-bool IRGeneratorForStatements::visit(FunctionCall const& _functionCall)
-{
-	setLocation(_functionCall);
-	FunctionTypePointer functionType = dynamic_cast<FunctionType const*>(&type(_functionCall.expression()));
-	if (
-		functionType &&
-		functionType->kind() == FunctionType::Kind::Internal &&
-		IRHelpers::referencedFunctionDeclaration(_functionCall.expression())
-	)
-		m_context.internalFunctionCalledDirectly(_functionCall.expression());
-
-	return true;
-}
-
 void IRGeneratorForStatements::endVisit(FunctionCall const& _functionCall)
 {
 	setLocation(_functionCall);
@@ -1567,6 +1553,7 @@ void IRGeneratorForStatements::endVisit(FunctionCallOptions const& _options)
 void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 {
 	setLocation(_memberAccess);
+
 	ASTString const& member = _memberAccess.memberName();
 	auto memberFunctionType = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type);
 	Type::Category objectCategory = _memberAccess.expression().annotation().type->category();
@@ -1592,7 +1579,8 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 		if (memberFunctionType->kind() == FunctionType::Kind::Internal)
 		{
 			define(IRVariable(_memberAccess).part("functionIdentifier")) << to_string(functionDefinition.id()) << "\n";
-			m_context.internalFunctionAccessed(_memberAccess, functionDefinition);
+			if (!_memberAccess.annotation().calledDirectly)
+				m_context.addToInternalDispatch(functionDefinition);
 		}
 		else
 		{
@@ -1622,7 +1610,9 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 			define(_memberAccess) << to_string(resolvedFunctionDef.id()) << "\n";
 			solAssert(resolvedFunctionDef.functionType(true), "");
 			solAssert(resolvedFunctionDef.functionType(true)->kind() == FunctionType::Kind::Internal, "");
-			m_context.internalFunctionAccessed(_memberAccess, resolvedFunctionDef);
+
+			if (!_memberAccess.annotation().calledDirectly)
+				m_context.addToInternalDispatch(resolvedFunctionDef);
 		}
 		// ordinary contract type
 		else if (Declaration const* declaration = _memberAccess.annotation().referencedDeclaration)
@@ -1889,7 +1879,8 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 					if (auto const* function = dynamic_cast<FunctionDefinition const*>(_memberAccess.annotation().referencedDeclaration))
 					{
 						define(_memberAccess) << to_string(function->id()) << "\n";
-						m_context.internalFunctionAccessed(_memberAccess, *function);
+						if (!_memberAccess.annotation().calledDirectly)
+							m_context.addToInternalDispatch(*function);
 					}
 					else
 						solAssert(false, "Function not found in member access");
@@ -1969,7 +1960,9 @@ void IRGeneratorForStatements::endVisit(MemberAccess const& _memberAccess)
 			solAssert(*_memberAccess.annotation().requiredLookup == VirtualLookup::Static, "");
 
 			define(_memberAccess) << to_string(function->id()) << "\n";
-			m_context.internalFunctionAccessed(_memberAccess, *function);
+
+			if (!_memberAccess.annotation().calledDirectly)
+				m_context.addToInternalDispatch(*function);
 		}
 		break;
 	}
@@ -2211,7 +2204,8 @@ void IRGeneratorForStatements::endVisit(Identifier const& _identifier)
 
 		solAssert(resolvedFunctionDef.functionType(true), "");
 		solAssert(resolvedFunctionDef.functionType(true)->kind() == FunctionType::Kind::Internal, "");
-		m_context.internalFunctionAccessed(_identifier, resolvedFunctionDef);
+		if (!_identifier.annotation().calledDirectly)
+			m_context.addToInternalDispatch(resolvedFunctionDef);
 	}
 	else if (VariableDeclaration const* varDecl = dynamic_cast<VariableDeclaration const*>(declaration))
 		handleVariableReference(*varDecl, _identifier);

libsolidity/codegen/ir/IRGeneratorForStatements.h

@@ -74,7 +74,6 @@ public:
 	void endVisit(Return const& _return) override;
 	void endVisit(UnaryOperation const& _unaryOperation) override;
 	bool visit(BinaryOperation const& _binOp) override;
-	bool visit(FunctionCall const& _funCall) override;
 	void endVisit(FunctionCall const& _funCall) override;
 	void endVisit(FunctionCallOptions const& _funCallOptions) override;
 	void endVisit(MemberAccess const& _memberAccess) override;

test/libsolidity/semanticTests/libraries/internal_call_bound_with_parentheses1.sol

@@ -0,0 +1,16 @@
+library L {
+	function f() internal returns (uint) {
+		return 3;
+	}
+}
+
+contract C {
+	function foo() public returns (uint) {
+		return (L.f)();
+	}
+}
+
+// ====
+// compileViaYul: also
+// ----
+// foo() -> 3