From 7fecab07a8a5ece29177c4f03b4a920de4b58baf Mon Sep 17 00:00:00 2001 From: a3d4 Date: Wed, 5 Feb 2020 22:13:03 +0100 Subject: [PATCH 1/2] Simplified Parser::createWithLocation(). In all but one case the function was called with the default argument value. And when it was location, the location should be valid (see Parser::parseElementaryOperation()). --- libyul/AsmParser.cpp | 4 ++-- libyul/AsmParser.h | 13 +++---------- 2 files changed, 5 insertions(+), 12 deletions(-) diff --git a/libyul/AsmParser.cpp b/libyul/AsmParser.cpp index 5a0f615d0..6d251c3ba 100644 --- a/libyul/AsmParser.cpp +++ b/libyul/AsmParser.cpp @@ -206,8 +206,8 @@ Statement Parser::parseStatement() elementary = parseElementaryOperation(); } - Assignment assignment = - createWithLocation(std::get(elementary).location); + Assignment assignment; + assignment.location = std::get(elementary).location; assignment.variableNames = std::move(variableNames); expectToken(Token::AssemblyAssign); diff --git a/libyul/AsmParser.h b/libyul/AsmParser.h index 42663ae0a..6148ef725 100644 --- a/libyul/AsmParser.h +++ b/libyul/AsmParser.h @@ -60,18 +60,11 @@ public: protected: using ElementaryOperation = std::variant; - /// Creates an inline assembly node with the given source location. - template T createWithLocation(langutil::SourceLocation const& _loc = {}) const + /// Creates an inline assembly node with the current source location. + template T createWithLocation() const { T r; - r.location = _loc; - if (!r.location.hasText()) - { - r.location.start = position(); - r.location.end = endPosition(); - } - if (!r.location.source) - r.location.source = m_scanner->charStream(); + r.location = location(); return r; } langutil::SourceLocation location() const { return {position(), endPosition(), m_scanner->charStream()}; } From 4ec4d23886cf0f686c56d36d1f501b5ac4a8aab3 Mon Sep 17 00:00:00 2001 From: a3d4 Date: Thu, 6 Feb 2020 00:04:18 +0100 Subject: [PATCH 2/2] Replaced ParserBase::position() and ParserBase::endPosition() with ParserBase::currentLocation(). It might be simpler to pass `SourceLocation` object instead of splitting it into `start` and `end`, and creating another SourceLocation object using the same `start` and `end` later. --- liblangutil/ParserBase.cpp | 19 +++++++------------ liblangutil/ParserBase.h | 6 ++---- liblangutil/Scanner.cpp | 1 + libsolidity/parsing/Parser.cpp | 14 +++++++------- libyul/AsmParser.cpp | 24 ++++++++++++------------ libyul/AsmParser.h | 3 +-- 6 files changed, 30 insertions(+), 37 deletions(-) diff --git a/liblangutil/ParserBase.cpp b/liblangutil/ParserBase.cpp index 3c0fe0b89..747cb37cf 100644 --- a/liblangutil/ParserBase.cpp +++ b/liblangutil/ParserBase.cpp @@ -28,14 +28,9 @@ using namespace std; using namespace solidity; using namespace solidity::langutil; -int ParserBase::position() const +SourceLocation ParserBase::currentLocation() const { - return m_scanner->currentLocation().start; -} - -int ParserBase::endPosition() const -{ - return m_scanner->currentLocation().end; + return m_scanner->currentLocation(); } Token ParserBase::currentToken() const @@ -101,8 +96,8 @@ void ParserBase::expectTokenOrConsumeUntil(Token _value, string const& _currentN Token tok = m_scanner->currentToken(); if (tok != _value) { - int startPosition = position(); - SourceLocation errorLoc = SourceLocation{startPosition, endPosition(), source()}; + SourceLocation errorLoc = currentLocation(); + int startPosition = errorLoc.start; while (m_scanner->currentToken() != _value && m_scanner->currentToken() != Token::EOS) m_scanner->next(); @@ -150,7 +145,7 @@ void ParserBase::decreaseRecursionDepth() void ParserBase::parserWarning(string const& _description) { - m_errorReporter.warning(SourceLocation{position(), endPosition(), source()}, _description); + m_errorReporter.warning(currentLocation(), _description); } void ParserBase::parserError(SourceLocation const& _location, string const& _description) @@ -160,12 +155,12 @@ void ParserBase::parserError(SourceLocation const& _location, string const& _des void ParserBase::parserError(string const& _description) { - parserError(SourceLocation{position(), endPosition(), source()}, _description); + parserError(currentLocation(), _description); } void ParserBase::fatalParserError(string const& _description) { - fatalParserError(SourceLocation{position(), endPosition(), source()}, _description); + fatalParserError(currentLocation(), _description); } void ParserBase::fatalParserError(SourceLocation const& _location, string const& _description) diff --git a/liblangutil/ParserBase.h b/liblangutil/ParserBase.h index 08f2a9dad..575a049e2 100644 --- a/liblangutil/ParserBase.h +++ b/liblangutil/ParserBase.h @@ -62,10 +62,8 @@ protected: ParserBase& m_parser; }; - /// Start position of the current token - int position() const; - /// End position of the current token - int endPosition() const; + /// Location of the current token + SourceLocation currentLocation() const; ///@{ ///@name Helper functions diff --git a/liblangutil/Scanner.cpp b/liblangutil/Scanner.cpp index f98327405..9b9ad8847 100644 --- a/liblangutil/Scanner.cpp +++ b/liblangutil/Scanner.cpp @@ -681,6 +681,7 @@ void Scanner::scanToken() } while (token == Token::Whitespace); m_tokens[NextNext].location.end = sourcePos(); + m_tokens[NextNext].location.source = m_source; m_tokens[NextNext].token = token; m_tokens[NextNext].extendedTokenInfo = make_tuple(m, n); } diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index c6f139dbf..99aa6e2a1 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -45,11 +45,11 @@ class Parser::ASTNodeFactory { public: explicit ASTNodeFactory(Parser& _parser): - m_parser(_parser), m_location{_parser.position(), -1, _parser.source()} {} + m_parser(_parser), m_location{_parser.currentLocation().start, -1, _parser.currentLocation().source} {} ASTNodeFactory(Parser& _parser, ASTPointer const& _childNode): m_parser(_parser), m_location{_childNode->location()} {} - void markEndPosition() { m_location.end = m_parser.endPosition(); } + void markEndPosition() { m_location.end = m_parser.currentLocation().end; } void setLocation(SourceLocation const& _location) { m_location = _location; } void setLocationEmpty() { m_location.end = m_location.start; } /// Set the end position to the one of the given node. @@ -218,12 +218,12 @@ ASTPointer Parser::parseImportDirective() while (true) { ASTPointer alias; - SourceLocation aliasLocation = SourceLocation{position(), endPosition(), source()}; + SourceLocation aliasLocation = currentLocation(); ASTPointer id = parseIdentifier(); if (m_scanner->currentToken() == Token::As) { expectToken(Token::As); - aliasLocation = SourceLocation{position(), endPosition(), source()}; + aliasLocation = currentLocation(); alias = expectIdentifierToken(); } symbolAliases.emplace_back(ImportDirective::SymbolAlias{move(id), move(alias), aliasLocation}); @@ -1189,7 +1189,7 @@ ASTPointer Parser::parseStatement() ASTPointer Parser::parseInlineAssembly(ASTPointer const& _docString) { RecursionGuard recursionGuard(*this); - SourceLocation location{position(), -1, source()}; + SourceLocation location = currentLocation(); expectToken(Token::Assembly); yul::Dialect const& dialect = yul::EVMDialect::strictAssemblyForEVM(m_evmVersion); @@ -2024,13 +2024,13 @@ Parser::IndexAccessedPath Parser::parseIndexAccessedPath() ASTPointer endIndex; if (m_scanner->currentToken() != Token::RBrack) endIndex = parseExpression(); - indexLocation.end = endPosition(); + indexLocation.end = currentLocation().end; iap.indices.emplace_back(IndexAccessedPath::Index{index, {endIndex}, indexLocation}); expectToken(Token::RBrack); } else { - indexLocation.end = endPosition(); + indexLocation.end = currentLocation().end; iap.indices.emplace_back(IndexAccessedPath::Index{index, {}, indexLocation}); expectToken(Token::RBrack); } diff --git a/libyul/AsmParser.cpp b/libyul/AsmParser.cpp index 6d251c3ba..708e76339 100644 --- a/libyul/AsmParser.cpp +++ b/libyul/AsmParser.cpp @@ -88,7 +88,7 @@ Block Parser::parseBlock() expectToken(Token::LBrace); while (currentToken() != Token::RBrace) block.statements.emplace_back(parseStatement()); - block.location.end = endPosition(); + block.location.end = currentLocation().end; advance(); return block; } @@ -151,7 +151,7 @@ Statement Parser::parseStatement() { Statement stmt{createWithLocation()}; if (!m_insideFunction) - m_errorReporter.syntaxError(location(), "Keyword \"leave\" can only be used inside a function."); + m_errorReporter.syntaxError(currentLocation(), "Keyword \"leave\" can only be used inside a function."); m_scanner->next(); return stmt; } @@ -328,13 +328,13 @@ Parser::ElementaryOperation Parser::parseElementaryOperation() YulString literal{currentLiteral()}; if (m_dialect.builtin(literal)) { - Identifier identifier{location(), literal}; + Identifier identifier{currentLocation(), literal}; advance(); expectToken(Token::LParen, false); return FunctionCall{identifier.location, identifier, {}}; } else - ret = Identifier{location(), literal}; + ret = Identifier{currentLocation(), literal}; advance(); break; } @@ -363,7 +363,7 @@ Parser::ElementaryOperation Parser::parseElementaryOperation() } Literal literal{ - location(), + currentLocation(), kind, YulString{currentLiteral()}, kind == LiteralKind::Boolean ? m_dialect.boolType : m_dialect.defaultType @@ -372,7 +372,7 @@ Parser::ElementaryOperation Parser::parseElementaryOperation() if (currentToken() == Token::Colon) { expectToken(Token::Colon); - literal.location.end = endPosition(); + literal.location.end = currentLocation().end; literal.type = expectAsmIdentifier(); } @@ -415,7 +415,7 @@ FunctionDefinition Parser::parseFunctionDefinition() if (m_currentForLoopComponent == ForLoopComponent::ForLoopPre) m_errorReporter.syntaxError( - location(), + currentLocation(), "Functions cannot be defined inside a for-loop init block." ); @@ -481,7 +481,7 @@ Expression Parser::parseCall(Parser::ElementaryOperation&& _initialOp) ret.arguments.emplace_back(parseExpression()); } } - ret.location.end = endPosition(); + ret.location.end = currentLocation().end; expectToken(Token::RParen); return ret; } @@ -494,7 +494,7 @@ TypedName Parser::parseTypedName() if (currentToken() == Token::Colon) { expectToken(Token::Colon); - typedName.location.end = endPosition(); + typedName.location.end = currentLocation().end; typedName.type = expectAsmIdentifier(); } else @@ -530,13 +530,13 @@ void Parser::checkBreakContinuePosition(string const& _which) switch (m_currentForLoopComponent) { case ForLoopComponent::None: - m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" needs to be inside a for-loop body."); + m_errorReporter.syntaxError(currentLocation(), "Keyword \"" + _which + "\" needs to be inside a for-loop body."); break; case ForLoopComponent::ForLoopPre: - m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" in for-loop init block is not allowed."); + m_errorReporter.syntaxError(currentLocation(), "Keyword \"" + _which + "\" in for-loop init block is not allowed."); break; case ForLoopComponent::ForLoopPost: - m_errorReporter.syntaxError(location(), "Keyword \"" + _which + "\" in for-loop post block is not allowed."); + m_errorReporter.syntaxError(currentLocation(), "Keyword \"" + _which + "\" in for-loop post block is not allowed."); break; case ForLoopComponent::ForLoopBody: break; diff --git a/libyul/AsmParser.h b/libyul/AsmParser.h index 6148ef725..54c79a0e7 100644 --- a/libyul/AsmParser.h +++ b/libyul/AsmParser.h @@ -64,10 +64,9 @@ protected: template T createWithLocation() const { T r; - r.location = location(); + r.location = currentLocation(); return r; } - langutil::SourceLocation location() const { return {position(), endPosition(), m_scanner->charStream()}; } Block parseBlock(); Statement parseStatement();