Merge pull request #14482 from ethereum/fix-error-ids-in-format-errors

Fix error IDs in `AnalysisFramework::formatErrors()`
ethereum · Aug 11, 2023 · c50c9b2 · c50c9b2
2 parents 3edf91a + 339053f
commit c50c9b2
Show file tree

Hide file tree

Showing 14 changed files with 58 additions and 54 deletions.
diff --git a/liblangutil/SourceReferenceFormatter.h b/liblangutil/SourceReferenceFormatter.h
@@ -46,9 +46,16 @@ class SourceReferenceFormatter
 		bool _colored,
 		bool _withErrorIds
 	):
-		m_stream(_stream), m_charStreamProvider(_charStreamProvider), m_colored(_colored), m_withErrorIds(_withErrorIds)
+		m_stream(_stream),
+		m_charStreamProvider(_charStreamProvider),
+		m_colored(_colored),
+		m_withErrorIds(_withErrorIds)
 	{}
 
+	// WARNING: Use the xyzErrorInformation() variants over xyzExceptionInformation() when you
+	// do have access to an Error instance. Error is implicitly convertible to util::Exception
+	// but the conversion loses the error ID.
+
 	/// Prints source location if it is given.
 	void printSourceLocation(SourceReference const& _ref);
 	void printExceptionInformation(SourceReferenceExtractor::Message const& _msg);
@@ -61,12 +68,11 @@ class SourceReferenceFormatter
 		util::Exception const& _exception,
 		Error::Type _type,
 		CharStreamProvider const& _charStreamProvider,
-		bool _colored = false,
-		bool _withErrorIds = false
+		bool _colored = false
 	)
 	{
 		std::ostringstream errorOutput;
-		SourceReferenceFormatter formatter(errorOutput, _charStreamProvider, _colored, _withErrorIds);
+		SourceReferenceFormatter formatter(errorOutput, _charStreamProvider, _colored, false /* _withErrorIds */);
 		formatter.printExceptionInformation(_exception, _type);
 		return errorOutput.str();
 	}
@@ -75,26 +81,39 @@ class SourceReferenceFormatter
 		util::Exception const& _exception,
 		Error::Severity _severity,
 		CharStreamProvider const& _charStreamProvider,
+		bool _colored = false
+	)
+	{
+		std::ostringstream errorOutput;
+		SourceReferenceFormatter formatter(errorOutput, _charStreamProvider, _colored, false /* _withErrorIds */);
+		formatter.printExceptionInformation(_exception, _severity);
+		return errorOutput.str();
+	}
+
+	static std::string formatErrorInformation(
+		Error const& _error,
+		CharStreamProvider const& _charStreamProvider,
 		bool _colored = false,
 		bool _withErrorIds = false
 	)
 	{
 		std::ostringstream errorOutput;
 		SourceReferenceFormatter formatter(errorOutput, _charStreamProvider, _colored, _withErrorIds);
-		formatter.printExceptionInformation(_exception, _severity);
+		formatter.printErrorInformation(_error);
 		return errorOutput.str();
 	}
 
 	static std::string formatErrorInformation(
-		Error const& _error,
-		CharStreamProvider const& _charStreamProvider
+		langutil::ErrorList const& _errors,
+		CharStreamProvider const& _charStreamProvider,
+		bool _colored = false,
+		bool _withErrorIds = false
 	)
 	{
-		return formatExceptionInformation(
-			_error,
-			Error::errorSeverity(_error.type()),
-			_charStreamProvider
-		);
+		std::ostringstream errorOutput;
+		SourceReferenceFormatter formatter(errorOutput, _charStreamProvider, _colored, _withErrorIds);
+		formatter.printErrorInformation(_errors);
+		return errorOutput.str();
 	}
 
 	static std::string formatErrorInformation(Error const& _error, CharStream const& _charStream);

diff --git a/libsolidity/codegen/ir/IRGenerator.cpp b/libsolidity/codegen/ir/IRGenerator.cpp
@@ -38,8 +38,6 @@
 #include <libsolutil/StringUtils.h>
 #include <libsolutil/Whiskers.h>
 
-#include <liblangutil/SourceReferenceFormatter.h>
-
 #include <json/json.h>
 
 #include <sstream>

diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp
@@ -1486,16 +1486,13 @@ void CompilerStack::generateIR(ContractDefinition const& _contract)
 		m_optimiserSettings,
 		m_debugInfoSelection
 	);
-	if (!stack.parseAndAnalyze("", compiledContract.yulIR))
-	{
-		string errorMessage;
-		for (auto const& error: stack.errors())
-			errorMessage += langutil::SourceReferenceFormatter::formatErrorInformation(
-				*error,
-				stack.charStream("")
-			);
-		solAssert(false, compiledContract.yulIR + "\n\nInvalid IR generated:\n" + errorMessage + "\n");
-	}
+	bool yulAnalysisSuccessful = stack.parseAndAnalyze("", compiledContract.yulIR);
+	solAssert(
+		yulAnalysisSuccessful,
+		compiledContract.yulIR + "\n\n"
+		"Invalid IR generated:\n" +
+		langutil::SourceReferenceFormatter::formatErrorInformation(stack.errors(), stack) + "\n"
+	);
 
 	compiledContract.yulIRAst = stack.astJson();
 	stack.optimize();

diff --git a/libsolidity/interface/StandardCompiler.cpp b/libsolidity/interface/StandardCompiler.cpp
@@ -125,8 +125,7 @@ Json::Value formatErrorWithException(
 		_exception,
 		_type,
 		_charStreamProvider,
-		false, // colored
-		false // _withErrorIds
+		false // colored
 	);
 
 	if (string const* description = _exception.comment())

diff --git a/solc/CommandLineInterface.cpp b/solc/CommandLineInterface.cpp
@@ -827,7 +827,7 @@ void CommandLineInterface::compile()
 		else
 		{
 			m_hasOutput = true;
-			formatter.printExceptionInformation(_error, Error::errorSeverity(_error.type()));
+			formatter.printErrorInformation(_error);
 			solThrow(CommandLineExecutionError, "");
 		}
 	}

diff --git a/test/libsolidity/AnalysisFramework.cpp b/test/libsolidity/AnalysisFramework.cpp
@@ -147,15 +147,17 @@ ErrorList AnalysisFramework::expectError(std::string const& _source, bool _warni
 }
 
 string AnalysisFramework::formatErrors(
-	langutil::ErrorList _errors,
+	langutil::ErrorList const& _errors,
 	bool _colored,
 	bool _withErrorIds
 ) const
 {
-	string message;
-	for (auto const& error: _errors)
-		message += formatError(*error, _colored, _withErrorIds);
-	return message;
+	return SourceReferenceFormatter::formatErrorInformation(
+		_errors,
+		*m_compiler,
+		_colored,
+		_withErrorIds
+	);
 }
 
 string AnalysisFramework::formatError(
@@ -164,9 +166,8 @@ string AnalysisFramework::formatError(
 	bool _withErrorIds
 ) const
 {
-	return SourceReferenceFormatter::formatExceptionInformation(
+	return SourceReferenceFormatter::formatErrorInformation(
 		_error,
-		_error.type(),
 		*m_compiler,
 		_colored,
 		_withErrorIds

diff --git a/test/libsolidity/AnalysisFramework.h b/test/libsolidity/AnalysisFramework.h
@@ -58,7 +58,7 @@ class AnalysisFramework
 	langutil::ErrorList expectError(std::string const& _source, bool _warning = false, bool _allowMultiple = false);
 
 	std::string formatErrors(
-		langutil::ErrorList _errors,
+		langutil::ErrorList const& _errors,
 		bool _colored = false,
 		bool _withErrorIds = false
 	) const;

diff --git a/test/libsolidity/GasTest.cpp b/test/libsolidity/GasTest.cpp
@@ -20,7 +20,6 @@
 #include <test/Common.h>
 #include <libsolutil/CommonIO.h>
 #include <libsolutil/JSON.h>
-#include <liblangutil/SourceReferenceFormatter.h>
 #include <boost/algorithm/string.hpp>
 #include <boost/algorithm/string/predicate.hpp>
 #include <boost/filesystem.hpp>

diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp
@@ -85,12 +85,10 @@ std::optional<Error> parseAndReturnFirstError(
 		if (_allowWarnings && e->type() == Error::Type::Warning)
 			continue;
 		if (error)
-		{
-			string errors;
-			for (auto const& err: stack.errors())
-				errors += SourceReferenceFormatter::formatErrorInformation(*err, stack);
-			BOOST_FAIL("Found more than one error:\n" + errors);
-		}
+			BOOST_FAIL(
+				"Found more than one error:\n" +
+				SourceReferenceFormatter::formatErrorInformation(stack.errors(), stack)
+			);
 		error = e;
 	}
 	if (!success)

diff --git a/test/libyul/Common.cpp b/test/libyul/Common.cpp
@@ -33,7 +33,6 @@
 #include <liblangutil/DebugInfoSelection.h>
 #include <liblangutil/ErrorReporter.h>
 #include <liblangutil/Scanner.h>
-#include <liblangutil/SourceReferenceFormatter.h>
 
 #include <boost/test/unit_test.hpp>
 

diff --git a/test/libyul/ControlFlowGraphTest.cpp b/test/libyul/ControlFlowGraphTest.cpp
@@ -24,7 +24,6 @@
 #include <libyul/backends/evm/ControlFlowGraphBuilder.h>
 #include <libyul/backends/evm/StackHelpers.h>
 #include <libyul/Object.h>
-#include <liblangutil/SourceReferenceFormatter.h>
 
 #include <libsolutil/AnsiColorized.h>
 #include <libsolutil/Visitor.h>

diff --git a/test/tools/ossfuzz/SolidityEvmoneInterface.cpp b/test/tools/ossfuzz/SolidityEvmoneInterface.cpp
@@ -42,11 +42,10 @@ optional<CompilerOutput> SolidityCompilationFramework::compileContract()
 		if (m_compilerInput.debugFailure)
 		{
 			cerr << "Compiling contract failed" << endl;
-			for (auto const& error: m_compiler.errors())
-				cerr << SourceReferenceFormatter::formatErrorInformation(
-					*error,
-					m_compiler
-				);
+			cerr << SourceReferenceFormatter::formatErrorInformation(
+				m_compiler.errors(),
+				m_compiler
+			);
 		}
 		return {};
 	}

diff --git a/test/tools/ossfuzz/yulProto_diff_ossfuzz.cpp b/test/tools/ossfuzz/yulProto_diff_ossfuzz.cpp
@@ -77,10 +77,7 @@ DEFINE_PROTO_FUZZER(Program const& _input)
 		Error::containsErrors(stack.errors())
 	)
 	{
-		SourceReferenceFormatter formatter(std::cout, stack, false, false);
-
-		for (auto const& error: stack.errors())
-			formatter.printExceptionInformation(*error, Error::errorSeverity(error->type()));
+		SourceReferenceFormatter{std::cout, stack, false, false}.printErrorInformation(stack.errors());
 		yulAssert(false, "Proto fuzzer generated malformed program");
 	}
 

diff --git a/tools/yulPhaser/Program.cpp b/tools/yulPhaser/Program.cpp
@@ -20,7 +20,6 @@
 
 #include <liblangutil/CharStream.h>
 #include <liblangutil/ErrorReporter.h>
-#include <liblangutil/SourceReferenceFormatter.h>
 
 #include <libyul/AsmAnalysis.h>
 #include <libyul/AsmAnalysisInfo.h>