From 43cde4e1750d713b262a6cd966db1a1debd160b7 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Mon, 14 Jun 2021 12:03:53 +0200 Subject: [PATCH 1/4] Adds missing include header (for DebugData). --- libyul/AsmParser.h | 1 + 1 file changed, 1 insertion(+) diff --git a/libyul/AsmParser.h b/libyul/AsmParser.h index 896cb6d51..d010dc99d 100644 --- a/libyul/AsmParser.h +++ b/libyul/AsmParser.h @@ -23,6 +23,7 @@ #pragma once +#include #include #include From 132fa46faa80641088dec7e53c4cb99023b47ba1 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Wed, 16 Jun 2021 12:38:34 +0200 Subject: [PATCH 2/4] Yul: Adds parsing @src comment in AsmParser to customize the AST's sourcer locations. --- libsolidity/interface/CompilerStack.cpp | 14 +- libsolidity/interface/CompilerStack.h | 4 + libyul/AsmParser.cpp | 102 ++++++- libyul/AsmParser.h | 38 ++- scripts/error_codes.py | 3 + test/libyul/Parser.cpp | 369 +++++++++++++++++++++++- 6 files changed, 513 insertions(+), 17 deletions(-) diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index 0aeb88d1e..6f86c3cb5 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -80,7 +80,6 @@ #include #include -#include #include @@ -927,6 +926,19 @@ map CompilerStack::sourceIndices() const return indices; } +map> CompilerStack::indicesToCharStreams() const +{ + map> result; + unsigned index = 0; + for (auto const& s: m_sources) + result[index++] = s.second.scanner->charStream(); + + // NB: CompilerContext::yulUtilityFileName() does not have a source, + result[index++] = shared_ptr{}; + + return result; +} + Json::Value const& CompilerStack::contractABI(string const& _contractName) const { if (m_stackState < AnalysisPerformed) diff --git a/libsolidity/interface/CompilerStack.h b/libsolidity/interface/CompilerStack.h index 44daa6381..9459d7cfa 100644 --- a/libsolidity/interface/CompilerStack.h +++ b/libsolidity/interface/CompilerStack.h @@ -239,6 +239,10 @@ public: /// by sourceNames(). std::map sourceIndices() const; + /// @returns the reverse mapping of source indices to their respective + /// CharStream instances. + std::map> indicesToCharStreams() const; + /// @returns the previously used scanner, useful for counting lines during error reporting. langutil::Scanner const& scanner(std::string const& _sourceName) const; diff --git a/libyul/AsmParser.cpp b/libyul/AsmParser.cpp index 6553dccfe..69c43563b 100644 --- a/libyul/AsmParser.cpp +++ b/libyul/AsmParser.cpp @@ -29,9 +29,12 @@ #include #include +#include + #include #include +#include using namespace std; using namespace solidity; @@ -53,6 +56,18 @@ shared_ptr updateLocationEndFrom( return make_shared(updatedLocation); } +optional toInt(string const& _value) +{ + try + { + return stoi(_value); + } + catch (...) + { + return nullopt; + } +} + } unique_ptr Parser::parse(std::shared_ptr const& _scanner, bool _reuseScanner) @@ -65,6 +80,8 @@ unique_ptr Parser::parse(std::shared_ptr const& _scanner, bool _ try { m_scanner = _scanner; + if (m_charStreamMap) + fetchSourceLocationFromComment(); auto block = make_unique(parseBlock()); if (!_reuseScanner) expectToken(Token::EOS); @@ -78,6 +95,56 @@ unique_ptr Parser::parse(std::shared_ptr const& _scanner, bool _ return nullptr; } +langutil::Token Parser::advance() +{ + auto const token = ParserBase::advance(); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Comments) + fetchSourceLocationFromComment(); + return token; +} + +void Parser::fetchSourceLocationFromComment() +{ + solAssert(m_charStreamMap.has_value(), ""); + + if (m_scanner->currentCommentLiteral().empty()) + return; + + static regex const lineRE = std::regex( + R"~~~((^|\s*)@src\s+(-1|\d+):(-1|\d+):(-1|\d+)(\s+|$))~~~", + std::regex_constants::ECMAScript | std::regex_constants::optimize + ); + + string const text = m_scanner->currentCommentLiteral(); + auto from = sregex_iterator(text.begin(), text.end(), lineRE); + auto to = sregex_iterator(); + + for (auto const& matchResult: ranges::make_subrange(from, to)) + { + solAssert(matchResult.size() == 6, ""); + + auto const sourceIndex = toInt(matchResult[2].str()); + auto const start = toInt(matchResult[3].str()); + auto const end = toInt(matchResult[4].str()); + + auto const commentLocation = m_scanner->currentCommentLocation(); + m_locationOverride = SourceLocation{}; + if (!sourceIndex || !start || !end) + m_errorReporter.syntaxError(6367_error, commentLocation, "Invalid value in source location mapping. Could not parse location specification."); + else if (!((start < 0 && end < 0) || (start >= 0 && *start <= *end))) + m_errorReporter.syntaxError(5798_error, commentLocation, "Invalid value in source location mapping. Start offset larger than end offset."); + else if (!(sourceIndex >= 0 && m_charStreamMap->count(static_cast(*sourceIndex)))) + m_errorReporter.syntaxError(2674_error, commentLocation, "Invalid source mapping. Source index not defined via @use-src."); + else if (sourceIndex >= 0) + { + shared_ptr charStream = m_charStreamMap->at(static_cast(*sourceIndex)); + solAssert(charStream, ""); + + m_locationOverride = SourceLocation{*start, *end, charStream}; + } + } +} + Block Parser::parseBlock() { RecursionGuard recursionGuard(*this); @@ -85,7 +152,8 @@ Block Parser::parseBlock() expectToken(Token::LBrace); while (currentToken() != Token::RBrace) block.statements.emplace_back(parseStatement()); - block.debugData = updateLocationEndFrom(block.debugData, currentLocation()); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + block.debugData = updateLocationEndFrom(block.debugData, currentLocation()); advance(); return block; } @@ -107,7 +175,8 @@ Statement Parser::parseStatement() advance(); _if.condition = make_unique(parseExpression()); _if.body = parseBlock(); - _if.debugData = updateLocationEndFrom(_if.debugData, _if.body.debugData->location); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + _if.debugData = updateLocationEndFrom(_if.debugData, _if.body.debugData->location); return Statement{move(_if)}; } case Token::Switch: @@ -125,7 +194,8 @@ Statement Parser::parseStatement() fatalParserError(4904_error, "Case not allowed after default case."); if (_switch.cases.empty()) fatalParserError(2418_error, "Switch statement without any cases."); - _switch.debugData = updateLocationEndFrom(_switch.debugData, _switch.cases.back().body.debugData->location); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + _switch.debugData = updateLocationEndFrom(_switch.debugData, _switch.cases.back().body.debugData->location); return Statement{move(_switch)}; } case Token::For: @@ -207,7 +277,8 @@ Statement Parser::parseStatement() expectToken(Token::AssemblyAssign); assignment.value = make_unique(parseExpression()); - assignment.debugData = updateLocationEndFrom(assignment.debugData, locationOf(*assignment.value)); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + assignment.debugData = updateLocationEndFrom(assignment.debugData, locationOf(*assignment.value)); return Statement{move(assignment)}; } @@ -237,7 +308,8 @@ Case Parser::parseCase() else yulAssert(false, "Case or default case expected."); _case.body = parseBlock(); - _case.debugData = updateLocationEndFrom(_case.debugData, _case.body.debugData->location); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + _case.debugData = updateLocationEndFrom(_case.debugData, _case.body.debugData->location); return _case; } @@ -257,7 +329,8 @@ ForLoop Parser::parseForLoop() forLoop.post = parseBlock(); m_currentForLoopComponent = ForLoopComponent::ForLoopBody; forLoop.body = parseBlock(); - forLoop.debugData = updateLocationEndFrom(forLoop.debugData, forLoop.body.debugData->location); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + forLoop.debugData = updateLocationEndFrom(forLoop.debugData, forLoop.body.debugData->location); m_currentForLoopComponent = outerForLoopComponent; @@ -336,7 +409,8 @@ variant Parser::parseLiteralOrIdentifier() if (currentToken() == Token::Colon) { expectToken(Token::Colon); - literal.debugData = updateLocationEndFrom(literal.debugData, currentLocation()); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + literal.debugData = updateLocationEndFrom(literal.debugData, currentLocation()); literal.type = expectAsmIdentifier(); } @@ -368,9 +442,10 @@ VariableDeclaration Parser::parseVariableDeclaration() { expectToken(Token::AssemblyAssign); varDecl.value = make_unique(parseExpression()); - varDecl.debugData = updateLocationEndFrom(varDecl.debugData, locationOf(*varDecl.value)); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + varDecl.debugData = updateLocationEndFrom(varDecl.debugData, locationOf(*varDecl.value)); } - else + else if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) varDecl.debugData = updateLocationEndFrom(varDecl.debugData, varDecl.variables.back().debugData->location); return varDecl; @@ -417,7 +492,8 @@ FunctionDefinition Parser::parseFunctionDefinition() m_insideFunction = true; funDef.body = parseBlock(); m_insideFunction = preInsideFunction; - funDef.debugData = updateLocationEndFrom(funDef.debugData, funDef.body.debugData->location); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + funDef.debugData = updateLocationEndFrom(funDef.debugData, funDef.body.debugData->location); m_currentForLoopComponent = outerForLoopComponent; return funDef; @@ -444,7 +520,8 @@ FunctionCall Parser::parseCall(variant&& _initialOp) ret.arguments.emplace_back(parseExpression()); } } - ret.debugData = updateLocationEndFrom(ret.debugData, currentLocation()); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + ret.debugData = updateLocationEndFrom(ret.debugData, currentLocation()); expectToken(Token::RParen); return ret; } @@ -457,7 +534,8 @@ TypedName Parser::parseTypedName() if (currentToken() == Token::Colon) { expectToken(Token::Colon); - typedName.debugData = updateLocationEndFrom(typedName.debugData, currentLocation()); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + typedName.debugData = updateLocationEndFrom(typedName.debugData, currentLocation()); typedName.type = expectAsmIdentifier(); } else diff --git a/libyul/AsmParser.h b/libyul/AsmParser.h index d010dc99d..ed4d249d5 100644 --- a/libyul/AsmParser.h +++ b/libyul/AsmParser.h @@ -46,6 +46,11 @@ public: None, ForLoopPre, ForLoopPost, ForLoopBody }; + enum class UseSourceLocationFrom + { + Scanner, LocationOverride, Comments, + }; + explicit Parser( langutil::ErrorReporter& _errorReporter, Dialect const& _dialect, @@ -53,7 +58,25 @@ public: ): ParserBase(_errorReporter), m_dialect(_dialect), - m_locationOverride(std::move(_locationOverride)) + m_locationOverride{_locationOverride ? *_locationOverride : langutil::SourceLocation{}}, + m_useSourceLocationFrom{ + _locationOverride ? + UseSourceLocationFrom::LocationOverride : + UseSourceLocationFrom::Scanner + } + {} + + /// Constructs a Yul parser that is using the source locations + /// from the comments (via @src). + explicit Parser( + langutil::ErrorReporter& _errorReporter, + Dialect const& _dialect, + std::map> _charStreamMap + ): + ParserBase(_errorReporter), + m_dialect(_dialect), + m_charStreamMap{std::move(_charStreamMap)}, + m_useSourceLocationFrom{UseSourceLocationFrom::Comments} {} /// Parses an inline assembly block starting with `{` and ending with `}`. @@ -64,9 +87,15 @@ public: protected: langutil::SourceLocation currentLocation() const override { - return m_locationOverride ? *m_locationOverride : ParserBase::currentLocation(); + if (m_useSourceLocationFrom == UseSourceLocationFrom::Scanner) + return ParserBase::currentLocation(); + return m_locationOverride; } + langutil::Token advance() override; + + void fetchSourceLocationFromComment(); + /// Creates an inline assembly node with the current source location. template T createWithLocation() const { @@ -97,7 +126,10 @@ protected: private: Dialect const& m_dialect; - std::optional m_locationOverride; + + std::optional>> m_charStreamMap; + langutil::SourceLocation m_locationOverride; + UseSourceLocationFrom m_useSourceLocationFrom = UseSourceLocationFrom::Scanner; ForLoopComponent m_currentForLoopComponent = ForLoopComponent::None; bool m_insideFunction = false; }; diff --git a/scripts/error_codes.py b/scripts/error_codes.py index adb579ebb..ffb95b889 100755 --- a/scripts/error_codes.py +++ b/scripts/error_codes.py @@ -191,6 +191,9 @@ def examine_id_coverage(top_dir, source_id_to_file_names, new_ids_only=False): # white list of ids which are not covered by tests white_ids = { + "6367", # these three are temporarily whitelisted until both PRs are merged. + "5798", + "2674", "3805", # "This is a pre-release compiler version, please do not use it in production." # The warning may or may not exist in a compiler build. "4591", # "There are more than 256 warnings. Ignoring the rest." diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index 6bb34064e..7fe43f842 100644 --- a/test/libyul/Parser.cpp +++ b/test/libyul/Parser.cpp @@ -51,12 +51,25 @@ namespace solidity::yul::test namespace { +string_view constexpr g_strAlternateSourceText = "{}"; + shared_ptr parse(string const& _source, Dialect const& _dialect, ErrorReporter& errorReporter) { try { auto scanner = make_shared(CharStream(_source, "")); - auto parserResult = yul::Parser(errorReporter, _dialect).parse(scanner, false); + map> indicesToCharStreams; + indicesToCharStreams[0] = scanner->charStream(); + indicesToCharStreams[1] = make_shared( + string(g_strAlternateSourceText.data(), g_strAlternateSourceText.size()), + "alternate.sol" + ); + + auto parserResult = yul::Parser( + errorReporter, + _dialect, + move(indicesToCharStreams) + ).parse(scanner, false); if (parserResult) { yul::AsmAnalysisInfo analysisInfo; @@ -187,7 +200,361 @@ BOOST_AUTO_TEST_CASE(default_types_set) ); } +#define CHECK_LOCATION(_actual, _sourceText, _start, _end) \ + do { \ + BOOST_CHECK_EQUAL((_sourceText), ((_actual).source ? (_actual).source->source() : "")); \ + BOOST_CHECK_EQUAL((_start), (_actual).start); \ + BOOST_CHECK_EQUAL((_end), (_actual).end); \ + } while (0) +BOOST_AUTO_TEST_CASE(customSourceLocations_empty_block) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "/// @src 0:234:543\n" + "{}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + CHECK_LOCATION(result->debugData->location, sourceText, 234, 543); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_block_with_children) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "/// @src 0:234:543\n" + "{\n" + "let x:bool := true:bool\n" + "/// @src 0:123:432\n" + "let z:bool := true\n" + "let y := add(1, 2)\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + CHECK_LOCATION(result->debugData->location, sourceText, 234, 543); + BOOST_REQUIRE_EQUAL(3, result->statements.size()); + CHECK_LOCATION(locationOf(result->statements.at(0)), sourceText, 234, 543); + CHECK_LOCATION(locationOf(result->statements.at(1)), sourceText, 123, 432); + // [2] is inherited source location + CHECK_LOCATION(locationOf(result->statements.at(2)), sourceText, 123, 432); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_block_different_sources) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "/// @src 0:234:543\n" + "{\n" + "let x:bool := true:bool\n" + "/// @src 1:123:432\n" + "let z:bool := true\n" + "let y := add(1, 2)\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + CHECK_LOCATION(result->debugData->location, sourceText, 234, 543); + BOOST_REQUIRE_EQUAL(3, result->statements.size()); + CHECK_LOCATION(locationOf(result->statements.at(0)), sourceText, 234, 543); + CHECK_LOCATION(locationOf(result->statements.at(1)), g_strAlternateSourceText, 123, 432); + // [2] is inherited source location + CHECK_LOCATION(locationOf(result->statements.at(2)), g_strAlternateSourceText, 123, 432); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_block_nested) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "/// @src 0:234:543\n" + "{\n" + "let y := add(1, 2)\n" + "/// @src 0:343:434\n" + "switch y case 0 {} default {}\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + CHECK_LOCATION(result->debugData->location, sourceText, 234, 543); + BOOST_REQUIRE_EQUAL(2, result->statements.size()); + CHECK_LOCATION(locationOf(result->statements.at(1)), sourceText, 343, 434); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_block_switch_case) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "/// @src 0:234:543\n" + "{\n" + "let y := add(1, 2)\n" + "/// @src 0:343:434\n" + "switch y\n" + "/// @src 0:3141:59265\n" + "case 0 {\n" + " /// @src 0:271:828\n" + " let z := add(3, 4)\n" + "}\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + CHECK_LOCATION(result->debugData->location, sourceText, 234, 543); + + BOOST_REQUIRE_EQUAL(2, result->statements.size()); + BOOST_REQUIRE(holds_alternative(result->statements.at(1))); + auto const& switchStmt = get(result->statements.at(1)); + + CHECK_LOCATION(switchStmt.debugData->location, sourceText, 343, 434); + BOOST_REQUIRE_EQUAL(1, switchStmt.cases.size()); + CHECK_LOCATION(switchStmt.cases.at(0).debugData->location, sourceText, 3141, 59265); + + auto const& caseBody = switchStmt.cases.at(0).body; + BOOST_REQUIRE_EQUAL(1, caseBody.statements.size()); + CHECK_LOCATION(locationOf(caseBody.statements.at(0)), sourceText, 271, 828); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_inherit_into_outer_scope) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "/// @src 0:1:100\n" + "{\n" + "{\n" + "/// @src 0:123:432\n" + "let x:bool := true:bool\n" + "}\n" + "let z:bool := true\n" + "let y := add(1, 2)\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + + CHECK_LOCATION(result->debugData->location, sourceText, 1, 100); + + BOOST_REQUIRE_EQUAL(3, result->statements.size()); + CHECK_LOCATION(locationOf(result->statements.at(0)), sourceText, 1, 100); + + // First child element must be a block itself with one statement. + BOOST_REQUIRE(holds_alternative(result->statements.at(0))); + BOOST_REQUIRE_EQUAL(get(result->statements.at(0)).statements.size(), 1); + CHECK_LOCATION(locationOf(get(result->statements.at(0)).statements.at(0)), sourceText, 123, 432); + + // The next two elements have an inherited source location from the prior inner scope. + CHECK_LOCATION(locationOf(result->statements.at(1)), sourceText, 123, 432); + CHECK_LOCATION(locationOf(result->statements.at(2)), sourceText, 123, 432); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_assign_empty) +{ + // Tests single AST node (e.g. VariableDeclaration) with different source locations for each child. + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "{\n" + "/// @src 0:123:432\n" + "let a:bool\n" + "/// @src 1:1:10\n" + "a := true:bool\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); // should still parse + BOOST_REQUIRE_EQUAL(2, result->statements.size()); + CHECK_LOCATION(locationOf(result->statements.at(0)), sourceText, 123, 432); + CHECK_LOCATION(locationOf(result->statements.at(1)), g_strAlternateSourceText, 1, 10); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_invalid_source_index) +{ + // Tests single AST node (e.g. VariableDeclaration) with different source locations for each child. + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "{\n" + "/// @src 1:123:432\n" + "let a:bool := true:bool\n" + "/// @src 2345:0:8\n" + "let b:bool := true:bool\n" + "\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); // should still parse +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_mixed_locations_1) +{ + // Tests single AST node (e.g. VariableDeclaration) with different source locations for each child. + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = + "{\n" + "/// @src 0:123:432\n" + "let x:bool \n" + "/// @src 0:234:2026\n" + ":= true:bool\n" + "}\n"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + + BOOST_REQUIRE_EQUAL(1, result->statements.size()); + CHECK_LOCATION(locationOf(result->statements.at(0)), sourceText, 123, 432); + BOOST_REQUIRE(holds_alternative(result->statements.at(0))); + VariableDeclaration const& varDecl = get(result->statements.at(0)); + CHECK_LOCATION(locationOf(*varDecl.value), sourceText, 234, 2026); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_mixed_locations_2) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = R"( + /// @src 0:0:5 + { + let x := /// @src 1:2:3 + add(1, /// @src 0:4:8 + 2) + } + )"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + BOOST_REQUIRE_EQUAL(1, result->statements.size()); + CHECK_LOCATION(result->debugData->location, sourceText, 0, 5); + + // `let x := add(1, ` + BOOST_REQUIRE(holds_alternative(result->statements.at(0))); + VariableDeclaration const& varDecl = get(result->statements.at(0)); + CHECK_LOCATION(varDecl.debugData->location, sourceText, 0, 5); + BOOST_REQUIRE(!!varDecl.value); + BOOST_REQUIRE(holds_alternative(*varDecl.value)); + FunctionCall const& call = get(*varDecl.value); + CHECK_LOCATION(call.debugData->location, g_strAlternateSourceText, 2, 3); + + // `2` + BOOST_REQUIRE_EQUAL(2, call.arguments.size()); + CHECK_LOCATION(locationOf(call.arguments.at(1)), sourceText, 4, 8); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_mixed_locations_3) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = R"( + /// @src 1:23:45 + { // Block + { // Block + sstore(0, 1) // FunctionCall + /// @src 0:420:680 + } + mstore(1, 2) // FunctionCall + } + )"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + BOOST_REQUIRE_EQUAL(2, result->statements.size()); + CHECK_LOCATION(result->debugData->location, g_strAlternateSourceText, 23, 45); + + BOOST_REQUIRE(holds_alternative(result->statements.at(0))); + Block const& innerBlock = get(result->statements.at(0)); + CHECK_LOCATION(innerBlock.debugData->location, g_strAlternateSourceText, 23, 45); + + BOOST_REQUIRE_EQUAL(1, innerBlock.statements.size()); + BOOST_REQUIRE(holds_alternative(result->statements.at(1))); + ExpressionStatement const& sstoreStmt = get(innerBlock.statements.at(0)); + BOOST_REQUIRE(holds_alternative(sstoreStmt.expression)); + FunctionCall const& sstoreCall = get(sstoreStmt.expression); + CHECK_LOCATION(sstoreCall.debugData->location, g_strAlternateSourceText, 23, 45); + + BOOST_REQUIRE(holds_alternative(result->statements.at(1))); + ExpressionStatement mstoreStmt = get(result->statements.at(1)); + BOOST_REQUIRE(holds_alternative(mstoreStmt.expression)); + FunctionCall const& mstoreCall = get(mstoreStmt.expression); + CHECK_LOCATION(mstoreCall.debugData->location, sourceText, 420, 680); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_invalid_comments_after_valid) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = R"( + /// @src 1:23:45 + { + /// @src 0:420:680 + /// @invalid + let a:bool := true + } + )"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + BOOST_REQUIRE_EQUAL(1, result->statements.size()); + CHECK_LOCATION(result->debugData->location, g_strAlternateSourceText, 23, 45); + + BOOST_REQUIRE(holds_alternative(result->statements.at(0))); + VariableDeclaration const& varDecl = get(result->statements.at(0)); + CHECK_LOCATION(varDecl.debugData->location, sourceText, 420, 680); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_invalid_suffix) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = R"( + /// @src 0:420:680foo + {} + )"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + CHECK_LOCATION(result->debugData->location, "", -1, -1); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_unspecified) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = R"( + /// @src -1:-1:-1 + {} + )"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + CHECK_LOCATION(result->debugData->location, "", -1, -1); +} + +BOOST_AUTO_TEST_CASE(customSourceLocations_ensure_last_match) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = R"( + /// @src 0:123:432 + { + /// @src 1:10:20 + /// @src 0:30:40 + let x:bool := true + } + )"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + BOOST_REQUIRE(holds_alternative(result->statements.at(0))); + VariableDeclaration const& varDecl = get(result->statements.at(0)); + + // Ensure the latest @src per documentation-comment is used (0:30:40). + CHECK_LOCATION(varDecl.debugData->location, sourceText, 30, 40); +} BOOST_AUTO_TEST_SUITE_END() From 5e4868d5d60cb659aea1db64b6946804cf79a234 Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Wed, 7 Jul 2021 13:31:01 +0200 Subject: [PATCH 3/4] Adapted tests due to more precise Yul source locations. Also added support for -1 source index, referencing original scanner's source location. --- libyul/AsmParser.cpp | 6 ++++-- test/libyul/Parser.cpp | 22 ++++++++++++++++++++++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/libyul/AsmParser.cpp b/libyul/AsmParser.cpp index 69c43563b..ce25724f9 100644 --- a/libyul/AsmParser.cpp +++ b/libyul/AsmParser.cpp @@ -131,11 +131,13 @@ void Parser::fetchSourceLocationFromComment() m_locationOverride = SourceLocation{}; if (!sourceIndex || !start || !end) m_errorReporter.syntaxError(6367_error, commentLocation, "Invalid value in source location mapping. Could not parse location specification."); - else if (!((start < 0 && end < 0) || (start >= 0 && *start <= *end))) + else if (!((*start < 0 && *end < 0) || (*start >= 0 && *start <= *end))) m_errorReporter.syntaxError(5798_error, commentLocation, "Invalid value in source location mapping. Start offset larger than end offset."); + else if (sourceIndex == -1 && (0 <= *start && *start <= *end)) // Use source index -1 to indicate original source. + m_locationOverride = SourceLocation{*start, *end, ParserBase::currentLocation().source}; else if (!(sourceIndex >= 0 && m_charStreamMap->count(static_cast(*sourceIndex)))) m_errorReporter.syntaxError(2674_error, commentLocation, "Invalid source mapping. Source index not defined via @use-src."); - else if (sourceIndex >= 0) + else { shared_ptr charStream = m_charStreamMap->at(static_cast(*sourceIndex)); solAssert(charStream, ""); diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp index 7fe43f842..84f9f8045 100644 --- a/test/libyul/Parser.cpp +++ b/test/libyul/Parser.cpp @@ -556,6 +556,28 @@ BOOST_AUTO_TEST_CASE(customSourceLocations_ensure_last_match) CHECK_LOCATION(varDecl.debugData->location, sourceText, 30, 40); } +BOOST_AUTO_TEST_CASE(customSourceLocations_reference_original_sloc) +{ + ErrorList errorList; + ErrorReporter reporter(errorList); + auto const sourceText = R"( + /// @src 1:2:3 + { + /// @src -1:10:20 + let x:bool := true + } + )"; + EVMDialectTyped const& dialect = EVMDialectTyped::instance(EVMVersion{}); + shared_ptr result = parse(sourceText, dialect, reporter); + BOOST_REQUIRE(!!result); + BOOST_REQUIRE(holds_alternative(result->statements.at(0))); + VariableDeclaration const& varDecl = get(result->statements.at(0)); + + // -1 points to original source code, which in this case is `sourceText` (which is also + // available via `0`, that's why the first @src is set to `1` instead.) + CHECK_LOCATION(varDecl.debugData->location, sourceText, 10, 20); +} + BOOST_AUTO_TEST_SUITE_END() } // end namespaces From f129a3498c689a086d65ef3a399dcd5516e4d58e Mon Sep 17 00:00:00 2001 From: Christian Parpart Date: Thu, 8 Jul 2021 16:38:16 +0200 Subject: [PATCH 4/4] Use shared DebugData for when using source locations from comments. --- libyul/AsmParser.cpp | 11 +++++------ libyul/AsmParser.h | 20 +++++++++++++++++++- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/libyul/AsmParser.cpp b/libyul/AsmParser.cpp index ce25724f9..05b3c4b1c 100644 --- a/libyul/AsmParser.cpp +++ b/libyul/AsmParser.cpp @@ -128,21 +128,20 @@ void Parser::fetchSourceLocationFromComment() auto const end = toInt(matchResult[4].str()); auto const commentLocation = m_scanner->currentCommentLocation(); - m_locationOverride = SourceLocation{}; + m_debugDataOverride = DebugData::create(); if (!sourceIndex || !start || !end) m_errorReporter.syntaxError(6367_error, commentLocation, "Invalid value in source location mapping. Could not parse location specification."); else if (!((*start < 0 && *end < 0) || (*start >= 0 && *start <= *end))) m_errorReporter.syntaxError(5798_error, commentLocation, "Invalid value in source location mapping. Start offset larger than end offset."); else if (sourceIndex == -1 && (0 <= *start && *start <= *end)) // Use source index -1 to indicate original source. - m_locationOverride = SourceLocation{*start, *end, ParserBase::currentLocation().source}; + m_debugDataOverride = DebugData::create(SourceLocation{*start, *end, ParserBase::currentLocation().source}); else if (!(sourceIndex >= 0 && m_charStreamMap->count(static_cast(*sourceIndex)))) m_errorReporter.syntaxError(2674_error, commentLocation, "Invalid source mapping. Source index not defined via @use-src."); else { shared_ptr charStream = m_charStreamMap->at(static_cast(*sourceIndex)); solAssert(charStream, ""); - - m_locationOverride = SourceLocation{*start, *end, charStream}; + m_debugDataOverride = DebugData::create(SourceLocation{*start, *end, charStream}); } } } @@ -371,7 +370,7 @@ variant Parser::parseLiteralOrIdentifier() { case Token::Identifier: { - Identifier identifier{DebugData::create(currentLocation()), YulString{currentLiteral()}}; + Identifier identifier{createDebugData(), YulString{currentLiteral()}}; advance(); return identifier; } @@ -402,7 +401,7 @@ variant Parser::parseLiteralOrIdentifier() } Literal literal{ - DebugData::create(currentLocation()), + createDebugData(), kind, YulString{currentLiteral()}, kind == LiteralKind::Boolean ? m_dialect.boolType : m_dialect.defaultType diff --git a/libyul/AsmParser.h b/libyul/AsmParser.h index ed4d249d5..fc0c9bb64 100644 --- a/libyul/AsmParser.h +++ b/libyul/AsmParser.h @@ -59,6 +59,7 @@ public: ParserBase(_errorReporter), m_dialect(_dialect), m_locationOverride{_locationOverride ? *_locationOverride : langutil::SourceLocation{}}, + m_debugDataOverride{}, m_useSourceLocationFrom{ _locationOverride ? UseSourceLocationFrom::LocationOverride : @@ -76,6 +77,7 @@ public: ParserBase(_errorReporter), m_dialect(_dialect), m_charStreamMap{std::move(_charStreamMap)}, + m_debugDataOverride{DebugData::create()}, m_useSourceLocationFrom{UseSourceLocationFrom::Comments} {} @@ -96,11 +98,26 @@ protected: void fetchSourceLocationFromComment(); + /// Creates a DebugData object with the correct source location set. + std::shared_ptr createDebugData() const + { + switch (m_useSourceLocationFrom) + { + case UseSourceLocationFrom::Scanner: + return DebugData::create(ParserBase::currentLocation()); + case UseSourceLocationFrom::LocationOverride: + return DebugData::create(m_locationOverride); + case UseSourceLocationFrom::Comments: + return m_debugDataOverride; + } + solAssert(false, ""); + } + /// Creates an inline assembly node with the current source location. template T createWithLocation() const { T r; - r.debugData = DebugData::create(currentLocation()); + r.debugData = createDebugData(); return r; } @@ -129,6 +146,7 @@ private: std::optional>> m_charStreamMap; langutil::SourceLocation m_locationOverride; + std::shared_ptr m_debugDataOverride; UseSourceLocationFrom m_useSourceLocationFrom = UseSourceLocationFrom::Scanner; ForLoopComponent m_currentForLoopComponent = ForLoopComponent::None; bool m_insideFunction = false;