Merge pull request #8422 from imapp-pl/yul-phaser-error-handling

[yul-phaser] Error handling

test/yulPhaser/FitnessMetrics.cpp

@@ -36,7 +36,7 @@ class FitnessMetricFixture
 protected:
 	FitnessMetricFixture():
 		m_sourceStream(SampleSourceCode, ""),
-		m_program(Program::load(m_sourceStream)) {}
+		m_program(get<Program>(Program::load(m_sourceStream))) {}
 
 	static constexpr char SampleSourceCode[] =
 		"{\n"

test/yulPhaser/Program.cpp

@@ -68,7 +68,7 @@ BOOST_AUTO_TEST_CASE(copy_constructor_should_make_deep_copy_of_ast)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	Program programCopy(program);
 
@@ -91,7 +91,7 @@ BOOST_AUTO_TEST_CASE(load_should_rewind_the_stream)
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
 	sourceStream.setPosition(5);
 
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	BOOST_TEST(CodeSize::codeSize(program.ast()) == 2);
 }
@@ -109,7 +109,7 @@ BOOST_AUTO_TEST_CASE(load_should_disambiguate)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	// skipRedundantBlocks() makes the test independent of whether load() includes function grouping or not.
 	Block const& parentBlock = skipRedundantBlocks(program.ast());
@@ -141,7 +141,7 @@ BOOST_AUTO_TEST_CASE(load_should_do_function_grouping_and_hoisting)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	BOOST_TEST(program.ast().statements.size() == 3);
 	BOOST_TEST(holds_alternative<Block>(program.ast().statements[0]));
@@ -159,7 +159,7 @@ BOOST_AUTO_TEST_CASE(load_should_do_loop_init_rewriting)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	// skipRedundantBlocks() makes the test independent of whether load() includes function grouping or not.
 	Block const& parentBlock = skipRedundantBlocks(program.ast());
@@ -172,7 +172,7 @@ BOOST_AUTO_TEST_CASE(load_should_throw_InvalidProgram_if_program_cant_be_parsed)
 	string sourceCode("invalid program\n");
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
 
-	BOOST_CHECK_THROW(Program::load(sourceStream), InvalidProgram);
+	BOOST_TEST(holds_alternative<ErrorList>(Program::load(sourceStream)));
 }
 
 BOOST_AUTO_TEST_CASE(load_should_throw_InvalidProgram_if_program_cant_be_analyzed)
@@ -186,7 +186,7 @@ BOOST_AUTO_TEST_CASE(load_should_throw_InvalidProgram_if_program_cant_be_analyze
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
 
-	BOOST_CHECK_THROW(Program::load(sourceStream), InvalidProgram);
+	BOOST_TEST(holds_alternative<ErrorList>(Program::load(sourceStream)));
 }
 
 BOOST_AUTO_TEST_CASE(optimise)
@@ -200,7 +200,7 @@ BOOST_AUTO_TEST_CASE(optimise)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	[[maybe_unused]] Block const& parentBlockBefore = skipRedundantBlocks(program.ast());
 	assert(parentBlockBefore.statements.size() == 2);
@@ -231,7 +231,7 @@ BOOST_AUTO_TEST_CASE(output_operator)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	// NOTE: The snippet above was chosen so that the few optimisations applied automatically by load()
 	// as of now do not change the code significantly. If that changes, you may have to update it.
@@ -250,7 +250,7 @@ BOOST_AUTO_TEST_CASE(toJson)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	Json::Value parsingResult;
 	string errors;
@@ -270,7 +270,7 @@ BOOST_AUTO_TEST_CASE(codeSize)
 		"}\n"
 	);
 	CharStream sourceStream(sourceCode, current_test_case().p_name);
-	auto program = Program::load(sourceStream);
+	Program program = get<Program>(Program::load(sourceStream));
 
 	BOOST_TEST(program.codeSize() == CodeSize::codeSizeIncludingFunctions(program.ast()));
 }

tools/yulPhaser/Exceptions.h

@@ -22,6 +22,9 @@
 namespace solidity::phaser
 {
 
-struct InvalidProgram: virtual util::Exception {};
+struct BadInput: virtual util::Exception {};
+struct InvalidProgram: virtual BadInput {};
+struct NoInputFiles: virtual BadInput {};
+struct MissingFile: virtual BadInput {};
 
 }

tools/yulPhaser/Phaser.cpp

@@ -125,27 +125,32 @@ ProgramFactory::Options ProgramFactory::Options::fromCommandLine(po::variables_m
 Program ProgramFactory::build(Options const& _options)
 {
 	CharStream sourceCode = loadSource(_options.inputFile);
-	return Program::load(sourceCode);
+	variant<Program, ErrorList> programOrErrors = Program::load(sourceCode);
+	if (holds_alternative<ErrorList>(programOrErrors))
+	{
+		cerr << get<ErrorList>(programOrErrors) << endl;
+		assertThrow(false, InvalidProgram, "Failed to load program " + _options.inputFile);
+	}
+	return move(get<Program>(programOrErrors));
 }
 
 CharStream ProgramFactory::loadSource(string const& _sourcePath)
 {
-	assertThrow(boost::filesystem::exists(_sourcePath), InvalidProgram, "Source file does not exist");
+	assertThrow(boost::filesystem::exists(_sourcePath), MissingFile, "Source file does not exist: " + _sourcePath);
 
 	string sourceCode = readFileAsString(_sourcePath);
 	return CharStream(sourceCode, _sourcePath);
 }
 
-int Phaser::main(int _argc, char** _argv)
+void Phaser::main(int _argc, char** _argv)
 {
-	CommandLineParsingResult parsingResult = parseCommandLine(_argc, _argv);
-	if (parsingResult.exitCode != 0)
-		return parsingResult.exitCode;
+	optional<po::variables_map> arguments = parseCommandLine(_argc, _argv);
+	if (!arguments.has_value())
+		return;
 
-	initialiseRNG(parsingResult.arguments);
+	initialiseRNG(arguments.value());
 
-	runAlgorithm(parsingResult.arguments);
-	return 0;
+	runAlgorithm(arguments.value());
 }
 
 Phaser::CommandLineDescription Phaser::buildCommandLineDescription()
@@ -192,38 +197,27 @@ Phaser::CommandLineDescription Phaser::buildCommandLineDescription()
 	return {keywordDescription, positionalDescription};
 }
 
-Phaser::CommandLineParsingResult Phaser::parseCommandLine(int _argc, char** _argv)
+optional<po::variables_map> Phaser::parseCommandLine(int _argc, char** _argv)
 {
 	auto [keywordDescription, positionalDescription] = buildCommandLineDescription();
 
 	po::variables_map arguments;
 	po::notify(arguments);
 
-	try
-	{
-		po::command_line_parser parser(_argc, _argv);
-		parser.options(keywordDescription).positional(positionalDescription);
-		po::store(parser.run(), arguments);
-	}
-	catch (po::error const & _exception)
-	{
-		cerr << _exception.what() << endl;
-		return {1, move(arguments)};
-	}
+	po::command_line_parser parser(_argc, _argv);
+	parser.options(keywordDescription).positional(positionalDescription);
+	po::store(parser.run(), arguments);
 
 	if (arguments.count("help") > 0)
 	{
 		cout << keywordDescription << endl;
-		return {2, move(arguments)};
+		return nullopt;
 	}
 
 	if (arguments.count("input-file") == 0)
-	{
-		cerr << "Missing argument: input-file." << endl;
-		return {1, move(arguments)};
-	}
+		assertThrow(false, NoInputFiles, "Missing argument: input-file.");
 
-	return {0, arguments};
+	return arguments;
 }
 
 void Phaser::initialiseRNG(po::variables_map const& _arguments)

tools/yulPhaser/Phaser.h

@@ -25,6 +25,7 @@
 
 #include <istream>
 #include <memory>
+#include <optional>
 #include <ostream>
 #include <string>
 
@@ -128,7 +129,7 @@ private:
 class Phaser
 {
 public:
-	static int main(int argc, char** argv);
+	static void main(int argc, char** argv);
 
 private:
 	struct CommandLineDescription
@@ -137,14 +138,8 @@ private:
 		boost::program_options::positional_options_description positionalDescription;
 	};
 
-	struct CommandLineParsingResult
-	{
-		int exitCode;
-		boost::program_options::variables_map arguments;
-	};
-
 	static CommandLineDescription buildCommandLineDescription();
-	static CommandLineParsingResult parseCommandLine(int _argc, char** _argv);
+	static std::optional<boost::program_options::variables_map> parseCommandLine(int _argc, char** _argv);
 	static void initialiseRNG(boost::program_options::variables_map const& _arguments);
 
 	static void runAlgorithm(boost::program_options::variables_map const& _arguments);

tools/yulPhaser/Program.cpp

@@ -17,11 +17,9 @@
 
 #include <tools/yulPhaser/Program.h>
 
-#include <tools/yulPhaser/Exceptions.h>
-
 #include <liblangutil/CharStream.h>
 #include <liblangutil/ErrorReporter.h>
-#include <liblangutil/Exceptions.h>
+#include <liblangutil/SourceReferenceFormatter.h>
 
 #include <libyul/AsmAnalysis.h>
 #include <libyul/AsmAnalysisInfo.h>
@@ -57,6 +55,16 @@ ostream& operator<<(ostream& _stream, Program const& _program);
 
 }
 
+ostream& std::operator<<(ostream& _outputStream, ErrorList const& _errors)
+{
+	SourceReferenceFormatter formatter(_outputStream);
+
+	for (auto const& error: _errors)
+		formatter.printErrorInformation(*error);
+
+	return _outputStream;
+}
+
 Program::Program(Program const& program):
 	m_ast(make_unique<Block>(get<Block>(ASTCopier{}(*program.m_ast)))),
 	m_dialect{program.m_dialect},
@@ -64,16 +72,29 @@ Program::Program(Program const& program):
 {
 }
 
-Program Program::load(CharStream& _sourceCode)
+variant<Program, ErrorList> Program::load(CharStream& _sourceCode)
 {
 	// ASSUMPTION: parseSource() rewinds the stream on its own
 	Dialect const& dialect = EVMDialect::strictAssemblyForEVMObjects(EVMVersion{});
-	unique_ptr<Block> ast = parseSource(dialect, _sourceCode);
-	unique_ptr<AsmAnalysisInfo> analysisInfo = analyzeAST(dialect, *ast);
+
+	variant<unique_ptr<Block>, ErrorList> astOrErrors = parseSource(dialect, _sourceCode);
+	if (holds_alternative<ErrorList>(astOrErrors))
+		return get<ErrorList>(astOrErrors);
+
+	variant<unique_ptr<AsmAnalysisInfo>, ErrorList> analysisInfoOrErrors = analyzeAST(
+		dialect,
+		*get<unique_ptr<Block>>(astOrErrors)
+	);
+	if (holds_alternative<ErrorList>(analysisInfoOrErrors))
+		return get<ErrorList>(analysisInfoOrErrors);
 
 	Program program(
 		dialect,
-		disambiguateAST(dialect, *ast, *analysisInfo)
+		disambiguateAST(
+			dialect,
+			*get<unique_ptr<Block>>(astOrErrors),
+			*get<unique_ptr<AsmAnalysisInfo>>(analysisInfoOrErrors)
+		)
 	);
 	program.optimise({
 		FunctionHoister::name,
@@ -100,7 +121,7 @@ string Program::toJson() const
 	return jsonPrettyPrint(serializedAst);
 }
 
-unique_ptr<Block> Program::parseSource(Dialect const& _dialect, CharStream _source)
+variant<unique_ptr<Block>, ErrorList> Program::parseSource(Dialect const& _dialect, CharStream _source)
 {
 	ErrorList errors;
 	ErrorReporter errorReporter(errors);
@@ -108,13 +129,14 @@ unique_ptr<Block> Program::parseSource(Dialect const& _dialect, CharStream _sour
 	Parser parser(errorReporter, _dialect);
 
 	unique_ptr<Block> ast = parser.parse(scanner, false);
-	assertThrow(ast != nullptr, InvalidProgram, "Error parsing source");
-	assert(errorReporter.errors().empty());
+	if (ast == nullptr)
+		return errors;
 
-	return ast;
+	assert(errorReporter.errors().empty());
+	return variant<unique_ptr<Block>, ErrorList>(move(ast));
 }
 
-unique_ptr<AsmAnalysisInfo> Program::analyzeAST(Dialect const& _dialect, Block const& _ast)
+variant<unique_ptr<AsmAnalysisInfo>, ErrorList> Program::analyzeAST(Dialect const& _dialect, Block const& _ast)
 {
 	ErrorList errors;
 	ErrorReporter errorReporter(errors);
@@ -122,10 +144,11 @@ unique_ptr<AsmAnalysisInfo> Program::analyzeAST(Dialect const& _dialect, Block c
 	AsmAnalyzer analyzer(*analysisInfo, errorReporter, _dialect);
 
 	bool analysisSuccessful = analyzer.analyze(_ast);
-	assertThrow(analysisSuccessful, InvalidProgram, "Error analyzing source");
-	assert(errorReporter.errors().empty());
+	if (!analysisSuccessful)
+		return errors;
 
-	return analysisInfo;
+	assert(errorReporter.errors().empty());
+	return variant<unique_ptr<AsmAnalysisInfo>, ErrorList>(move(analysisInfo));
 }
 
 unique_ptr<Block> Program::disambiguateAST(

tools/yulPhaser/Program.h

@@ -20,10 +20,13 @@
 #include <libyul/optimiser/NameDispenser.h>
 #include <libyul/AsmData.h>
 
+#include <liblangutil/Exceptions.h>
+
 #include <optional>
 #include <ostream>
 #include <set>
 #include <string>
+#include <variant>
 #include <vector>
 
 namespace solidity::langutil
@@ -41,6 +44,13 @@ struct Dialect;
 
 }
 
+namespace std
+{
+
+std::ostream& operator<<(std::ostream& _outputStream, solidity::langutil::ErrorList const& _errors);
+
+}
+
 namespace solidity::phaser
 {
 
@@ -65,7 +75,7 @@ public:
 	Program operator=(Program const& program) = delete;
 	Program operator=(Program&& program) = delete;
 
-	static Program load(langutil::CharStream& _sourceCode);
+	static std::variant<Program, langutil::ErrorList> load(langutil::CharStream& _sourceCode);
 	void optimise(std::vector<std::string> const& _optimisationSteps);
 
 	size_t codeSize() const { return computeCodeSize(*m_ast); }
@@ -84,11 +94,11 @@ private:
 		m_nameDispenser(_dialect, *m_ast, {})
 	{}
 
-	static std::unique_ptr<yul::Block> parseSource(
+	static std::variant<std::unique_ptr<yul::Block>, langutil::ErrorList> parseSource(
 		yul::Dialect const& _dialect,
 		langutil::CharStream _source
 	);
-	static std::unique_ptr<yul::AsmAnalysisInfo> analyzeAST(
+	static std::variant<std::unique_ptr<yul::AsmAnalysisInfo>, langutil::ErrorList> analyzeAST(
 		yul::Dialect const& _dialect,
 		yul::Block const& _ast
 	);

tools/yulPhaser/main.cpp

@@ -18,17 +18,72 @@
 #include <tools/yulPhaser/Exceptions.h>
 #include <tools/yulPhaser/Phaser.h>
 
+#include <libsolutil/Exceptions.h>
+
 #include <iostream>
 
 int main(int argc, char** argv)
 {
 	try
 	{
-		return solidity::phaser::Phaser::main(argc, argv);
+		solidity::phaser::Phaser::main(argc, argv);
+		return 0;
 	}
-	catch (solidity::phaser::InvalidProgram const& exception)
+	catch (boost::program_options::error const& exception)
 	{
+		// Bad input data. Invalid command-line parameters.
+
+		std::cerr << std::endl;
 		std::cerr << "ERROR: " << exception.what() << std::endl;
 		return 1;
 	}
+	catch (solidity::phaser::BadInput const& exception)
+	{
+		// Bad input data. Syntax errors in the input program, semantic errors in command-line
+		// parameters, etc.
+
+		std::cerr << std::endl;
+		std::cerr << "ERROR: " << exception.what() << std::endl;
+		return 1;
+	}
+	catch (solidity::util::Exception const& exception)
+	{
+		// Something's seriously wrong. Probably a bug in the program or a missing handler (which
+		// is really also a bug). The exception should have been handled gracefully by this point
+		// if it's something that can happen in normal usage. E.g. an error in the input or a
+		// failure of some part of the system that's outside of control of the application (disk,
+		// network, etc.). The bug should be reported and investigated so our job here is just to
+		// provide as much useful information about it as possible.
+
+		std::cerr << std::endl;
+		std::cerr << "UNCAUGHT EXCEPTION!" << std::endl;
+
+		// We can print some useful diagnostic info for this particular exception type.
+		std::cerr << "Location: " << exception.lineInfo() << std::endl;
+
+		char const* const* function = boost::get_error_info<boost::throw_function>(exception);
+		if (function != nullptr)
+			std::cerr << "Function: " << *function << std::endl;
+
+		// Let it crash. The terminate() will print some more stuff useful for debugging like
+		// what() and the actual exception type.
+		throw;
+	}
+	catch (std::exception const&)
+	{
+		// Again, probably a bug but this time it's just plain std::exception so there's no point
+		// in doing anything special. terminate() will do an adequate job.
+		std::cerr << std::endl;
+		std::cerr << "UNCAUGHT EXCEPTION!" << std::endl;
+		throw;
+	}
+	catch (...)
+	{
+		// Some people don't believe these exist.
+		// I have no idea what this is and it's flying towards me so technically speaking it's an
+		// unidentified flying object.
+		std::cerr << std::endl;
+		std::cerr << "UFO SPOTTED!" << std::endl;
+		throw;
+	}
 }