Merge pull request #4779 from ethereum/optimise-string-literals

Decide better in storing string literals as data

8 files changed, 116 insertions, 2 deletions

diff --git a/Changelog.md b/Changelog.md
index 6a366ca0..02322a1c 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -5,6 +5,7 @@ Language Features:
 
 Compiler Features:
  * Inline Assembly: Improve error messages around invalid function argument count.
+ * Code Generator: Use codecopy for string constants more aggressively.
  * Code Generator: Use binary search for dispatch function if more efficient. The size/speed tradeoff can be tuned using ``--optimize-runs``.
  * Type Checker: Add an additional reason to be displayed when type conversion fails.
 
diff --git a/libsolidity/codegen/CompilerUtils.cpp b/libsolidity/codegen/CompilerUtils.cpp
index 7d2ad9d2..259f1620 100644
--- a/libsolidity/codegen/CompilerUtils.cpp
+++ b/libsolidity/codegen/CompilerUtils.cpp
@@ -1205,7 +1205,7 @@ void CompilerUtils::storeStringData(bytesConstRef _data)
 {
 	//@todo provide both alternatives to the optimiser
 	// stack: mempos
-	if (_data.size() <= 128)
+	if (_data.size() <= 32)
 	{
 		for (unsigned i = 0; i < _data.size(); i += 32)
 		{
diff --git a/test/boostTest.cpp b/test/boostTest.cpp
index 034aaef3..d6e75cb9 100644
--- a/test/boostTest.cpp
+++ b/test/boostTest.cpp
@@ -160,6 +160,7 @@ test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] )
 			"LLLEndToEndTest",
 #endif
 			"GasMeterTests",
+			"GasCostTests",
 			"SolidityEndToEndTest",
 			"SolidityOptimizer"
 		})
diff --git a/test/cmdlineTests/data_storage.sol b/test/cmdlineTests/data_storage.sol
new file mode 100644
index 00000000..cc602cc9
--- /dev/null
+++ b/test/cmdlineTests/data_storage.sol
@@ -0,0 +1,15 @@
+pragma solidity >=0.0;
+
+contract C {
+  function f() pure public {
+    require(false, "1234567890123456789012345678901");
+    require(false, "12345678901234567890123456789012");
+    require(false, "123456789012345678901234567890123");
+    require(false, "1234567890123456789012345678901234");
+    require(false, "12345678901234567890123456789012345");
+    require(false, "123456789012345678901234567890123456");
+    require(false, "123456789012345678901234567890121234567890123456789012345678901");
+    require(false, "1234567890123456789012345678901212345678901234567890123456789012");
+    require(false, "12345678901234567890123456789012123456789012345678901234567890123");
+  }
+}
diff --git a/test/cmdlineTests/data_storage.sol.args b/test/cmdlineTests/data_storage.sol.args
new file mode 100644
index 00000000..3684987e
--- /dev/null
+++ b/test/cmdlineTests/data_storage.sol.args
@@ -0,0 +1 @@
+--gas
diff --git a/test/cmdlineTests/data_storage.sol.stdout b/test/cmdlineTests/data_storage.sol.stdout
new file mode 100644
index 00000000..4a5250f7
--- /dev/null
+++ b/test/cmdlineTests/data_storage.sol.stdout
@@ -0,0 +1,7 @@
+
+======= data_storage.sol:C =======
+Gas estimation:
+construction:
+   306 + 264400 = 264706
+external:
+   f():	263
diff --git a/test/libsolidity/GasCosts.cpp b/test/libsolidity/GasCosts.cpp
new file mode 100644
index 00000000..15658a91
--- /dev/null
+++ b/test/libsolidity/GasCosts.cpp
@@ -0,0 +1,89 @@
+/*
+	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/>.
+*/
+/**
+ * Tests that check that the cost of certain operations stay within range.
+ */
+
+#include <test/libsolidity/SolidityExecutionFramework.h>
+
+#include <cmath>
+
+using namespace std;
+using namespace langutil;
+using namespace dev::eth;
+using namespace dev::solidity;
+using namespace dev::test;
+
+namespace dev
+{
+namespace solidity
+{
+namespace test
+{
+
+#define CHECK_GAS(_gasNoOpt, _gasOpt, _tolerance) \
+	do \
+	{ \
+		u256 gasOpt{_gasOpt}; \
+		u256 gasNoOpt{_gasNoOpt}; \
+		u256 tolerance{_tolerance}; \
+		u256 gas = m_optimize ? gasOpt : gasNoOpt; \
+		u256 diff = gas < m_gasUsed ? m_gasUsed - gas : gas - m_gasUsed; \
+		BOOST_CHECK_MESSAGE( \
+			diff <= tolerance, \
+			"Gas used: " + \
+			m_gasUsed.str() + \
+			" - expected: " + \
+			gas.str() + \
+			" (tolerance: " + \
+			tolerance.str() + \
+			")" \
+		); \
+	} while(0)
+
+BOOST_FIXTURE_TEST_SUITE(GasCostTests, SolidityExecutionFramework)
+
+BOOST_AUTO_TEST_CASE(string_storage)
+{
+	char const* sourceCode = R"(
+		contract C {
+			function f() pure public {
+				require(false, "Not Authorized. This function can only be called by the custodian or owner of this contract");
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+
+	if (Options::get().evmVersion() <= EVMVersion::byzantium())
+		CHECK_GAS(134435, 130591, 100);
+	else
+		CHECK_GAS(127225, 124873, 100);
+	if (Options::get().evmVersion() >= EVMVersion::byzantium())
+	{
+		callContractFunction("f()");
+		if (Options::get().evmVersion() == EVMVersion::byzantium())
+			CHECK_GAS(21551, 21526, 20);
+		else
+			CHECK_GAS(21546, 21526, 20);
+	}
+}
+
+BOOST_AUTO_TEST_SUITE_END()
+
+}
+}
+}
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index c6135a72..dfa60fc5 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -12245,7 +12245,7 @@ BOOST_AUTO_TEST_CASE(include_creation_bytecode_only_once)
 	compileAndRun(sourceCode);
 	BOOST_CHECK_LE(
 		double(m_compiler.object("Double").bytecode.size()),
-		1.1 * double(m_compiler.object("Single").bytecode.size())
+		1.2 * double(m_compiler.object("Single").bytecode.size())
 	);
 }