Merge pull request #3686 from ethereum/doNotIncludeItnernal

Do not include internal functions only used by constructor

Changelog.md

@@ -5,6 +5,7 @@ Features:
 
 Bugfixes:
  * Code Generator: Allow ``block.blockhash`` without being called.
+ * Code Generator: Do not include internal functions in the runtime bytecode which are only referenced in the constructor.
  * Code Generator: Properly skip unneeded storage array cleanup when not reducing length.
  * Code Generator: Bugfix in modifier lookup in libraries.
  * Commandline interface: Support ``--evm-version constantinople`` properly.

libsolidity/codegen/CompilerUtils.cpp

@@ -585,7 +585,7 @@ void CompilerUtils::combineExternalFunctionType(bool _leftAligned)
 		leftShiftNumberOnStack(64);
 }
 
-void CompilerUtils::pushCombinedFunctionEntryLabel(Declaration const& _function)
+void CompilerUtils::pushCombinedFunctionEntryLabel(Declaration const& _function, bool _runtimeOnly)
 {
 	m_context << m_context.functionEntryLabel(_function).pushTag();
 	// If there is a runtime context, we have to merge both labels into the same
@@ -593,9 +593,10 @@ void CompilerUtils::pushCombinedFunctionEntryLabel(Declaration const& _function)
 	if (CompilerContext* rtc = m_context.runtimeContext())
 	{
 		leftShiftNumberOnStack(32);
-		m_context <<
-			rtc->functionEntryLabel(_function).toSubAssemblyTag(m_context.runtimeSub()) <<
-			Instruction::OR;
+		if (_runtimeOnly)
+			m_context <<
+				rtc->functionEntryLabel(_function).toSubAssemblyTag(m_context.runtimeSub()) <<
+				Instruction::OR;
 	}
 }
 

libsolidity/codegen/CompilerUtils.h

@@ -188,7 +188,8 @@ public:
 	/// Appends code that combines the construction-time (if available) and runtime function
 	/// entry label of the given function into a single stack slot.
 	/// Note: This might cause the compilation queue of the runtime context to be extended.
-	void pushCombinedFunctionEntryLabel(Declaration const& _function);
+	/// If @a _runtimeOnly, the entry label will include the runtime assembly tag.
+	void pushCombinedFunctionEntryLabel(Declaration const& _function, bool _runtimeOnly = true);
 
 	/// Appends code for an implicit or explicit type conversion. This includes erasing higher
 	/// order bits (@see appendHighBitCleanup) when widening integer but also copy to memory

libsolidity/codegen/ExpressionCompiler.cpp

@@ -518,7 +518,23 @@ bool ExpressionCompiler::visit(FunctionCall const& _functionCall)
 				arguments[i]->accept(*this);
 				utils().convertType(*arguments[i]->annotation().type, *function.parameterTypes()[i]);
 			}
-			_functionCall.expression().accept(*this);
+
+			{
+				bool shortcutTaken = false;
+				if (auto identifier = dynamic_cast<Identifier const*>(&_functionCall.expression()))
+					if (auto functionDef = dynamic_cast<FunctionDefinition const*>(identifier->annotation().referencedDeclaration))
+					{
+						// Do not directly visit the identifier, because this way, we can avoid
+						// the runtime entry label to be created at the creation time context.
+						CompilerContext::LocationSetter locationSetter2(m_context, *identifier);
+						utils().pushCombinedFunctionEntryLabel(m_context.resolveVirtualFunction(*functionDef), false);
+						shortcutTaken = true;
+					}
+
+				if (!shortcutTaken)
+					_functionCall.expression().accept(*this);
+			}
+
 			unsigned parameterSize = CompilerUtils::sizeOnStack(function.parameterTypes());
 			if (function.bound())
 			{
@@ -1359,6 +1375,10 @@ void ExpressionCompiler::endVisit(Identifier const& _identifier)
 		}
 	}
 	else if (FunctionDefinition const* functionDef = dynamic_cast<FunctionDefinition const*>(declaration))
+		// If the identifier is called right away, this code is executed in visit(FunctionCall...), because
+		// we want to avoid having a reference to the runtime function entry point in the
+		// constructor context, since this would force the compiler to include unreferenced
+		// internal functions in the runtime contex.
 		utils().pushCombinedFunctionEntryLabel(m_context.resolveVirtualFunction(*functionDef));
 	else if (auto variable = dynamic_cast<VariableDeclaration const*>(declaration))
 		appendVariable(*variable, static_cast<Expression const&>(_identifier));

test/libsolidity/SolidityCompiler.cpp

@@ -0,0 +1,60 @@
+/*
+	This file is part of solidity.
+
+	solidity is free software: you can redistribute it and/or modify
+	it under the terms of the GNU General Public License as published by
+	the Free Software Foundation, either version 3 of the License, or
+	(at your option) any later version.
+
+	solidity is distributed in the hope that it will be useful,
+	but WITHOUT ANY WARRANTY; without even the implied warranty of
+	MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+	GNU General Public License for more details.
+
+	You should have received a copy of the GNU General Public License
+	along with solidity.  If not, see <http://www.gnu.org/licenses/>.
+ */
+/**
+ * Unit tests for the compiler itself.
+ */
+
+#include <test/libsolidity/AnalysisFramework.h>
+
+#include <test/Options.h>
+
+using namespace std;
+
+namespace dev
+{
+namespace solidity
+{
+namespace test
+{
+
+BOOST_FIXTURE_TEST_SUITE(Compiler, AnalysisFramework)
+
+BOOST_AUTO_TEST_CASE(does_not_include_creation_time_only_internal_functions)
+{
+	char const* sourceCode = R"(
+		contract C {
+			uint x;
+			function C() { f(); }
+			function f() internal { for (uint i = 0; i < 10; ++i) x += 3 + i; }
+		}
+	)";
+	BOOST_REQUIRE(success(sourceCode));
+	m_compiler.setOptimiserSettings(dev::test::Options::get().optimize);
+	BOOST_REQUIRE_MESSAGE(m_compiler.compile(), "Compiling contract failed");
+	bytes const& creationBytecode = m_compiler.object("C").bytecode;
+	bytes const& runtimeBytecode = m_compiler.runtimeObject("C").bytecode;
+	BOOST_CHECK(creationBytecode.size() >= 120);
+	BOOST_CHECK(creationBytecode.size() <= 150);
+	BOOST_CHECK(runtimeBytecode.size() >= 50);
+	BOOST_CHECK(runtimeBytecode.size() <= 70);
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+
+}
+}
+}