AsmParser: Parsing rules for source location comments have been relaxed: Whitespace between the indices as well as single-quoted code snippets are now allowed.

clonker · clonker · commit 5bca035091d7 · 2024-06-24T10:21:12.000+02:00
diff --git a/Changelog.md b/Changelog.md
@@ -13,6 +13,7 @@ Compiler Features:
 
 
 Bugfixes:
+ * AsmParser: Parsing rules for source location comments have been relaxed: Whitespace between the indices as well as single-quoted code snippets are now allowed.
  * SMTChecker: Fix error that reports invalid number of verified checks for BMC and CHC engines.
  * SMTChecker: Fix internal compiler error when reporting proved targets for BMC engine.
  * TypeChecker: Fix segfault when assigning nested tuple to tuple.
diff --git a/liblangutil/Scanner.cpp b/liblangutil/Scanner.cpp
@@ -826,31 +826,44 @@ Token Scanner::scanString(bool const _isUnicode)
 	char const quote = m_char;
 	advance();  // consume quote
 	LiteralScope literal(this, LITERAL_TYPE_STRING);
-	while (m_char != quote && !isSourcePastEndOfInput() && !isUnicodeLinebreak())
+	// for source location comments we allow multiline string literals
+	while (m_char != quote && !isSourcePastEndOfInput() && (!isUnicodeLinebreak() || m_kind == ScannerKind::SpecialComment))
 	{
 		char c = m_char;
 		advance();
-		if (c == '\\')
+
+		if (m_kind == ScannerKind::SpecialComment)
 		{
-			if (isSourcePastEndOfInput() || !scanEscape())
-				return setError(ScannerError::IllegalEscapeSequence);
+			if (c == '\\')
+				scanEscape();
+			else
+				addLiteralChar(c);
 		}
 		else
 		{
-			// Report error on non-printable characters in string literals, however
-			// allow anything for unicode string literals, because their validity will
-			// be verified later (in the syntax checker).
-			//
-			// We are using a manual range and not isprint() to avoid
-			// any potential complications with locale.
-			if (!_isUnicode && (static_cast<unsigned>(c) <= 0x1f || static_cast<unsigned>(c) >= 0x7f))
+			if (c == '\\')
 			{
-				if (m_kind == ScannerKind::Yul)
-					return setError(ScannerError::IllegalCharacterInString);
-				return setError(ScannerError::UnicodeCharacterInNonUnicodeString);
+				if (isSourcePastEndOfInput() || !scanEscape())
+					return setError(ScannerError::IllegalEscapeSequence);
+			}
+			else
+			{
+				// Report error on non-printable characters in string literals, however
+				// allow anything for unicode string literals, because their validity will
+				// be verified later (in the syntax checker).
+				//
+				// We are using a manual range and not isprint() to avoid
+				// any potential complications with locale.
+				if (!_isUnicode && (static_cast<unsigned>(c) <= 0x1f || static_cast<unsigned>(c) >= 0x7f))
+				{
+					if (m_kind == ScannerKind::Yul)
+						return setError(ScannerError::IllegalCharacterInString);
+					return setError(ScannerError::UnicodeCharacterInNonUnicodeString);
+				}
+				addLiteralChar(c);
 			}
-			addLiteralChar(c);
 		}
+
 	}
 	if (m_char != quote)
 		return setError(ScannerError::IllegalStringEndQuote);
@@ -1023,6 +1036,9 @@ std::tuple<Token, unsigned, unsigned> Scanner::scanIdentifierOrKeyword()
 	auto const token = TokenTraits::fromIdentifierOrKeyword(m_tokens[NextNext].literal);
 	switch (m_kind)
 	{
+	case ScannerKind::SpecialComment:
+		// there are no keywords in special comments
+		return std::make_tuple(Token::Identifier, 0, 0);
 	case ScannerKind::Solidity:
 		// Turn experimental Solidity keywords that are not keywords in legacy Solidity into identifiers.
 		if (TokenTraits::isExperimentalSolidityOnlyKeyword(std::get<0>(token)))
diff --git a/liblangutil/Scanner.h b/liblangutil/Scanner.h
@@ -62,15 +62,12 @@
 namespace solidity::langutil
 {
 
-class AstRawString;
-class AstValueFactory;
-class ParserRecorder;
-
 enum class ScannerKind
 {
 	Solidity,
 	Yul,
-	ExperimentalSolidity
+	ExperimentalSolidity,
+	SpecialComment
 };
 
 enum class ScannerError
diff --git a/libyul/AsmParser.cpp b/libyul/AsmParser.cpp
@@ -27,11 +27,10 @@
 #include <liblangutil/ErrorReporter.h>
 #include <liblangutil/Exceptions.h>
 #include <liblangutil/Scanner.h>
+#include <liblangutil/Common.h>
 #include <libsolutil/Common.h>
 #include <libsolutil/Visitor.h>
 
-#include <range/v3/view/subrange.hpp>
-
 #include <boost/algorithm/string.hpp>
 
 #include <algorithm>
@@ -52,7 +51,7 @@ std::optional<int> toInt(std::string const& _value)
 	{
 		return stoi(_value);
 	}
-	catch (...)
+	catch (std::out_of_range const&)
 	{
 		return std::nullopt;
 	}
@@ -192,13 +191,42 @@ std::optional<std::pair<std::string_view, SourceLocation>> Parser::parseSrcComme
 	langutil::SourceLocation const& _commentLocation
 )
 {
-	static std::regex const argsRegex = std::regex(
-		R"~~(^(-1|\d+):(-1|\d+):(-1|\d+)(?:\s+|$))~~"  // index and location, e.g.: 1:234:-1
-		R"~~(("(?:[^"\\]|\\.)*"?)?)~~",                // optional code snippet, e.g.: "string memory s = \"abc\";..."
-		std::regex_constants::ECMAScript | std::regex_constants::optimize
-	);
-	std::match_results<std::string_view::const_iterator> match;
-	if (!regex_search(_arguments.cbegin(), _arguments.cend(), match, argsRegex))
+	CharStream argumentStream (std::string(_arguments), "");
+	Scanner scanner (argumentStream);
+	scanner.setScannerMode(ScannerKind::SpecialComment);
+
+	std::string_view tail { _arguments.substr(_arguments.size()) };
+	auto const parseLocationComponent = [](Scanner& _scanner, bool expectTrailingColon) -> std::optional<std::string>
+	{
+		bool negative = false;
+		if (_scanner.currentToken() == Token::Sub)
+		{
+			negative = true;
+			_scanner.next();
+		}
+		if (_scanner.currentToken() != Token::Number)
+			return std::nullopt;
+		if (expectTrailingColon && _scanner.peekNextToken() != Token::Colon)
+			return std::nullopt;
+		if (!isValidDecimal(_scanner.currentLiteral()))
+			return std::nullopt;
+		std::string decimal = (negative ? "-" : "") + _scanner.currentLiteral();
+		_scanner.next();
+		if (expectTrailingColon)
+			_scanner.next();
+		return decimal;
+	};
+	std::optional<std::string> rawSourceIndex = parseLocationComponent(scanner, true);
+	std::optional<std::string> rawStart = parseLocationComponent(scanner, true);
+	std::optional<std::string> rawEnd = parseLocationComponent(scanner, false);
+
+	size_t const snippetStart = static_cast<size_t>(scanner.currentLocation().start);
+	bool const locationScannedSuccessfully = rawSourceIndex && rawStart && rawEnd;
+	bool const locationIsWhitespaceSeparated =
+		scanner.peekNextToken() == Token::EOS ||
+		(snippetStart > 0 && langutil::isWhiteSpace(_arguments[snippetStart - 1]));
+
+	if (!locationScannedSuccessfully || !locationIsWhitespaceSeparated)
 	{
 		m_errorReporter.syntaxError(
 			8387_error,
@@ -208,13 +236,12 @@ std::optional<std::pair<std::string_view, SourceLocation>> Parser::parseSrcComme
 		return std::nullopt;
 	}
 
-	solAssert(match.size() == 5, "");
-	std::string_view tail = _arguments.substr(static_cast<size_t>(match.position() + match.length()));
+	if (scanner.currentToken() == Token::StringLiteral || (scanner.currentToken() == Token::Illegal && scanner.currentError() == ScannerError::IllegalStringEndQuote))
+		tail = _arguments.substr(static_cast<size_t>(scanner.currentLocation().end));
+	else
+		tail = _arguments.substr(static_cast<size_t>(scanner.currentLocation().start));
 
-	if (match[4].matched && (
-		!boost::algorithm::ends_with(match[4].str(), "\"") ||
-		boost::algorithm::ends_with(match[4].str(), "\\\"")
-	))
+	if (scanner.currentToken() == Token::Illegal && scanner.currentError() == ScannerError::IllegalStringEndQuote)
 	{
 		m_errorReporter.syntaxError(
 			1544_error,
@@ -224,30 +251,34 @@ std::optional<std::pair<std::string_view, SourceLocation>> Parser::parseSrcComme
 		return {{tail, SourceLocation{}}};
 	}
 
-	std::optional<int> const sourceIndex = toInt(match[1].str());
-	std::optional<int> const start = toInt(match[2].str());
-	std::optional<int> const end = toInt(match[3].str());
+	std::optional<int> const sourceIndex = toInt(*rawSourceIndex);
+	std::optional<int> const start = toInt(*rawStart);
+	std::optional<int> const end = toInt(*rawEnd);
 
-	if (!sourceIndex.has_value() || !start.has_value() || !end.has_value())
+	if (
+		!sourceIndex.has_value() || *sourceIndex < -1 ||
+		!start.has_value() || *start < -1 ||
+		!end.has_value() || *end < -1
+	)
 		m_errorReporter.syntaxError(
 			6367_error,
 			_commentLocation,
 			"Invalid value in source location mapping. "
 			"Expected non-negative integer values or -1 for source index and location."
 		);
 	else if (sourceIndex == -1)
-		return {{tail, SourceLocation{start.value(), end.value(), nullptr}}};
-	else if (!(sourceIndex >= 0 && m_sourceNames->count(static_cast<unsigned>(sourceIndex.value()))))
+		return {{tail, SourceLocation{*start, *end, nullptr}}};
+	else if (!(sourceIndex >= 0 && m_sourceNames->count(static_cast<unsigned>(*sourceIndex))))
 		m_errorReporter.syntaxError(
 			2674_error,
 			_commentLocation,
 			"Invalid source mapping. Source index not defined via @use-src."
 		);
 	else
 	{
-		std::shared_ptr<std::string const> sourceName = m_sourceNames->at(static_cast<unsigned>(sourceIndex.value()));
+		std::shared_ptr<std::string const> sourceName = m_sourceNames->at(static_cast<unsigned>(*sourceIndex));
 		solAssert(sourceName, "");
-		return {{tail, SourceLocation{start.value(), end.value(), std::move(sourceName)}}};
+		return {{tail, SourceLocation{*start, *end, std::move(sourceName)}}};
 	}
 	return {{tail, SourceLocation{}}};
 }
diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp
@@ -767,6 +767,58 @@ BOOST_AUTO_TEST_CASE(customSourceLocations_two_locations_with_snippets_untermina
 	CHECK_LOCATION(result->debugData->originLocation, "", -1, -1);
 }
 
+BOOST_AUTO_TEST_CASE(customSourceLocations_single_quote)
+{
+	ErrorList errorList;
+	ErrorReporter reporter(errorList);
+	auto const sourceText = R"(
+		/// @src 0:111:222 "
+		///
+		{}
+	)";
+	EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{});
+	std::shared_ptr<Block> result = parse(sourceText, dialect, reporter);
+	BOOST_REQUIRE(!!result);
+	BOOST_REQUIRE(errorList.size() == 1);
+	BOOST_TEST(errorList[0]->type() == Error::Type::SyntaxError);
+	BOOST_TEST(errorList[0]->errorId() == 1544_error);
+	CHECK_LOCATION(result->debugData->originLocation, "", -1, -1);
+}
+
+BOOST_AUTO_TEST_CASE(customSourceLocations_two_snippets_with_hex_comment)
+{
+	ErrorList errorList;
+	ErrorReporter reporter(errorList);
+	auto const sourceText = R"(
+		/// @src 0:111:222 hex"abc"@src 1:333:444 "abc"
+		{}
+	)";
+	EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{});
+	std::shared_ptr<Block> result = parse(sourceText, dialect, reporter);
+	BOOST_REQUIRE(!!result && errorList.size() == 0);
+	// the second source location is not parsed as such, as the hex string isn't interpreted as snippet but
+	// as the beginning of the tail in AsmParser
+	CHECK_LOCATION(result->debugData->originLocation, "source0", 111, 222);
+}
+
+BOOST_AUTO_TEST_CASE(customSourceLocations_multi_line_source_loc)
+{
+	ErrorList errorList;
+	ErrorReporter reporter(errorList);
+	auto const sourceText = R"(
+		/// @src 1	: 111:
+		/// 222 "
+		/// abc\"def
+		///
+		/// " @src 0:333:444
+		{}
+	)";
+	EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{});
+	std::shared_ptr<Block> result = parse(sourceText, dialect, reporter);
+	BOOST_REQUIRE(!!result && errorList.empty());
+	CHECK_LOCATION(result->debugData->originLocation, "source0", 333, 444);
+}
+
 BOOST_AUTO_TEST_CASE(customSourceLocations_with_code_snippets_with_nested_locations)
 {
 	ErrorList errorList;
