Merge pull request #13019 from ethereum/enforceViaYulNoSmtTests

Remove enforce-via-yul CI run and instead always force via yul in regular test runs.

Author: ekpyron

.circleci/config.yml

@@ -1035,16 +1035,6 @@ jobs:
           condition: true
           <<: *steps_soltest
 
-  t_ubu_soltest_enforce_yul: &t_ubu_soltest_enforce_yul
-    <<: *base_ubuntu2004
-    parallelism: 20
-    environment:
-      EVM: << pipeline.parameters.evm-version >>
-      SOLTEST_FLAGS: --enforce-via-yul
-      OPTIMIZE: 0
-      TERM: xterm
-    <<: *steps_soltest
-
   t_ubu_clang_soltest: &t_ubu_clang_soltest
     <<: *base_ubuntu2004_clang
     parallelism: 20
@@ -1515,7 +1505,6 @@ workflows:
       - t_ubu_cli: *workflow_ubuntu2004
       - t_ubu_locale: *workflow_ubuntu2004
       - t_ubu_soltest_all: *workflow_ubuntu2004
-      - t_ubu_soltest_enforce_yul: *workflow_ubuntu2004
       - b_ubu_clang: *workflow_trigger_on_tags
       - t_ubu_clang_soltest: *workflow_ubuntu2004_clang
       - t_ubu_lsp: *workflow_ubuntu2004

test/Common.cpp

@@ -107,7 +107,6 @@ void CommonOptions::addOptions()
 		("no-semantic-tests", po::bool_switch(&disableSemanticTests)->default_value(disableSemanticTests), "disable semantic tests")
 		("no-smt", po::bool_switch(&disableSMT)->default_value(disableSMT), "disable SMT checker")
 		("optimize", po::bool_switch(&optimize)->default_value(optimize), "enables optimization")
-		("enforce-via-yul", po::value<bool>(&enforceViaYul)->default_value(enforceViaYul)->implicit_value(true), "Enforce compiling all tests via yul to see if additional tests can be activated.")
 		("enforce-compile-to-ewasm", po::bool_switch(&enforceCompileToEwasm)->default_value(enforceCompileToEwasm), "Enforce compiling all tests to Ewasm to see if additional tests can be activated.")
 		("enforce-gas-cost", po::value<bool>(&enforceGasTest)->default_value(enforceGasTest)->implicit_value(true), "Enforce checking gas cost in semantic tests.")
 		("enforce-gas-cost-min-value", po::value(&enforceGasTestMinValue)->default_value(enforceGasTestMinValue), "Threshold value to enforce adding gas checks to a test.")

test/Common.h

@@ -60,7 +60,6 @@ struct CommonOptions
 	boost::filesystem::path testPath;
 	bool ewasm = false;
 	bool optimize = false;
-	bool enforceViaYul = false;
 	bool enforceCompileToEwasm = false;
 	bool enforceGasTest = false;
 	u256 enforceGasTestMinValue = 100000;

test/TestCase.h

@@ -40,7 +40,6 @@ class TestCase
 		std::string filename;
 		langutil::EVMVersion evmVersion;
 		std::vector<boost::filesystem::path> vmPaths;
-		bool enforceCompileViaYul = false;
 		bool enforceCompileToEwasm = false;
 		bool enforceGasCost = false;
 		u256 enforceGasCostMinValue;

test/boostTest.cpp

@@ -133,7 +133,6 @@ int registerTests(
 	boost::unit_test::test_suite& _suite,
 	boost::filesystem::path const& _basepath,
 	boost::filesystem::path const& _path,
-	bool _enforceViaYul,
 	bool _enforceCompileToEwasm,
 	vector<string> const& _labels,
 	TestCase::TestCaseCreator _testCaseCreator,
@@ -146,7 +145,6 @@ int registerTests(
 		fullpath.string(),
 		solidity::test::CommonOptions::get().evmVersion(),
 		solidity::test::CommonOptions::get().vmPaths,
-		_enforceViaYul,
 		_enforceCompileToEwasm,
 		solidity::test::CommonOptions::get().enforceGasTest,
 		solidity::test::CommonOptions::get().enforceGasTestMinValue,
@@ -165,7 +163,6 @@ int registerTests(
 				numTestsAdded += registerTests(
 					*sub_suite,
 					_basepath, _path / entry.path().filename(),
-					_enforceViaYul,
 					_enforceCompileToEwasm,
 					_labels,
 					_testCaseCreator,
@@ -271,7 +268,6 @@ test_suite* init_unit_test_suite(int /*argc*/, char* /*argv*/[])
 				master,
 				options.testPath / ts.path,
 				ts.subpath,
-				options.enforceViaYul,
 				options.enforceCompileToEwasm,
 				ts.labels,
 				ts.testCaseCreator,

test/libsolidity/SemanticTest.cpp

@@ -49,7 +49,6 @@ SemanticTest::SemanticTest(
 	string const& _filename,
 	langutil::EVMVersion _evmVersion,
 	vector<boost::filesystem::path> const& _vmPaths,
-	bool _enforceViaYul,
 	bool _enforceCompileToEwasm,
 	bool _enforceGasCost,
 	u256 _enforceGasCostMinValue
@@ -60,27 +59,32 @@ SemanticTest::SemanticTest(
 	m_lineOffset(m_reader.lineNumber()),
 	m_builtins(makeBuiltins()),
 	m_sideEffectHooks(makeSideEffectHooks()),
-	m_enforceViaYul(_enforceViaYul),
 	m_enforceCompileToEwasm(_enforceCompileToEwasm),
 	m_enforceGasCost(_enforceGasCost),
 	m_enforceGasCostMinValue(move(_enforceGasCostMinValue))
 {
-	static set<string> const compileViaYulAllowedValues{"also", "true", "false", "default"};
+	static set<string> const compileViaYulAllowedValues{"also", "true", "false"};
 	static set<string> const yulRunTriggers{"also", "true"};
 	static set<string> const legacyRunTriggers{"also", "false", "default"};
 
-	string compileViaYul = m_reader.stringSetting("compileViaYul", "default");
+	m_runWithABIEncoderV1Only = m_reader.boolSetting("ABIEncoderV1Only", false);
+	if (m_runWithABIEncoderV1Only && !solidity::test::CommonOptions::get().useABIEncoderV1)
+		m_shouldRun = false;
+
+	string compileViaYul = m_reader.stringSetting("compileViaYul", "also");
+	if (m_runWithABIEncoderV1Only && compileViaYul != "false")
+		BOOST_THROW_EXCEPTION(runtime_error(
+			"ABIEncoderV1Only tests cannot be run via yul, "
+			"so they need to also specify ``compileViaYul: false``"
+		));
 	if (!util::contains(compileViaYulAllowedValues, compileViaYul))
 		BOOST_THROW_EXCEPTION(runtime_error("Invalid compileViaYul value: " + compileViaYul + "."));
 	m_testCaseWantsYulRun = util::contains(yulRunTriggers, compileViaYul);
 	m_testCaseWantsLegacyRun = util::contains(legacyRunTriggers, compileViaYul);
 
-	// Do not enforce via yul and ewasm, if via yul was explicitly denied.
+	// Do not enforce ewasm, if via yul was explicitly denied.
 	if (compileViaYul == "false")
-	{
-		m_enforceViaYul = false;
 		m_enforceCompileToEwasm = false;
-	}
 
 	string compileToEwasm = m_reader.stringSetting("compileToEwasm", "false");
 	if (compileToEwasm == "also")
@@ -97,18 +101,6 @@ SemanticTest::SemanticTest(
 	if (m_testCaseWantsEwasmRun && !m_supportsEwasm)
 		m_testCaseWantsEwasmRun = false;
 
-	m_runWithABIEncoderV1Only = m_reader.boolSetting("ABIEncoderV1Only", false);
-	if (m_runWithABIEncoderV1Only && !solidity::test::CommonOptions::get().useABIEncoderV1)
-		m_shouldRun = false;
-
-	// Sanity check
-	if (m_runWithABIEncoderV1Only && (compileViaYul == "true" || compileViaYul == "also"))
-		BOOST_THROW_EXCEPTION(runtime_error(
-			"ABIEncoderV1Only can not be used with compileViaYul=" + compileViaYul +
-			", set it to false or omit the flag. The compileViaYul setting ignores the abicoder pragma"
-			" and runs everything with ABICoder V2."
-		));
-
 	auto revertStrings = revertStringsFromString(m_reader.stringSetting("revertStrings", "default"));
 	soltestAssert(revertStrings, "Invalid revertStrings setting.");
 	m_revertStrings = revertStrings.value();
@@ -307,7 +299,7 @@ TestCase::TestResult SemanticTest::run(ostream& _stream, string const& _linePref
 	if (m_testCaseWantsLegacyRun)
 		result = runTest(_stream, _linePrefix, _formatted, false, false);
 
-	if ((m_testCaseWantsYulRun || m_enforceViaYul) && result == TestResult::Success)
+	if (m_testCaseWantsYulRun && result == TestResult::Success)
 		result = runTest(_stream, _linePrefix, _formatted, true, false);
 
 	if ((m_testCaseWantsEwasmRun || m_enforceCompileToEwasm) && result == TestResult::Success)
@@ -353,7 +345,6 @@ TestCase::TestResult SemanticTest::runTest(
 		m_compileToEwasm = _isEwasmRun;
 	}
 
-	m_canEnableYulRun = false;
 	m_canEnableEwasmRun = false;
 
 	if (_isYulRun)
@@ -473,18 +464,6 @@ TestCase::TestResult SemanticTest::runTest(
 		success &= test.call().expectedSideEffects == test.call().actualSideEffects;
 	}
 
-	if (!m_testCaseWantsYulRun && _isYulRun)
-	{
-		m_canEnableYulRun = success;
-		string message = success ?
-			"Test can pass via Yul, but marked with \"compileViaYul: false.\"" :
-			"Test compiles via Yul, but it gives different test results.";
-		AnsiColorized(_stream, _formatted, {BOLD, success ? YELLOW : MAGENTA}) <<
-			_linePrefix << endl <<
-			_linePrefix << message << endl;
-		return TestResult::Failure;
-	}
-
 	// Right now we have sometimes different test results in Yul vs. Ewasm.
 	// The main reason is that Ewasm just returns a failure in some cases.
 	// TODO: If Ewasm support got fully implemented, we could implement this in the same way as above.
@@ -662,25 +641,19 @@ void SemanticTest::printUpdatedExpectations(ostream& _stream, string const&) con
 void SemanticTest::printUpdatedSettings(ostream& _stream, string const& _linePrefix)
 {
 	auto& settings = m_reader.settings();
-	if (settings.empty() && !m_canEnableYulRun)
+	if (settings.empty() && !m_canEnableEwasmRun)
 		return;
 
 	_stream << _linePrefix << "// ====" << endl;
 	if (m_canEnableEwasmRun)
 	{
-		soltestAssert(m_canEnableYulRun || m_testCaseWantsYulRun, "");
-		string compileViaYul = m_reader.stringSetting("compileViaYul", "");
-		if (!compileViaYul.empty())
-			_stream << _linePrefix << "// compileViaYul: " << compileViaYul << "\n";
+		soltestAssert(m_testCaseWantsYulRun, "");
 		_stream << _linePrefix << "// compileToEwasm: also\n";
 	}
-	else if (m_canEnableYulRun)
-		_stream << _linePrefix << "// compileViaYul: also\n";
 
 	for (auto const& [settingName, settingValue]: settings)
 		if (
-			!(settingName == "compileToEwasm" && m_canEnableEwasmRun) &&
-			!(settingName == "compileViaYul" && (m_canEnableYulRun || m_canEnableEwasmRun))
+			!(settingName == "compileToEwasm" && m_canEnableEwasmRun)
 		)
 			_stream << _linePrefix << "// " << settingName << ": " << settingValue<< endl;
 }

test/libsolidity/SemanticTest.h

@@ -52,7 +52,6 @@ class SemanticTest: public SolidityExecutionFramework, public EVMVersionRestrict
 			_options.filename,
 			_options.evmVersion,
 			_options.vmPaths,
-			_options.enforceCompileViaYul,
 			_options.enforceCompileToEwasm,
 			_options.enforceGasCost,
 			_options.enforceGasCostMinValue
@@ -63,7 +62,6 @@ class SemanticTest: public SolidityExecutionFramework, public EVMVersionRestrict
 		std::string const& _filename,
 		langutil::EVMVersion _evmVersion,
 		std::vector<boost::filesystem::path> const& _vmPaths,
-		bool _enforceViaYul = false,
 		bool _enforceCompileToEwasm = false,
 		bool _enforceGasCost = false,
 		u256 _enforceGasCostMinValue = 100000
@@ -97,14 +95,12 @@ class SemanticTest: public SolidityExecutionFramework, public EVMVersionRestrict
 	std::vector<TestFunctionCall> m_tests;
 	std::map<std::string, Builtin> const m_builtins;
 	std::vector<SideEffectHook> const m_sideEffectHooks;
-	bool m_testCaseWantsYulRun = false;
+	bool m_testCaseWantsYulRun = true;
 	bool m_testCaseWantsEwasmRun = false;
 	bool m_testCaseWantsLegacyRun = true;
-	bool m_enforceViaYul = false;
 	bool m_enforceCompileToEwasm = false;
 	bool m_runWithABIEncoderV1Only = false;
 	bool m_allowNonExistingFunctions = false;
-	bool m_canEnableYulRun = false;
 	bool m_canEnableEwasmRun = false;
 	bool m_gasCostFailure = false;
 	bool m_enforceGasCost = false;

test/libsolidity/semanticTests/abiEncoderV1/abi_decode_dynamic_array.sol

@@ -4,7 +4,5 @@ contract C {
     }
 }
 
-// ====
-// compileViaYul: also
 // ----
 // f(bytes): 0x20, 0xc0, 0x20, 0x4, 0x3, 0x4, 0x5, 0x6 -> 0x20, 0x4, 0x3, 0x4, 0x5, 0x6

test/libsolidity/semanticTests/abiEncoderV1/abi_decode_fixed_arrays.sol

@@ -4,7 +4,5 @@ contract C {
         return (a[i], b[j][k]);
     }
 }
-// ====
-// compileViaYul: also
 // ----
 // f(uint16[3],uint16[2][3],uint256,uint256,uint256): 1, 2, 3, 11, 12, 21, 22, 31, 32, 1, 2, 1 -> 2, 32

test/libsolidity/semanticTests/abiEncoderV1/abi_decode_static_array.sol

@@ -8,7 +8,5 @@ contract C {
     }
 }
 
-// ====
-// compileViaYul: also
 // ----
 // f(bytes): 0x20, 0xc0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6 -> 1, 2, 3, 4, 5, 6