Catch and report UnimplementedFeatureError

Changelog.md

@@ -4,6 +4,7 @@ Language Features:
 
 
 Compiler Features:
+ * Error Reporting: Unimplemented features are now properly reported as errors instead of being handled as if they were bugs.
  * EVM: Support for the EVM version "Prague".
  * SMTChecker: Add CHC engine check for underflow and overflow in unary minus operation.
  * SMTChecker: Replace CVC4 as a possible BMC backend with cvc5.

liblangutil/ErrorReporter.cpp

@@ -263,6 +263,16 @@ void ErrorReporter::docstringParsingError(ErrorId _error, SourceLocation const&
 	);
 }
 
+void ErrorReporter::unimplementedFeatureError(ErrorId _error, SourceLocation const& _location, std::string const& _description)
+{
+	error(
+		_error,
+		Error::Type::UnimplementedFeatureError,
+		_location,
+		_description
+	);
+}
+
 void ErrorReporter::info(
 	ErrorId _error,
 	SourceLocation const& _location,

liblangutil/ErrorReporter.h

@@ -118,6 +118,8 @@ class ErrorReporter
 
 	void docstringParsingError(ErrorId _error, SourceLocation const& _location, std::string const& _description);
 
+	void unimplementedFeatureError(ErrorId _error, SourceLocation const& _location, std::string const& _description);
+
 	ErrorList const& errors() const;
 
 	void clear();

libsolidity/interface/CompilerStack.cpp

@@ -346,64 +346,71 @@ bool CompilerStack::parse()
 	if (SemVerVersion{std::string(VersionString)}.isPrerelease())
 		m_errorReporter.warning(3805_error, "This is a pre-release compiler version, please do not use it in production.");
 
-	Parser parser{m_errorReporter, m_evmVersion};
+	try
+	{
+		Parser parser{m_errorReporter, m_evmVersion};
 
-	std::vector<std::string> sourcesToParse;
-	for (auto const& s: m_sources)
-		sourcesToParse.push_back(s.first);
+		std::vector<std::string> sourcesToParse;
+		for (auto const& s: m_sources)
+			sourcesToParse.push_back(s.first);
 
-	for (size_t i = 0; i < sourcesToParse.size(); ++i)
-	{
-		std::string const& path = sourcesToParse[i];
-		Source& source = m_sources[path];
-		source.ast = parser.parse(*source.charStream);
-		if (!source.ast)
-			solAssert(Error::containsErrors(m_errorReporter.errors()), "Parser returned null but did not report error.");
-		else
+		for (size_t i = 0; i < sourcesToParse.size(); ++i)
 		{
-			source.ast->annotation().path = path;
-
-			for (auto const& import: ASTNode::filteredNodes<ImportDirective>(source.ast->nodes()))
+			std::string const& path = sourcesToParse[i];
+			Source& source = m_sources[path];
+			source.ast = parser.parse(*source.charStream);
+			if (!source.ast)
+				solAssert(Error::containsErrors(m_errorReporter.errors()), "Parser returned null but did not report error.");
+			else
 			{
-				solAssert(!import->path().empty(), "Import path cannot be empty.");
-				// Check whether the import directive is for the standard library,
-				// and if yes, add specified file to source units to be parsed.
-				auto it = stdlib::sources.find(import->path());
-				if (it != stdlib::sources.end())
+				source.ast->annotation().path = path;
+
+				for (auto const& import: ASTNode::filteredNodes<ImportDirective>(source.ast->nodes()))
 				{
-					auto [name, content] = *it;
-					m_sources[name].charStream = std::make_unique<CharStream>(content, name);
-					sourcesToParse.push_back(name);
+					solAssert(!import->path().empty(), "Import path cannot be empty.");
+					// Check whether the import directive is for the standard library,
+					// and if yes, add specified file to source units to be parsed.
+					auto it = stdlib::sources.find(import->path());
+					if (it != stdlib::sources.end())
+					{
+						auto [name, content] = *it;
+						m_sources[name].charStream = std::make_unique<CharStream>(content, name);
+						sourcesToParse.push_back(name);
+					}
+
+					// The current value of `path` is the absolute path as seen from this source file.
+					// We first have to apply remappings before we can store the actual absolute path
+					// as seen globally.
+					import->annotation().absolutePath = applyRemapping(util::absolutePath(
+						import->path(),
+						path
+					), path);
 				}
 
-				// The current value of `path` is the absolute path as seen from this source file.
-				// We first have to apply remappings before we can store the actual absolute path
-				// as seen globally.
-				import->annotation().absolutePath = applyRemapping(util::absolutePath(
-					import->path(),
-					path
-				), path);
+				if (m_stopAfter >= ParsedAndImported)
+					for (auto const& newSource: loadMissingSources(*source.ast))
+					{
+						std::string const& newPath = newSource.first;
+						std::string const& newContents = newSource.second;
+						m_sources[newPath].charStream = std::make_shared<CharStream>(newContents, newPath);
+						sourcesToParse.push_back(newPath);
+					}
 			}
-
-			if (m_stopAfter >= ParsedAndImported)
-				for (auto const& newSource: loadMissingSources(*source.ast))
-				{
-					std::string const& newPath = newSource.first;
-					std::string const& newContents = newSource.second;
-					m_sources[newPath].charStream = std::make_shared<CharStream>(newContents, newPath);
-					sourcesToParse.push_back(newPath);
-				}
 		}
-	}
 
-	if (Error::containsErrors(m_errorReporter.errors()))
-		return false;
+		if (Error::containsErrors(m_errorReporter.errors()))
+			return false;
 
-	m_stackState = (m_stopAfter <= Parsed ? Parsed : ParsedAndImported);
-	storeContractDefinitions();
+		m_stackState = (m_stopAfter <= Parsed ? Parsed : ParsedAndImported);
+		storeContractDefinitions();
 
-	solAssert(!m_maxAstId.has_value());
-	m_maxAstId = parser.maxID();
+		solAssert(!m_maxAstId.has_value());
+		m_maxAstId = parser.maxID();
+	}
+	catch (UnimplementedFeatureError const& _error)
+	{
+		reportUnimplementedFeatureError(_error);
+	}
 
 	return true;
 }
@@ -494,6 +501,11 @@ bool CompilerStack::analyze()
 		solAssert(m_errorReporter.hasErrors(), "Unreported fatal error: "s + error.what());
 		noErrors = false;
 	}
+	catch (UnimplementedFeatureError const& _error)
+	{
+		reportUnimplementedFeatureError(_error);
+		noErrors = false;
+	}
 
 	if (!noErrors)
 		return false;
@@ -737,28 +749,10 @@ bool CompilerStack::compile(State _stopAfter)
 						m_errorReporter.error(_error.errorId(), _error.type(), SourceLocation(), _error.what());
 						return false;
 					}
-					catch (UnimplementedFeatureError const& _unimplementedError)
+					catch (UnimplementedFeatureError const& _error)
 					{
-						if (
-							SourceLocation const* sourceLocation =
-							boost::get_error_info<langutil::errinfo_sourceLocation>(_unimplementedError)
-						)
-						{
-							std::string const* comment = _unimplementedError.comment();
-							m_errorReporter.error(
-								1834_error,
-								Error::Type::CodeGenerationError,
-								*sourceLocation,
-								fmt::format(
-									"Unimplemented feature error {} in {}",
-									(comment && !comment->empty()) ? ": " + *comment : "",
-									_unimplementedError.lineInfo()
-								)
-							);
-							return false;
-						}
-						else
-							throw;
+						reportUnimplementedFeatureError(_error);
+						return false;
 					}
 				}
 	m_stackState = CompilationSuccessful;
@@ -1908,3 +1902,9 @@ experimental::Analysis const& CompilerStack::experimentalAnalysis() const
 	solAssert(!!m_experimentalAnalysis);
 	return *m_experimentalAnalysis;
 }
+
+void CompilerStack::reportUnimplementedFeatureError(UnimplementedFeatureError const& _error)
+{
+	solAssert(_error.comment(), "Unimplemented feature errors must include a message for the user");
+	m_errorReporter.unimplementedFeatureError(1834_error, _error.sourceLocation(), *_error.comment());
+}

libsolidity/interface/CompilerStack.h

@@ -503,6 +503,8 @@ class CompilerStack: public langutil::CharStreamProvider, public evmasm::Abstrac
 		FunctionDefinition const& _function
 	) const;
 
+	void reportUnimplementedFeatureError(langutil::UnimplementedFeatureError const& _error);
+
 	ReadCallback::Callback m_readFile;
 	OptimiserSettings m_optimiserSettings;
 	RevertStrings m_revertStrings = RevertStrings::Default;

libsolidity/interface/StandardCompiler.cpp

@@ -49,6 +49,7 @@ using namespace solidity;
 using namespace solidity::yul;
 using namespace solidity::frontend;
 using namespace solidity::langutil;
+using namespace solidity::util;
 using namespace std::string_literals;
 
 namespace
@@ -1402,13 +1403,8 @@ Json StandardCompiler::compileSolidity(StandardCompiler::InputsAndSettings _inpu
 	}
 	catch (UnimplementedFeatureError const& _exception)
 	{
-		errors.emplace_back(formatErrorWithException(
-			compilerStack,
-			_exception,
-			Error::Type::UnimplementedFeatureError,
-			"general",
-			"Unimplemented feature (" + _exception.lineInfo() + ")"
-		));
+		// let StandardCompiler::compile handle this
+		throw _exception;
 	}
 	catch (yul::YulException const& _exception)
 	{
@@ -1469,7 +1465,7 @@ Json StandardCompiler::compileSolidity(StandardCompiler::InputsAndSettings _inpu
 	Json output;
 
 	if (errors.size() > 0)
-		output["errors"] = std::move(errors);
+	output["errors"] = std::move(errors);
 
 	if (!compilerStack.unhandledSMTLib2Queries().empty())
 		for (std::string const& query: compilerStack.unhandledSMTLib2Queries())
@@ -1783,6 +1779,11 @@ Json StandardCompiler::compile(Json const& _input) noexcept
 	{
 		return formatFatalError(Error::Type::InternalCompilerError, std::string("JSON runtime exception: ") + util::removeNlohmannInternalErrorIdentifier(_exception.what()));
 	}
+	catch (UnimplementedFeatureError const& _exception)
+	{
+		solAssert(_exception.comment(), "Unimplemented feature errors must include a message for the user");
+		return formatFatalError(Error::Type::UnimplementedFeatureError, stringOrDefault(_exception.comment()));
+	}
 	catch (util::Exception const& _exception)
 	{
 		return formatFatalError(Error::Type::InternalCompilerError, "Internal exception in StandardCompiler::compile: " + boost::diagnostic_information(_exception));

libsolutil/Assertions.h

@@ -26,6 +26,7 @@
 #pragma once
 
 #include <libsolutil/Exceptions.h>
+#include <libsolutil/CommonData.h>
 
 #include <string>
 #include <utility>
@@ -63,18 +64,6 @@ namespace solidity::util
 }
 #endif
 
-namespace assertions
-{
-
-inline std::string stringOrDefault(std::string _string, std::string _defaultString)
-{
-	// NOTE: Putting this in a function rather than directly in a macro prevents the string from
-	// being evaluated multiple times if it's not just a literal.
-	return (!_string.empty() ? std::move(_string) : std::move(_defaultString));
-}
-
-}
-
 /// Base macro that can be used to implement assertion macros.
 /// Throws an exception containing the given description if the condition is not met.
 /// Allows you to provide the default description for the case where the user of your macro does
@@ -86,7 +75,7 @@ inline std::string stringOrDefault(std::string _string, std::string _defaultStri
 		if (!(_condition)) \
 			solThrow( \
 				_exceptionType, \
-				::solidity::util::assertions::stringOrDefault((_description), (_defaultDescription)) \
+				::solidity::util::stringOrDefault((_description), (_defaultDescription)) \
 			); \
 	} \
 	while (false)

libsolutil/CommonData.h

@@ -632,4 +632,14 @@ std::vector<T> make_vector(Args&&... _args)
 	return result;
 }
 
+inline std::string stringOrDefault(std::string _string, std::string _defaultString = "")
+{
+	return (!_string.empty() ? std::move(_string) : std::move(_defaultString));
+}
+
+inline std::string stringOrDefault(std::string const* _string, std::string const& _defaultString = "")
+{
+	return (_string ? stringOrDefault(*_string, _defaultString) : _defaultString);
+}
+
 }

libsolutil/Exceptions.cpp

@@ -17,8 +17,10 @@
 // SPDX-License-Identifier: GPL-3.0
 
 #include <libsolutil/Exceptions.h>
+#include <liblangutil/Exceptions.h>
 
 using namespace solidity::util;
+using namespace solidity::langutil;
 
 char const* Exception::what() const noexcept
 {
@@ -49,3 +51,11 @@ std::string const* Exception::comment() const noexcept
 {
 	return boost::get_error_info<errinfo_comment>(*this);
 }
+
+SourceLocation Exception::sourceLocation() const noexcept
+{
+	if (SourceLocation const* sourceLocation = boost::get_error_info<errinfo_sourceLocation>(*this))
+		return *sourceLocation;
+
+	return SourceLocation{};
+}

libsolutil/Exceptions.h

@@ -18,6 +18,8 @@
 
 #pragma once
 
+#include <liblangutil/SourceLocation.h>
+
 #include <boost/exception/exception.hpp>
 #include <boost/exception/info.hpp>
 #include <boost/exception/info_tuple.hpp>
@@ -39,6 +41,9 @@ struct Exception: virtual std::exception, virtual boost::exception
 
 	/// @returns the errinfo_comment of this exception.
 	std::string const* comment() const noexcept;
+
+	/// @returns the errinfo_sourceLocation of this exception
+	langutil::SourceLocation sourceLocation() const noexcept;
 };
 
 /// Throws an exception with a given description and extra information about the location the