Merge pull request #5554 from ethereum/cp-SourceReferenceFormatter-refactor

[2/3] Refactor SourceReferenceFormatter (split-out data extraction & make use of new SourceLocation knowledge)

liblangutil/CMakeLists.txt

@@ -5,6 +5,7 @@ set(sources
 	Exceptions.cpp
 	ParserBase.cpp
 	Scanner.cpp
+	SourceReferenceExtractor.cpp
 	SourceReferenceFormatter.cpp
 	Token.cpp
 )

liblangutil/SourceReferenceExtractor.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/>.
+*/
+#include <liblangutil/SourceReferenceExtractor.h>
+#include <liblangutil/CharStream.h>
+#include <liblangutil/Exceptions.h>
+
+#include <cmath>
+#include <iomanip>
+
+using namespace std;
+using namespace dev;
+using namespace langutil;
+
+SourceReferenceExtractor::Message SourceReferenceExtractor::extract(Exception const& _exception, string _category)
+{
+	SourceLocation const* location = boost::get_error_info<errinfo_sourceLocation>(_exception);
+
+	string const* message = boost::get_error_info<errinfo_comment>(_exception);
+	SourceReference primary = extract(location, message ? *message : "");
+
+	std::vector<SourceReference> secondary;
+	auto secondaryLocation = boost::get_error_info<errinfo_secondarySourceLocation>(_exception);
+	if (secondaryLocation && !secondaryLocation->infos.empty())
+		for (auto const& info: secondaryLocation->infos)
+			secondary.emplace_back(extract(&info.second, info.first));
+
+	return Message{std::move(primary), _category, std::move(secondary)};
+}
+
+SourceReference SourceReferenceExtractor::extract(SourceLocation const* _location, std::string message)
+{
+	if (!_location || !_location->source.get()) // Nothing we can extract here
+		return SourceReference::MessageOnly(std::move(message));
+
+	shared_ptr<CharStream> const& source = _location->source;
+
+	LineColumn const interest = source->translatePositionToLineColumn(_location->start);
+	LineColumn start = interest;
+	LineColumn end = source->translatePositionToLineColumn(_location->end);
+	bool const isMultiline = start.line != end.line;
+
+	string line = source->lineAtPosition(_location->start);
+
+	int locationLength = isMultiline ? line.length() - start.column : end.column - start.column;
+	if (locationLength > 150)
+	{
+		line = line.substr(0, start.column + 35) + " ... " + line.substr(end.column - 35);
+		end.column = start.column + 75;
+		locationLength = 75;
+	}
+
+	if (line.length() > 150)
+	{
+		int const len = line.length();
+		line = line.substr(max(0, start.column - 35), min(start.column, 35) + min(locationLength + 35, len - start.column));
+		if (start.column + locationLength + 35 < len)
+			line += " ...";
+		if (start.column > 35)
+		{
+			line = " ... " + line;
+			start.column = 40;
+		}
+		end.column = start.column + locationLength;
+	}
+
+	return SourceReference{
+		std::move(message),
+		source->name(),
+		interest,
+		isMultiline,
+		line,
+		start.column,
+		end.column,
+	};
+}

liblangutil/SourceReferenceExtractor.h

@@ -0,0 +1,74 @@
+/*
+	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/>.
+*/
+#pragma once
+
+#include <iosfwd>
+#include <string>
+#include <tuple>
+#include <vector>
+
+namespace dev
+{
+struct Exception;
+}
+
+namespace langutil
+{
+
+struct LineColumn
+{
+	int line;
+	int column;
+
+	LineColumn(std::tuple<int, int> const& _t): line{std::get<0>(_t)}, column{std::get<1>(_t)} {}
+	LineColumn(int _line, int _column): line{_line}, column{_column} {}
+	LineColumn(): line{-1}, column{-1} {}
+};
+
+struct SourceReference
+{
+	std::string message;    ///< A message that relates to this source reference (such as a warning or an error message).
+	std::string sourceName; ///< Underlying source name (for example the filename).
+	LineColumn position;    ///< Actual (error) position this source reference is surrounding.
+	bool multiline;         ///< Indicates whether the actual SourceReference is truncated to one line.
+	std::string text;       ///< Extracted source code text (potentially truncated if multiline or too long).
+	int startColumn;        ///< Highlighting range-start of text field.
+	int endColumn;          ///< Highlighting range-end of text field.
+
+	/// Constructs a SourceReference containing a message only.
+	static SourceReference MessageOnly(std::string _msg)
+	{
+		return SourceReference{std::move(_msg), "", LineColumn{-1, -1}, false, "", -1, -1};
+	}
+};
+
+struct SourceLocation;
+
+namespace SourceReferenceExtractor
+{
+	struct Message
+	{
+		SourceReference primary;
+		std::string category; // "Error", "Warning", ...
+		std::vector<SourceReference> secondary;
+	};
+
+	Message extract(dev::Exception const& _exception, std::string _category);
+	SourceReference extract(SourceLocation const* _location, std::string message = "");
+}
+
+}

liblangutil/SourceReferenceFormatter.cpp

@@ -30,100 +30,63 @@ using namespace langutil;
 
 void SourceReferenceFormatter::printSourceLocation(SourceLocation const* _location)
 {
-	if (!_location || !_location->source)
+	printSourceLocation(SourceReferenceExtractor::extract(_location));
+}
+
+void SourceReferenceFormatter::printSourceLocation(SourceReference const& _ref)
+{
+	if (_ref.position.line < 0)
 		return; // Nothing we can print here
-	auto const& scanner = m_scannerFromSourceName(_location->source->name());
+
-	int startLine;
+	if (!_ref.multiline)
-	int startColumn;
-	tie(startLine, startColumn) = scanner.translatePositionToLineColumn(_location->start);
-	int endLine;
-	int endColumn;
-	tie(endLine, endColumn) = scanner.translatePositionToLineColumn(_location->end);
-	if (startLine == endLine)
 	{
-		string line = scanner.lineAtPosition(_location->start);
+		m_stream << _ref.text << endl;
-
-		int locationLength = endColumn - startColumn;
-		if (locationLength > 150)
-		{
-			line = line.substr(0, startColumn + 35) + " ... " + line.substr(endColumn - 35);
-			endColumn = startColumn + 75;
-			locationLength = 75;
-		}
-		if (line.length() > 150)
-		{
-			int len = line.length();
-			line = line.substr(max(0, startColumn - 35), min(startColumn, 35) + min(locationLength + 35, len - startColumn));
-			if (startColumn + locationLength + 35 < len)
-				line += " ...";
-			if (startColumn > 35)
-			{
-				line = " ... " + line;
-				startColumn = 40;
-			}
-			endColumn = startColumn + locationLength;
-		}
-
-		m_stream << line << endl;
 
+		// mark the text-range like this: ^-----^
 		for_each(
-			line.cbegin(),
+			_ref.text.cbegin(),
-			line.cbegin() + startColumn,
+			_ref.text.cbegin() + _ref.startColumn,
-			[this](char const& ch) { m_stream << (ch == '\t' ? '\t' : ' '); }
+			[this](char ch) { m_stream << (ch == '\t' ? '\t' : ' '); }
 		);
 		m_stream << "^";
-		if (endColumn > startColumn + 2)
+		if (_ref.endColumn > _ref.startColumn + 2)
-			m_stream << string(endColumn - startColumn - 2, '-');
+			m_stream << string(_ref.endColumn - _ref.startColumn - 2, '-');
-		if (endColumn > startColumn + 1)
+		if (_ref.endColumn > _ref.startColumn + 1)
 			m_stream << "^";
 		m_stream << endl;
 	}
 	else
 		m_stream <<
-			scanner.lineAtPosition(_location->start) <<
+			_ref.text <<
 			endl <<
-			string(startColumn, ' ') <<
+			string(_ref.startColumn, ' ') <<
 			"^ (Relevant source part starts here and spans across multiple lines)." <<
 			endl;
 }
 
-void SourceReferenceFormatter::printSourceName(SourceLocation const* _location)
+void SourceReferenceFormatter::printSourceName(SourceReference const& _ref)
 {
-	if (!_location || !_location->source)
+	if (_ref.position.line != -1)
-		return; // Nothing we can print here
+		m_stream << _ref.sourceName << ":" << (_ref.position.line + 1) << ":" << (_ref.position.column + 1) << ": ";
-	auto const& scanner = m_scannerFromSourceName(_location->source->name());
-	int startLine;
-	int startColumn;
-	tie(startLine, startColumn) = scanner.translatePositionToLineColumn(_location->start);
-	m_stream << _location->source->name() << ":" << (startLine + 1) << ":" << (startColumn + 1) << ": ";
 }
 
-void SourceReferenceFormatter::printExceptionInformation(
+void SourceReferenceFormatter::printExceptionInformation(dev::Exception const& _error, std::string const& _category)
-	dev::Exception const& _exception,
-	string const& _name
-)
 {
-	SourceLocation const* location = boost::get_error_info<errinfo_sourceLocation>(_exception);
+	printExceptionInformation(SourceReferenceExtractor::extract(_error, _category));
-	auto secondarylocation = boost::get_error_info<errinfo_secondarySourceLocation>(_exception);
+}
 
-	printSourceName(location);
+void SourceReferenceFormatter::printExceptionInformation(SourceReferenceExtractor::Message const& _msg)
+{
+	printSourceName(_msg.primary);
 
-	m_stream << _name;
+	m_stream << _msg.category << ": " << _msg.primary.message << endl;
-	if (string const* description = boost::get_error_info<errinfo_comment>(_exception))
-		m_stream << ": " << *description << endl;
-	else
-		m_stream << endl;
 
-	printSourceLocation(location);
+	printSourceLocation(_msg.primary);
 
-	if (secondarylocation && !secondarylocation->infos.empty())
+	for (auto const& ref: _msg.secondary)
 	{
-		for (auto info: secondarylocation->infos)
+		printSourceName(ref);
-		{
+		m_stream << ref.message << endl;
-			printSourceName(&info.second);
+		printSourceLocation(ref);
-			m_stream << info.first << endl;
-			printSourceLocation(&info.second);
-		}
-		m_stream << endl;
 	}
 }

liblangutil/SourceReferenceFormatter.h

@@ -25,6 +25,7 @@
 #include <ostream>
 #include <sstream>
 #include <functional>
+#include <liblangutil/SourceReferenceExtractor.h>
 
 namespace dev
 {
@@ -39,38 +40,33 @@ class Scanner;
 class SourceReferenceFormatter
 {
 public:
-	using ScannerFromSourceNameFun = std::function<langutil::Scanner const&(std::string const&)>;
+	explicit SourceReferenceFormatter(std::ostream& _stream):
-
+		m_stream(_stream)
-	explicit SourceReferenceFormatter(
-		std::ostream& _stream,
-		ScannerFromSourceNameFun _scannerFromSourceName
-	):
-		m_stream(_stream),
-		m_scannerFromSourceName(std::move(_scannerFromSourceName))
 	{}
 
 	/// Prints source location if it is given.
-	void printSourceLocation(langutil::SourceLocation const* _location);
+	void printSourceLocation(SourceLocation const* _location);
-	void printExceptionInformation(dev::Exception const& _exception, std::string const& _name);
+	void printSourceLocation(SourceReference const& _ref);
+	void printExceptionInformation(dev::Exception const& _error, std::string const& _category);
+	void printExceptionInformation(SourceReferenceExtractor::Message const& _msg);
 
 	static std::string formatExceptionInformation(
 		dev::Exception const& _exception,
-		std::string const& _name,
+		std::string const& _name
-		ScannerFromSourceNameFun const& _scannerFromSourceName
 	)
 	{
 		std::ostringstream errorOutput;
 
-		SourceReferenceFormatter formatter(errorOutput, _scannerFromSourceName);
+		SourceReferenceFormatter formatter(errorOutput);
 		formatter.printExceptionInformation(_exception, _name);
 		return errorOutput.str();
 	}
+
 private:
 	/// Prints source name if location is given.
-	void printSourceName(langutil::SourceLocation const* _location);
+	void printSourceName(SourceReference const& _ref);
 
 	std::ostream& m_stream;
-	ScannerFromSourceNameFun m_scannerFromSourceName;
 };
 
 }

libsolidity/codegen/CompilerContext.cpp

@@ -386,8 +386,7 @@ void CompilerContext::appendInlineAssembly(
 		for (auto const& error: errorReporter.errors())
 			message += SourceReferenceFormatter::formatExceptionInformation(
 				*error,
-				(error->type() == Error::Type::Warning) ? "Warning" : "Error",
+				(error->type() == Error::Type::Warning) ? "Warning" : "Error"
-				[&](string const&) -> Scanner const& { return *scanner; }
 			);
 		message += "-------------------------------------------\n";
 

libsolidity/interface/StandardCompiler.cpp

@@ -69,12 +69,11 @@ Json::Value formatErrorWithException(
 	bool const& _warning,
 	string const& _type,
 	string const& _component,
-	string const& _message,
+	string const& _message
-	function<Scanner const&(string const&)> const& _scannerFromSourceName
 )
 {
 	string message;
-	string formattedMessage = SourceReferenceFormatter::formatExceptionInformation(_exception, _type, _scannerFromSourceName);
+	string formattedMessage = SourceReferenceFormatter::formatExceptionInformation(_exception, _type);
 
 	// NOTE: the below is partially a copy from SourceReferenceFormatter
 	SourceLocation const* location = boost::get_error_info<errinfo_sourceLocation>(_exception);
@@ -433,8 +432,6 @@ Json::Value StandardCompiler::compileInternal(Json::Value const& _input)
 	Json::Value outputSelection = settings.get("outputSelection", Json::Value());
 	m_compilerStack.setRequestedContractNames(requestedContractNames(outputSelection));
 
-	auto scannerFromSourceName = [&](string const& _sourceName) -> Scanner const& { return m_compilerStack.scanner(_sourceName); };
-
 	try
 	{
 		m_compilerStack.compile();
@@ -448,8 +445,7 @@ Json::Value StandardCompiler::compileInternal(Json::Value const& _input)
 				err.type() == Error::Type::Warning,
 				err.typeName(),
 				"general",
-				"",
+				""
-				scannerFromSourceName
 			));
 		}
 	}
@@ -461,8 +457,7 @@ Json::Value StandardCompiler::compileInternal(Json::Value const& _input)
 			false,
 			_error.typeName(),
 			"general",
-			"Uncaught error: ",
+			"Uncaught error: "
-			scannerFromSourceName
 		));
 	}
 	/// This should not be leaked from compile().
@@ -482,8 +477,7 @@ Json::Value StandardCompiler::compileInternal(Json::Value const& _input)
 			false,
 			"CompilerError",
 			"general",
-			"Compiler error (" + _exception.lineInfo() + ")",
+			"Compiler error (" + _exception.lineInfo() + ")"
-			scannerFromSourceName
 		));
 	}
 	catch (InternalCompilerError const& _exception)
@@ -493,8 +487,7 @@ Json::Value StandardCompiler::compileInternal(Json::Value const& _input)
 			false,
 			"InternalCompilerError",
 			"general",
-			"Internal compiler error (" + _exception.lineInfo() + ")",
+			"Internal compiler error (" + _exception.lineInfo() + ")"
-			scannerFromSourceName
 		));
 	}
 	catch (UnimplementedFeatureError const& _exception)
@@ -504,8 +497,7 @@ Json::Value StandardCompiler::compileInternal(Json::Value const& _input)
 			false,
 			"UnimplementedFeatureError",
 			"general",
-			"Unimplemented feature (" + _exception.lineInfo() + ")",
+			"Unimplemented feature (" + _exception.lineInfo() + ")"
-			scannerFromSourceName
 		));
 	}
 	catch (Exception const& _exception)

solc/CommandLineInterface.cpp

@@ -858,8 +858,7 @@ bool CommandLineInterface::processInput()
 
 	m_compiler.reset(new CompilerStack(fileReader));
 
-	auto scannerFromSourceName = [&](string const& _sourceName) -> Scanner const& { return m_compiler->scanner(_sourceName); };
+	SourceReferenceFormatter formatter(serr(false));
-	SourceReferenceFormatter formatter(serr(false), scannerFromSourceName);
 
 	try
 	{
@@ -1222,8 +1221,7 @@ bool CommandLineInterface::assemble(
 	for (auto const& sourceAndStack: assemblyStacks)
 	{
 		auto const& stack = sourceAndStack.second;
-		auto scannerFromSourceName = [&](string const&) -> Scanner const& { return stack.scanner(); };
+		SourceReferenceFormatter formatter(serr(false));
-		SourceReferenceFormatter formatter(serr(false), scannerFromSourceName);
 
 		for (auto const& error: stack.errors())
 		{

test/libsolidity/AnalysisFramework.cpp

@@ -127,8 +127,7 @@ string AnalysisFramework::formatError(Error const& _error) const
 {
 	return SourceReferenceFormatter::formatExceptionInformation(
 			_error,
-			(_error.type() == Error::Type::Warning) ? "Warning" : "Error",
+			(_error.type() == Error::Type::Warning) ? "Warning" : "Error"
-			[&](std::string const& _sourceName) -> Scanner const& { return m_compiler.scanner(_sourceName); }
 		);
 }
 

test/libsolidity/GasMeter.cpp

@@ -138,8 +138,7 @@ BOOST_AUTO_TEST_CASE(non_overlapping_filtered_costs)
 			if (first->first->location().intersects(second->first->location()))
 			{
 				BOOST_CHECK_MESSAGE(false, "Source locations should not overlap!");
-				auto scannerFromSource = [&](string const& _sourceName) -> Scanner const& { return m_compiler.scanner(_sourceName); };
+				SourceReferenceFormatter formatter(cout);
-				SourceReferenceFormatter formatter(cout, scannerFromSource);
 
 				formatter.printSourceLocation(&first->first->location());
 				formatter.printSourceLocation(&second->first->location());

test/libsolidity/SolidityExecutionFramework.h

@@ -72,8 +72,7 @@ public:
 		m_compiler.setOptimiserSettings(m_optimize, m_optimizeRuns);
 		if (!m_compiler.compile())
 		{
-			auto scannerFromSourceName = [&](std::string const& _sourceName) -> langutil::Scanner const& { return m_compiler.scanner(_sourceName); };
+			langutil::SourceReferenceFormatter formatter(std::cerr);
-			langutil::SourceReferenceFormatter formatter(std::cerr, scannerFromSourceName);
 
 			for (auto const& error: m_compiler.errors())
 				formatter.printExceptionInformation(

test/libyul/Common.cpp

@@ -40,9 +40,9 @@ using namespace langutil;
 using namespace yul;
 using namespace dev::solidity;
 
-void yul::test::printErrors(ErrorList const& _errors, Scanner const& _scanner)
+void yul::test::printErrors(ErrorList const& _errors)
 {
-	SourceReferenceFormatter formatter(cout, [&](std::string const&) -> Scanner const& { return _scanner; });
+	SourceReferenceFormatter formatter(cout);
 
 	for (auto const& error: _errors)
 		formatter.printExceptionInformation(
@@ -76,7 +76,7 @@ pair<shared_ptr<Block>, shared_ptr<yul::AsmAnalysisInfo>> yul::test::parse(strin
 			return make_pair(parserResult, analysisInfo);
 		}
 	}
-	printErrors(errors, *scanner);
+	printErrors(errors);
 	BOOST_FAIL("Invalid source.");
 
 	// Unreachable.

test/libyul/Common.h

@@ -29,7 +29,6 @@
 
 namespace langutil
 {
-class Scanner;
 class Error;
 using ErrorList = std::vector<std::shared_ptr<Error const>>;
 }
@@ -44,7 +43,7 @@ namespace yul
 namespace test
 {
 
-void printErrors(langutil::ErrorList const& _errors, langutil::Scanner const& _scanner);
+void printErrors(langutil::ErrorList const& _errors);
 std::pair<std::shared_ptr<Block>, std::shared_ptr<AsmAnalysisInfo>>
 parse(std::string const& _source, bool _yul = true);
 Block disambiguate(std::string const& _source, bool _yul = true);

test/libyul/YulOptimizerTest.cpp

@@ -264,7 +264,7 @@ bool YulOptimizerTest::parse(ostream& _stream, string const& _linePrefix, bool c
 	if (!m_ast || !errorReporter.errors().empty())
 	{
 		FormattedScope(_stream, _formatted, {formatting::BOLD, formatting::RED}) << _linePrefix << "Error parsing source." << endl;
-		printErrors(_stream, errorReporter.errors(), *scanner);
+		printErrors(_stream, errorReporter.errors());
 		return false;
 	}
 	m_analysisInfo = make_shared<yul::AsmAnalysisInfo>();
@@ -278,7 +278,7 @@ bool YulOptimizerTest::parse(ostream& _stream, string const& _linePrefix, bool c
 	if (!analyzer.analyze(*m_ast) || !errorReporter.errors().empty())
 	{
 		FormattedScope(_stream, _formatted, {formatting::BOLD, formatting::RED}) << _linePrefix << "Error analyzing source." << endl;
-		printErrors(_stream, errorReporter.errors(), *scanner);
+		printErrors(_stream, errorReporter.errors());
 		return false;
 	}
 	return true;
@@ -290,9 +290,9 @@ void YulOptimizerTest::disambiguate()
 	m_analysisInfo.reset();
 }
 
-void YulOptimizerTest::printErrors(ostream& _stream, ErrorList const& _errors, Scanner const& _scanner)
+void YulOptimizerTest::printErrors(ostream& _stream, ErrorList const& _errors)
 {
-	SourceReferenceFormatter formatter(_stream, [&](string const&) -> Scanner const& { return _scanner; });
+	SourceReferenceFormatter formatter(_stream);
 
 	for (auto const& error: _errors)
 		formatter.printExceptionInformation(

test/libyul/YulOptimizerTest.h

@@ -57,7 +57,7 @@ private:
 	bool parse(std::ostream& _stream, std::string const& _linePrefix, bool const _formatted);
 	void disambiguate();
 
-	static void printErrors(std::ostream& _stream, langutil::ErrorList const& _errors, langutil::Scanner const& _scanner);
+	static void printErrors(std::ostream& _stream, langutil::ErrorList const& _errors);
 
 	std::string m_source;
 	bool m_yul = false;

test/tools/yulopti.cpp

@@ -67,9 +67,9 @@ namespace po = boost::program_options;
 class YulOpti
 {
 public:
-	void printErrors(Scanner const& _scanner)
+	void printErrors()
 	{
-		SourceReferenceFormatter formatter(cout, [&](string const&) -> Scanner const& { return _scanner; });
+		SourceReferenceFormatter formatter(cout);
 
 		for (auto const& error: m_errors)
 			formatter.printExceptionInformation(
@@ -86,7 +86,7 @@ public:
 		if (!m_ast || !errorReporter.errors().empty())
 		{
 			cout << "Error parsing source." << endl;
-			printErrors(*scanner);
+			printErrors();
 			return false;
 		}
 		m_analysisInfo = make_shared<yul::AsmAnalysisInfo>();
@@ -100,7 +100,7 @@ public:
 		if (!analyzer.analyze(*m_ast) || !errorReporter.errors().empty())
 		{
 			cout << "Error analyzing source." << endl;
-			printErrors(*scanner);
+			printErrors();
 			return false;
 		}
 		return true;