From 0bb885dab2c64dc1dec40b0f1f898b141a7e65d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 15 Nov 2021 17:04:37 +0100 Subject: [PATCH 1/7] Bring soltestAssert() up to date with solAssert() - Allow omitting description. - Provide a default description. - Use a custom exception type derived from util::Exception rather than std::exception. --- test/libsolidity/util/SoltestErrors.h | 29 ++++++++++++++++++++------- 1 file changed, 22 insertions(+), 7 deletions(-) diff --git a/test/libsolidity/util/SoltestErrors.h b/test/libsolidity/util/SoltestErrors.h index e223f714b..849a8b34e 100644 --- a/test/libsolidity/util/SoltestErrors.h +++ b/test/libsolidity/util/SoltestErrors.h @@ -15,20 +15,35 @@ #pragma once #include +#include #include #include +#include +#include +#include + namespace solidity::frontend::test { -#define soltestAssert(CONDITION, DESCRIPTION) \ - do \ - { \ - if (!(CONDITION)) \ - BOOST_THROW_EXCEPTION(std::runtime_error(DESCRIPTION)); \ - } \ - while (false) +struct InternalSoltestError: virtual util::Exception {}; +#if !BOOST_PP_VARIADICS_MSVC +#define soltestAssert(...) BOOST_PP_OVERLOAD(soltestAssert_,__VA_ARGS__)(__VA_ARGS__) +#else +#define soltestAssert(...) BOOST_PP_CAT(BOOST_PP_OVERLOAD(soltestAssert_,__VA_ARGS__)(__VA_ARGS__),BOOST_PP_EMPTY()) +#endif + +#define soltestAssert_1(CONDITION) \ + soltestAssert_2((CONDITION), "") + +#define soltestAssert_2(CONDITION, DESCRIPTION) \ + assertThrowWithDefaultDescription( \ + (CONDITION), \ + ::solidity::frontend::test::InternalSoltestError, \ + (DESCRIPTION), \ + "Soltest assertion failed" \ + ) class TestParserError: virtual public util::Exception { From 3c5930dd8e0ed6622b34799ef98f59ef75813460 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Wed, 6 Apr 2022 22:22:23 +0200 Subject: [PATCH 2/7] Put arguments in parantheses in assert macro definitions --- liblangutil/Exceptions.h | 16 ++++++++-------- libsmtutil/Exceptions.h | 6 +++--- libsolutil/Assertions.h | 4 ++-- libsolutil/Exceptions.h | 2 +- libyul/Exceptions.h | 6 +++--- 5 files changed, 17 insertions(+), 17 deletions(-) diff --git a/liblangutil/Exceptions.h b/liblangutil/Exceptions.h index 10fca8029..fb6f23cf1 100644 --- a/liblangutil/Exceptions.h +++ b/liblangutil/Exceptions.h @@ -58,13 +58,13 @@ struct InvalidAstError: virtual util::Exception {}; #endif #define solAssert_1(CONDITION) \ - solAssert_2(CONDITION, "") + solAssert_2((CONDITION), "") #define solAssert_2(CONDITION, DESCRIPTION) \ assertThrowWithDefaultDescription( \ - CONDITION, \ + (CONDITION), \ ::solidity::langutil::InternalCompilerError, \ - DESCRIPTION, \ + (DESCRIPTION), \ "Solidity assertion failed" \ ) @@ -77,13 +77,13 @@ struct InvalidAstError: virtual util::Exception {}; #endif #define solUnimplementedAssert_1(CONDITION) \ - solUnimplementedAssert_2(CONDITION, "") + solUnimplementedAssert_2((CONDITION), "") #define solUnimplementedAssert_2(CONDITION, DESCRIPTION) \ assertThrowWithDefaultDescription( \ - CONDITION, \ + (CONDITION), \ ::solidity::langutil::UnimplementedFeatureError, \ - DESCRIPTION, \ + (DESCRIPTION), \ "Unimplemented feature" \ ) @@ -105,9 +105,9 @@ struct InvalidAstError: virtual util::Exception {}; #define astAssert_2(CONDITION, DESCRIPTION) \ assertThrowWithDefaultDescription( \ - CONDITION, \ + (CONDITION), \ ::solidity::langutil::InvalidAstError, \ - DESCRIPTION, \ + (DESCRIPTION), \ "AST assertion failed" \ ) diff --git a/libsmtutil/Exceptions.h b/libsmtutil/Exceptions.h index fd144ca72..2a32c4fcd 100644 --- a/libsmtutil/Exceptions.h +++ b/libsmtutil/Exceptions.h @@ -38,13 +38,13 @@ struct SMTLogicError: virtual util::Exception {}; #endif #define smtAssert_1(CONDITION) \ - smtAssert_2(CONDITION, "") + smtAssert_2((CONDITION), "") #define smtAssert_2(CONDITION, DESCRIPTION) \ assertThrowWithDefaultDescription( \ - CONDITION, \ + (CONDITION), \ ::solidity::smtutil::SMTLogicError, \ - DESCRIPTION, \ + (DESCRIPTION), \ "SMT assertion failed" \ ) diff --git a/libsolutil/Assertions.h b/libsolutil/Assertions.h index 81823e040..4924522d6 100644 --- a/libsolutil/Assertions.h +++ b/libsolutil/Assertions.h @@ -63,7 +63,7 @@ inline std::string stringOrDefault(std::string _string, std::string _defaultStri if (!(_condition)) \ solThrow( \ _exceptionType, \ - ::solidity::util::assertions::stringOrDefault(_description, _defaultDescription) \ + ::solidity::util::assertions::stringOrDefault((_description), (_defaultDescription)) \ ); \ } \ while (false) @@ -72,6 +72,6 @@ inline std::string stringOrDefault(std::string _string, std::string _defaultStri /// Use it as assertThrow(1 == 1, ExceptionType, "Mathematics is wrong."); /// The second parameter must be an exception class (rather than an instance). #define assertThrow(_condition, _exceptionType, _description) \ - assertThrowWithDefaultDescription(_condition, _exceptionType, _description, "Assertion failed") + assertThrowWithDefaultDescription((_condition), _exceptionType, (_description), "Assertion failed") } diff --git a/libsolutil/Exceptions.h b/libsolutil/Exceptions.h index 66b2442ef..8ac7a5fae 100644 --- a/libsolutil/Exceptions.h +++ b/libsolutil/Exceptions.h @@ -48,7 +48,7 @@ struct Exception: virtual std::exception, virtual boost::exception #define solThrow(_exceptionType, _description) \ ::boost::throw_exception( \ _exceptionType() << \ - ::solidity::util::errinfo_comment(_description) << \ + ::solidity::util::errinfo_comment((_description)) << \ ::boost::throw_function(ETH_FUNC) << \ ::boost::throw_file(__FILE__) << \ ::boost::throw_line(__LINE__) \ diff --git a/libyul/Exceptions.h b/libyul/Exceptions.h index 1ad7785e9..45c681a5d 100644 --- a/libyul/Exceptions.h +++ b/libyul/Exceptions.h @@ -63,13 +63,13 @@ struct StackTooDeepError: virtual YulException #endif #define yulAssert_1(CONDITION) \ - yulAssert_2(CONDITION, "") + yulAssert_2((CONDITION), "") #define yulAssert_2(CONDITION, DESCRIPTION) \ assertThrowWithDefaultDescription( \ - CONDITION, \ + (CONDITION), \ ::solidity::yul::YulAssertion, \ - DESCRIPTION, \ + (DESCRIPTION), \ "Yul assertion failed" \ ) From ed8403f45657964bf14f9bf8e8fa063e55bd045b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 15 Nov 2021 17:47:21 +0100 Subject: [PATCH 3/7] isoltest: Handle parsing errors differently from unexpected exceptions --- test/tools/isoltest.cpp | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/test/tools/isoltest.cpp b/test/tools/isoltest.cpp index ae2ce6e24..746cee7e6 100644 --- a/test/tools/isoltest.cpp +++ b/test/tools/isoltest.cpp @@ -499,9 +499,20 @@ int main(int argc, char const *argv[]) return global_stats ? 0 : 1; } - catch (std::exception const& _exception) + catch (boost::program_options::error const& exception) { - cerr << _exception.what() << endl; + cerr << exception.what() << endl; + return 1; + } + catch (std::runtime_error const& exception) + { + cerr << exception.what() << endl; + return 1; + } + catch (...) + { + cerr << "Unhandled exception caught." << endl; + cerr << boost::current_exception_diagnostic_information() << endl; return 1; } } From 7bace8d25d55a71a243ca117699f9c665a47ca2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 15 Nov 2021 17:49:27 +0100 Subject: [PATCH 4/7] soltest: Don't assume that parse() can never return false --- test/boostTest.cpp | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/test/boostTest.cpp b/test/boostTest.cpp index 1cc512c32..0d3b0eb34 100644 --- a/test/boostTest.cpp +++ b/test/boostTest.cpp @@ -204,15 +204,18 @@ int registerTests( return numTestsAdded; } -void initializeOptions() +bool initializeOptions() { auto const& suite = boost::unit_test::framework::master_test_suite(); auto options = std::make_unique(); - solAssert(options->parse(suite.argc, suite.argv), "Failed to parse options!"); + bool shouldContinue = options->parse(suite.argc, suite.argv); + if (!shouldContinue) + return false; options->validate(); solidity::test::CommonOptions::setSingleton(std::move(options)); + return true; } } @@ -228,7 +231,9 @@ test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] ) master_test_suite_t& master = framework::master_test_suite(); master.p_name.value = "SolidityTests"; - initializeOptions(); + bool shouldContinue = initializeOptions(); + if (!shouldContinue) + exit(0); if (!solidity::test::loadVMs(solidity::test::CommonOptions::get())) exit(1); From cf6704ae0636fce85fda9d4dd52e1e2f86e97fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 15 Nov 2021 18:19:37 +0100 Subject: [PATCH 5/7] isoltest: Do not return an error code from `--help` --- test/Common.h | 3 +++ test/tools/IsolTestOptions.cpp | 6 +++--- test/tools/isoltest.cpp | 5 +++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/test/Common.h b/test/Common.h index e7e211d9e..28862348f 100644 --- a/test/Common.h +++ b/test/Common.h @@ -75,6 +75,9 @@ struct CommonOptions langutil::EVMVersion evmVersion() const; virtual void addOptions(); + // @returns true if the program should continue, false if it should exit immediately without + // reporting an error. + // Throws ConfigException or std::runtime_error if parsing fails. virtual bool parse(int argc, char const* const* argv); // Throws a ConfigException on error virtual void validate() const; diff --git a/test/tools/IsolTestOptions.cpp b/test/tools/IsolTestOptions.cpp index cdd1b85ab..ca7761d74 100644 --- a/test/tools/IsolTestOptions.cpp +++ b/test/tools/IsolTestOptions.cpp @@ -75,9 +75,9 @@ void IsolTestOptions::addOptions() bool IsolTestOptions::parse(int _argc, char const* const* _argv) { - bool const res = CommonOptions::parse(_argc, _argv); + bool const shouldContinue = CommonOptions::parse(_argc, _argv); - if (showHelp || !res) + if (showHelp || !shouldContinue) { std::cout << options << std::endl; return false; @@ -85,7 +85,7 @@ bool IsolTestOptions::parse(int _argc, char const* const* _argv) enforceGasTest = enforceGasTest || (evmVersion() == langutil::EVMVersion{} && !useABIEncoderV1); - return res; + return shouldContinue; } void IsolTestOptions::validate() const diff --git a/test/tools/isoltest.cpp b/test/tools/isoltest.cpp index 746cee7e6..a2ef564cb 100644 --- a/test/tools/isoltest.cpp +++ b/test/tools/isoltest.cpp @@ -433,8 +433,9 @@ int main(int argc, char const *argv[]) { auto options = std::make_unique(); - if (!options->parse(argc, argv)) - return -1; + bool shouldContinue = options->parse(argc, argv); + if (!shouldContinue) + return 0; options->validate(); CommonOptions::setSingleton(std::move(options)); From 7c835598819877f3b1258f0fa29379fa2bc4b530 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Mon, 15 Nov 2021 17:51:18 +0100 Subject: [PATCH 6/7] soltest: Handle CLI validation errors gracefully --- test/Common.cpp | 45 +++++++++------- test/boostTest.cpp | 125 +++++++++++++++++++++++++-------------------- 2 files changed, 95 insertions(+), 75 deletions(-) diff --git a/test/Common.cpp b/test/Common.cpp index a702cddd6..8cdf0fed1 100644 --- a/test/Common.cpp +++ b/test/Common.cpp @@ -159,26 +159,33 @@ bool CommonOptions::parse(int argc, char const* const* argv) po::variables_map arguments; addOptions(); - po::command_line_parser cmdLineParser(argc, argv); - cmdLineParser.options(options); - auto parsedOptions = cmdLineParser.run(); - po::store(parsedOptions, arguments); - po::notify(arguments); + try + { + po::command_line_parser cmdLineParser(argc, argv); + cmdLineParser.options(options); + auto parsedOptions = cmdLineParser.run(); + po::store(parsedOptions, arguments); + po::notify(arguments); - for (auto const& parsedOption: parsedOptions.options) - if (parsedOption.position_key >= 0) - { - if ( - parsedOption.original_tokens.empty() || - (parsedOption.original_tokens.size() == 1 && parsedOption.original_tokens.front().empty()) - ) - continue; // ignore empty options - std::stringstream errorMessage; - errorMessage << "Unrecognized option: "; - for (auto const& token: parsedOption.original_tokens) - errorMessage << token; - BOOST_THROW_EXCEPTION(std::runtime_error(errorMessage.str())); - } + for (auto const& parsedOption: parsedOptions.options) + if (parsedOption.position_key >= 0) + { + if ( + parsedOption.original_tokens.empty() || + (parsedOption.original_tokens.size() == 1 && parsedOption.original_tokens.front().empty()) + ) + continue; // ignore empty options + std::stringstream errorMessage; + errorMessage << "Unrecognized option: "; + for (auto const& token: parsedOption.original_tokens) + errorMessage << token; + BOOST_THROW_EXCEPTION(std::runtime_error(errorMessage.str())); + } + } + catch (po::error const& exception) + { + solThrow(ConfigException, exception.what()); + } if (vmPaths.empty()) { diff --git a/test/boostTest.cpp b/test/boostTest.cpp index 0d3b0eb34..014673eba 100644 --- a/test/boostTest.cpp +++ b/test/boostTest.cpp @@ -222,75 +222,88 @@ bool initializeOptions() // TODO: Prototype -- why isn't this declared in the boost headers? // TODO: replace this with a (global) fixture. -test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] ); +test_suite* init_unit_test_suite(int /*argc*/, char* /*argv*/[]); -test_suite* init_unit_test_suite( int /*argc*/, char* /*argv*/[] ) +test_suite* init_unit_test_suite(int /*argc*/, char* /*argv*/[]) { using namespace solidity::test; master_test_suite_t& master = framework::master_test_suite(); master.p_name.value = "SolidityTests"; - bool shouldContinue = initializeOptions(); - if (!shouldContinue) - exit(0); - - if (!solidity::test::loadVMs(solidity::test::CommonOptions::get())) - exit(1); - - if (solidity::test::CommonOptions::get().disableSemanticTests) - cout << endl << "--- SKIPPING ALL SEMANTICS TESTS ---" << endl << endl; - - if (!solidity::test::CommonOptions::get().enforceGasTest) - cout << endl << "WARNING :: Gas Cost Expectations are not being enforced" << endl << endl; - - Batcher batcher(CommonOptions::get().selectedBatch, CommonOptions::get().batches); - if (CommonOptions::get().batches > 1) - cout << "Batch " << CommonOptions::get().selectedBatch << " out of " << CommonOptions::get().batches << endl; - - // Batch the boost tests - BoostBatcher boostBatcher(batcher); - traverse_test_tree(master, boostBatcher, true); - - // Include the interactive tests in the automatic tests as well - for (auto const& ts: g_interactiveTestsuites) + try { - auto const& options = solidity::test::CommonOptions::get(); + bool shouldContinue = initializeOptions(); + if (!shouldContinue) + exit(0); - if (ts.smt && options.disableSMT) - continue; + if (!solidity::test::loadVMs(solidity::test::CommonOptions::get())) + exit(1); - if (ts.needsVM && solidity::test::CommonOptions::get().disableSemanticTests) - continue; + if (solidity::test::CommonOptions::get().disableSemanticTests) + cout << endl << "--- SKIPPING ALL SEMANTICS TESTS ---" << endl << endl; - //TODO - //solAssert( - registerTests( - master, - options.testPath / ts.path, - ts.subpath, - options.enforceViaYul, - options.enforceCompileToEwasm, - ts.labels, - ts.testCaseCreator, - batcher - ); - // > 0, std::string("no ") + ts.title + " tests found"); + if (!solidity::test::CommonOptions::get().enforceGasTest) + cout << endl << "WARNING :: Gas Cost Expectations are not being enforced" << endl << endl; + + Batcher batcher(CommonOptions::get().selectedBatch, CommonOptions::get().batches); + if (CommonOptions::get().batches > 1) + cout << "Batch " << CommonOptions::get().selectedBatch << " out of " << CommonOptions::get().batches << endl; + + // Batch the boost tests + BoostBatcher boostBatcher(batcher); + traverse_test_tree(master, boostBatcher, true); + + // Include the interactive tests in the automatic tests as well + for (auto const& ts: g_interactiveTestsuites) + { + auto const& options = solidity::test::CommonOptions::get(); + + if (ts.smt && options.disableSMT) + continue; + + if (ts.needsVM && solidity::test::CommonOptions::get().disableSemanticTests) + continue; + + //TODO + //solAssert( + registerTests( + master, + options.testPath / ts.path, + ts.subpath, + options.enforceViaYul, + options.enforceCompileToEwasm, + ts.labels, + ts.testCaseCreator, + batcher + ); + // > 0, std::string("no ") + ts.title + " tests found"); + } + + if (solidity::test::CommonOptions::get().disableSemanticTests) + { + for (auto suite: { + "ABIDecoderTest", + "ABIEncoderTest", + "SolidityAuctionRegistrar", + "SolidityWallet", + "GasMeterTests", + "GasCostTests", + "SolidityEndToEndTest", + "SolidityOptimizer" + }) + removeTestSuite(suite); + } } - - if (solidity::test::CommonOptions::get().disableSemanticTests) + catch (solidity::test::ConfigException const& exception) { - for (auto suite: { - "ABIDecoderTest", - "ABIEncoderTest", - "SolidityAuctionRegistrar", - "SolidityWallet", - "GasMeterTests", - "GasCostTests", - "SolidityEndToEndTest", - "SolidityOptimizer" - }) - removeTestSuite(suite); + cerr << exception.what() << endl; + exit(1); + } + catch (std::runtime_error const& exception) + { + cerr << exception.what() << endl; + exit(1); } return nullptr; From b3048ccf077e3818c2ce9f2e05ad411c3525423d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kamil=20=C5=9Aliwak?= Date: Tue, 30 Nov 2021 14:26:59 +0100 Subject: [PATCH 7/7] Use EXIT_FAILURE and EXIT_SUCCESS constants in exit() and when returning from main() --- test/boostTest.cpp | 8 ++++---- test/tools/isoltest.cpp | 14 +++++++------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/test/boostTest.cpp b/test/boostTest.cpp index 014673eba..b7a4c80f0 100644 --- a/test/boostTest.cpp +++ b/test/boostTest.cpp @@ -235,10 +235,10 @@ test_suite* init_unit_test_suite(int /*argc*/, char* /*argv*/[]) { bool shouldContinue = initializeOptions(); if (!shouldContinue) - exit(0); + exit(EXIT_SUCCESS); if (!solidity::test::loadVMs(solidity::test::CommonOptions::get())) - exit(1); + exit(EXIT_FAILURE); if (solidity::test::CommonOptions::get().disableSemanticTests) cout << endl << "--- SKIPPING ALL SEMANTICS TESTS ---" << endl << endl; @@ -298,12 +298,12 @@ test_suite* init_unit_test_suite(int /*argc*/, char* /*argv*/[]) catch (solidity::test::ConfigException const& exception) { cerr << exception.what() << endl; - exit(1); + exit(EXIT_FAILURE); } catch (std::runtime_error const& exception) { cerr << exception.what() << endl; - exit(1); + exit(EXIT_FAILURE); } return nullptr; diff --git a/test/tools/isoltest.cpp b/test/tools/isoltest.cpp index a2ef564cb..ce45819b8 100644 --- a/test/tools/isoltest.cpp +++ b/test/tools/isoltest.cpp @@ -435,7 +435,7 @@ int main(int argc, char const *argv[]) bool shouldContinue = options->parse(argc, argv); if (!shouldContinue) - return 0; + return EXIT_SUCCESS; options->validate(); CommonOptions::setSingleton(std::move(options)); @@ -444,7 +444,7 @@ int main(int argc, char const *argv[]) auto& options = dynamic_cast(CommonOptions::get()); if (!solidity::test::loadVMs(options)) - return 1; + return EXIT_FAILURE; if (options.disableSemanticTests) cout << endl << "--- SKIPPING ALL SEMANTICS TESTS ---" << endl << endl; @@ -480,7 +480,7 @@ int main(int argc, char const *argv[]) if (stats) global_stats += *stats; else - return 1; + return EXIT_FAILURE; } cout << endl << "Summary: "; @@ -498,22 +498,22 @@ int main(int argc, char const *argv[]) if (options.disableSemanticTests) cout << "\nNOTE: Skipped semantics tests.\n" << endl; - return global_stats ? 0 : 1; + return global_stats ? EXIT_SUCCESS : EXIT_FAILURE; } catch (boost::program_options::error const& exception) { cerr << exception.what() << endl; - return 1; + return EXIT_FAILURE; } catch (std::runtime_error const& exception) { cerr << exception.what() << endl; - return 1; + return EXIT_FAILURE; } catch (...) { cerr << "Unhandled exception caught." << endl; cerr << boost::current_exception_diagnostic_information() << endl; - return 1; + return EXIT_FAILURE; } }