diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp index 329408832..e5c3ec2f8 100644 --- a/libsolidity/analysis/PostTypeChecker.cpp +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -86,6 +86,16 @@ void PostTypeChecker::endVisit(EmitStatement const& _emit) callEndVisit(_emit); } +bool PostTypeChecker::visit(RevertStatement const& _revert) +{ + return callVisit(_revert); +} + +void PostTypeChecker::endVisit(RevertStatement const& _revert) +{ + callEndVisit(_revert); +} + bool PostTypeChecker::visit(FunctionCall const& _functionCall) { return callVisit(_functionCall); @@ -281,42 +291,59 @@ private: bool m_insideModifierInvocation = false; }; -struct EventOutsideEmitChecker: public PostTypeChecker::Checker +struct EventOutsideEmitErrorOutsideRevertChecker: public PostTypeChecker::Checker { - EventOutsideEmitChecker(ErrorReporter& _errorReporter): + EventOutsideEmitErrorOutsideRevertChecker(ErrorReporter& _errorReporter): Checker(_errorReporter) {} - bool visit(EmitStatement const&) override + bool visit(EmitStatement const& _emitStatement) override { - m_insideEmitStatement = true; + m_currentStatement = &_emitStatement; return true; } void endVisit(EmitStatement const&) override { - m_insideEmitStatement = true; + m_currentStatement = nullptr; + } + + bool visit(RevertStatement const& _revertStatement) override + { + m_currentStatement = &_revertStatement; + return true; + } + + void endVisit(RevertStatement const&) override + { + m_currentStatement = nullptr; } bool visit(FunctionCall const& _functionCall) override { - if (*_functionCall.annotation().kind != FunctionCallKind::FunctionCall) - return true; - - if (FunctionTypePointer const functionType = dynamic_cast(_functionCall.expression().annotation().type)) - // Check for event outside of emit statement - if (!m_insideEmitStatement && functionType->kind() == FunctionType::Kind::Event) - m_errorReporter.typeError( - 3132_error, - _functionCall.location(), - "Event invocations have to be prefixed by \"emit\"." - ); + if (*_functionCall.annotation().kind == FunctionCallKind::FunctionCall) + if (auto const* functionType = dynamic_cast(_functionCall.expression().annotation().type)) + { + // Check for event outside of emit statement + if (!dynamic_cast(m_currentStatement) && functionType->kind() == FunctionType::Kind::Event) + m_errorReporter.typeError( + 3132_error, + _functionCall.location(), + "Event invocations have to be prefixed by \"emit\"." + ); + else if (!dynamic_cast(m_currentStatement) && functionType->kind() == FunctionType::Kind::Error) + m_errorReporter.typeError( + 7757_error, + _functionCall.location(), + "Errors can only be used with revert statements: \"revert MyError();\"." + ); + } + m_currentStatement = nullptr; return true; } private: - /// Flag indicating whether we are currently inside an EmitStatement. - bool m_insideEmitStatement = false; + Statement const* m_currentStatement = nullptr; }; struct NoVariablesInInterfaceChecker: public PostTypeChecker::Checker @@ -395,14 +422,16 @@ struct ReservedErrorSelector: public PostTypeChecker::Checker } } }; + } + PostTypeChecker::PostTypeChecker(langutil::ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) { m_checkers.push_back(make_shared(_errorReporter)); m_checkers.push_back(make_shared(_errorReporter)); m_checkers.push_back(make_shared(_errorReporter)); - m_checkers.push_back(make_shared(_errorReporter)); + m_checkers.push_back(make_shared(_errorReporter)); m_checkers.push_back(make_shared(_errorReporter)); m_checkers.push_back(make_shared(_errorReporter)); } diff --git a/libsolidity/analysis/PostTypeChecker.h b/libsolidity/analysis/PostTypeChecker.h index f35330fa1..04b4fbe7a 100644 --- a/libsolidity/analysis/PostTypeChecker.h +++ b/libsolidity/analysis/PostTypeChecker.h @@ -83,6 +83,9 @@ private: bool visit(EmitStatement const& _emit) override; void endVisit(EmitStatement const& _emit) override; + bool visit(RevertStatement const& _revert) override; + void endVisit(RevertStatement const& _revert) override; + bool visit(FunctionCall const& _functionCall) override; bool visit(Identifier const& _identifier) override; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 8c99c8396..f5aecd252 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -1167,6 +1167,18 @@ void TypeChecker::endVisit(EmitStatement const& _emit) m_errorReporter.typeError(9292_error, _emit.eventCall().expression().location(), "Expression has to be an event invocation."); } +void TypeChecker::endVisit(RevertStatement const& _revert) +{ + FunctionCall const& errorCall = _revert.errorCall(); + if ( + *errorCall.annotation().kind != FunctionCallKind::FunctionCall || + type(errorCall.expression())->category() != Type::Category::Function || + dynamic_cast(*type(errorCall.expression())).kind() != FunctionType::Kind::Error + ) + m_errorReporter.typeError(1885_error, errorCall.expression().location(), "Expression has to be an error."); +} + + bool TypeChecker::visit(VariableDeclarationStatement const& _statement) { if (!_statement.initialValue()) diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index efbf99ada..a9604bdec 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -134,6 +134,7 @@ private: bool visit(ForStatement const& _forStatement) override; void endVisit(Return const& _return) override; void endVisit(EmitStatement const& _emit) override; + void endVisit(RevertStatement const& _revert) override; bool visit(VariableDeclarationStatement const& _variable) override; void endVisit(ExpressionStatement const& _statement) override; bool visit(Conditional const& _conditional) override; diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 1f36875e5..6d58f504a 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -1726,6 +1726,31 @@ public: void accept(ASTConstVisitor& _visitor) const override; }; +/** + * The revert statement is used to revert state changes and return error data. + */ +class RevertStatement: public Statement +{ +public: + explicit RevertStatement( + int64_t _id, + SourceLocation const& _location, + ASTPointer const& _docString, + ASTPointer _functionCall + ): + Statement(_id, _location, _docString), m_errorCall(std::move(_functionCall)) + { + solAssert(m_errorCall, ""); + } + void accept(ASTVisitor& _visitor) override; + void accept(ASTConstVisitor& _visitor) const override; + + FunctionCall const& errorCall() const { return *m_errorCall; } + +private: + ASTPointer m_errorCall; +}; + /** * The emit statement is used to emit events: emit EventName(arg1, ..., argn) */ diff --git a/libsolidity/ast/ASTVisitor.h b/libsolidity/ast/ASTVisitor.h index 4acb47273..398387b33 100644 --- a/libsolidity/ast/ASTVisitor.h +++ b/libsolidity/ast/ASTVisitor.h @@ -90,6 +90,7 @@ public: virtual bool visit(Return& _node) { return visitNode(_node); } virtual bool visit(Throw& _node) { return visitNode(_node); } virtual bool visit(EmitStatement& _node) { return visitNode(_node); } + virtual bool visit(RevertStatement& _node) { return visitNode(_node); } virtual bool visit(VariableDeclarationStatement& _node) { return visitNode(_node); } virtual bool visit(ExpressionStatement& _node) { return visitNode(_node); } virtual bool visit(Conditional& _node) { return visitNode(_node); } @@ -144,6 +145,7 @@ public: virtual void endVisit(Return& _node) { endVisitNode(_node); } virtual void endVisit(Throw& _node) { endVisitNode(_node); } virtual void endVisit(EmitStatement& _node) { endVisitNode(_node); } + virtual void endVisit(RevertStatement& _node) { endVisitNode(_node); } virtual void endVisit(VariableDeclarationStatement& _node) { endVisitNode(_node); } virtual void endVisit(ExpressionStatement& _node) { endVisitNode(_node); } virtual void endVisit(Conditional& _node) { endVisitNode(_node); } @@ -220,6 +222,7 @@ public: virtual bool visit(Return const& _node) { return visitNode(_node); } virtual bool visit(Throw const& _node) { return visitNode(_node); } virtual bool visit(EmitStatement const& _node) { return visitNode(_node); } + virtual bool visit(RevertStatement const& _node) { return visitNode(_node); } virtual bool visit(VariableDeclarationStatement const& _node) { return visitNode(_node); } virtual bool visit(ExpressionStatement const& _node) { return visitNode(_node); } virtual bool visit(Conditional const& _node) { return visitNode(_node); } @@ -274,6 +277,7 @@ public: virtual void endVisit(Return const& _node) { endVisitNode(_node); } virtual void endVisit(Throw const& _node) { endVisitNode(_node); } virtual void endVisit(EmitStatement const& _node) { endVisitNode(_node); } + virtual void endVisit(RevertStatement const& _node) { endVisitNode(_node); } virtual void endVisit(VariableDeclarationStatement const& _node) { endVisitNode(_node); } virtual void endVisit(ExpressionStatement const& _node) { endVisitNode(_node); } virtual void endVisit(Conditional const& _node) { endVisitNode(_node); } diff --git a/libsolidity/ast/AST_accept.h b/libsolidity/ast/AST_accept.h index 678a9ef7e..075e6fe5c 100644 --- a/libsolidity/ast/AST_accept.h +++ b/libsolidity/ast/AST_accept.h @@ -682,6 +682,20 @@ void Throw::accept(ASTConstVisitor& _visitor) const _visitor.endVisit(*this); } +void RevertStatement::accept(ASTVisitor& _visitor) +{ + if (_visitor.visit(*this)) + m_errorCall->accept(_visitor); + _visitor.endVisit(*this); +} + +void RevertStatement::accept(ASTConstVisitor& _visitor) const +{ + if (_visitor.visit(*this)) + m_errorCall->accept(_visitor); + _visitor.endVisit(*this); +} + void EmitStatement::accept(ASTVisitor& _visitor) { if (_visitor.visit(*this)) diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index e05134310..10d396c4c 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -1250,11 +1250,13 @@ ASTPointer Parser::parseStatement(bool _allowUnchecked) statement = parseEmitStatement(docString); break; case Token::Identifier: - if (m_insideModifier && m_scanner->currentLiteral() == "_") - { - statement = ASTNodeFactory(*this).createNode(docString); - m_scanner->next(); - } + if (m_scanner->currentLiteral() == "revert" && m_scanner->peekNextToken() == Token::Identifier) + statement = parseRevertStatement(docString); + else if (m_insideModifier && m_scanner->currentLiteral() == "_") + { + statement = ASTNodeFactory(*this).createNode(docString); + m_scanner->next(); + } else statement = parseSimpleStatement(docString); break; @@ -1474,8 +1476,38 @@ ASTPointer Parser::parseEmitStatement(ASTPointer const nodeFactory.markEndPosition(); expectToken(Token::RParen); auto eventCall = eventCallNodeFactory.createNode(eventName, arguments, names); - auto statement = nodeFactory.createNode(_docString, eventCall); - return statement; + return nodeFactory.createNode(_docString, eventCall); +} + +ASTPointer Parser::parseRevertStatement(ASTPointer const& _docString) +{ + ASTNodeFactory nodeFactory(*this); + solAssert(*expectIdentifierToken() == "revert", ""); + + ASTNodeFactory errorCallNodeFactory(*this); + + solAssert(m_scanner->currentToken() == Token::Identifier, ""); + + IndexAccessedPath iap; + while (true) + { + iap.path.push_back(parseIdentifier()); + if (m_scanner->currentToken() != Token::Period) + break; + m_scanner->next(); + } + + auto errorName = expressionFromIndexAccessStructure(iap); + expectToken(Token::LParen); + + vector> arguments; + vector> names; + std::tie(arguments, names) = parseFunctionCallArguments(); + errorCallNodeFactory.markEndPosition(); + nodeFactory.markEndPosition(); + expectToken(Token::RParen); + auto errorCall = errorCallNodeFactory.createNode(errorName, arguments, names); + return nodeFactory.createNode(_docString, errorCall); } ASTPointer Parser::parseSimpleStatement(ASTPointer const& _docString) diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h index dd9125e5b..559f804c2 100644 --- a/libsolidity/parsing/Parser.h +++ b/libsolidity/parsing/Parser.h @@ -127,6 +127,7 @@ private: ASTPointer parseDoWhileStatement(ASTPointer const& _docString); ASTPointer parseForStatement(ASTPointer const& _docString); ASTPointer parseEmitStatement(ASTPointer const& docString); + ASTPointer parseRevertStatement(ASTPointer const& docString); /// A "simple statement" can be a variable declaration statement or an expression statement. ASTPointer parseSimpleStatement(ASTPointer const& _docString); ASTPointer parseVariableDeclarationStatement( diff --git a/test/libsolidity/semanticTests/errors/panic_via_import.sol b/test/libsolidity/semanticTests/errors/panic_via_import.sol new file mode 100644 index 000000000..bdd8d5e4b --- /dev/null +++ b/test/libsolidity/semanticTests/errors/panic_via_import.sol @@ -0,0 +1,18 @@ +==== Source: s1.sol ==== +error E(uint); +==== Source: s2.sol ==== +import { E as Panic } from "s1.sol"; +contract C { + error E(uint); + function a() public pure { + revert Panic(1); + } + function b() public pure { + revert E(1); + } +} +// ==== +// compileViaYul: also +// ---- +// a() -> FAILURE, hex"002ff067", hex"0000000000000000000000000000000000000000000000000000000000000001" +// b() -> FAILURE, hex"002ff067", hex"0000000000000000000000000000000000000000000000000000000000000001" \ No newline at end of file diff --git a/test/libsolidity/semanticTests/errors/using_structs.sol b/test/libsolidity/semanticTests/errors/using_structs.sol new file mode 100644 index 000000000..c9422c4ac --- /dev/null +++ b/test/libsolidity/semanticTests/errors/using_structs.sol @@ -0,0 +1,20 @@ +pragma abicoder v2; +struct S { uint a; string b; } +error E(uint a, S s, uint b); +contract C { + S s; + function f(bool c) public { + if (c) { + s.a = 9; + s.b = "abc"; + revert E(2, s, 7); + } else { + revert E({b: 7, a: 2, s: S({b: "abc", a: 9})}); + } + } +} +// ==== +// compileViaYul: also +// ---- +// f(bool): true -> FAILURE, hex"e96e07f0", hex"0000000000000000000000000000000000000000000000000000000000000002", hex"0000000000000000000000000000000000000000000000000000000000000060", hex"0000000000000000000000000000000000000000000000000000000000000007", hex"0000000000000000000000000000000000000000000000000000000000000009", hex"0000000000000000000000000000000000000000000000000000000000000040", hex"0000000000000000000000000000000000000000000000000000000000000003", hex"6162630000000000000000000000000000000000000000000000000000000000" +// f(bool): false -> FAILURE, hex"e96e07f0", hex"0000000000000000000000000000000000000000000000000000000000000002", hex"0000000000000000000000000000000000000000000000000000000000000060", hex"0000000000000000000000000000000000000000000000000000000000000007", hex"0000000000000000000000000000000000000000000000000000000000000009", hex"0000000000000000000000000000000000000000000000000000000000000040", hex"0000000000000000000000000000000000000000000000000000000000000003", hex"6162630000000000000000000000000000000000000000000000000000000000" diff --git a/test/libsolidity/syntaxTests/errors/require_custom.sol b/test/libsolidity/syntaxTests/errors/require_custom.sol new file mode 100644 index 000000000..f139a4fe5 --- /dev/null +++ b/test/libsolidity/syntaxTests/errors/require_custom.sol @@ -0,0 +1,8 @@ +error E(uint a, uint b); +contract C { + function f(bool c) public pure { + require(c, E(2, 7)); + } +} +// ---- +// TypeError 9322: (83-90): No matching declaration found after argument-dependent lookup. diff --git a/test/libsolidity/syntaxTests/errors/revert_parentheses.sol b/test/libsolidity/syntaxTests/errors/revert_parentheses.sol new file mode 100644 index 000000000..27c7044e5 --- /dev/null +++ b/test/libsolidity/syntaxTests/errors/revert_parentheses.sol @@ -0,0 +1,8 @@ +error E(uint a, uint b); +contract C { + function f() public pure { + revert(E(2, 7)); + } +} +// ---- +// TypeError 9322: (77-83): No matching declaration found after argument-dependent lookup. diff --git a/test/libsolidity/syntaxTests/revertStatement/at_file_level.sol b/test/libsolidity/syntaxTests/revertStatement/at_file_level.sol new file mode 100644 index 000000000..b2d53bd8b --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/at_file_level.sol @@ -0,0 +1,3 @@ +revert X(); +// ---- +// ParserError 2314: (8-9): Expected ';' but got '(' diff --git a/test/libsolidity/syntaxTests/revertStatement/error_used_elsewhere.sol b/test/libsolidity/syntaxTests/revertStatement/error_used_elsewhere.sol new file mode 100644 index 000000000..208a7326b --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/error_used_elsewhere.sol @@ -0,0 +1,6 @@ +error E(); +function f() pure { + E(); +} +// ---- +// TypeError 7757: (35-38): Errors can only be used with revert statements: "revert MyError();". diff --git a/test/libsolidity/syntaxTests/revertStatement/in_global_function.sol b/test/libsolidity/syntaxTests/revertStatement/in_global_function.sol new file mode 100644 index 000000000..a8ea0e444 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/in_global_function.sol @@ -0,0 +1,4 @@ +error E(); +function f() pure { + revert E(); +} diff --git a/test/libsolidity/syntaxTests/revertStatement/non_called.sol b/test/libsolidity/syntaxTests/revertStatement/non_called.sol new file mode 100644 index 000000000..63ecfbc6e --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/non_called.sol @@ -0,0 +1,6 @@ +error E(); +function f() public pure { + revert E; +} +// ---- +// ParserError 2314: (50-51): Expected '(' but got ';' diff --git a/test/libsolidity/syntaxTests/revertStatement/non_error.sol b/test/libsolidity/syntaxTests/revertStatement/non_error.sol new file mode 100644 index 000000000..329b68f93 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/non_error.sol @@ -0,0 +1,5 @@ +function f() public pure { + revert 1; +} +// ---- +// ParserError 2314: (38-39): Expected ';' but got 'Number' diff --git a/test/libsolidity/syntaxTests/revertStatement/regular.sol b/test/libsolidity/syntaxTests/revertStatement/regular.sol new file mode 100644 index 000000000..57de7a0e4 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/regular.sol @@ -0,0 +1,6 @@ +error E(); +contract C { + function f() public pure { + revert E(); + } +} \ No newline at end of file diff --git a/test/libsolidity/syntaxTests/revertStatement/revert_event.sol b/test/libsolidity/syntaxTests/revertStatement/revert_event.sol new file mode 100644 index 000000000..6b2832c87 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/revert_event.sol @@ -0,0 +1,8 @@ +contract C { + event E(); + function f() public pure { + revert E(); + } +} +// ---- +// TypeError 1885: (74-75): Expression has to be an error. diff --git a/test/libsolidity/syntaxTests/revertStatement/revert_revert.sol b/test/libsolidity/syntaxTests/revertStatement/revert_revert.sol new file mode 100644 index 000000000..f3b1e0805 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/revert_revert.sol @@ -0,0 +1,8 @@ +error revert(); +contract C { + function f() public pure { + revert revert(); + } +} +// ---- +// Warning 2319: (0-15): This declaration shadows a builtin symbol. diff --git a/test/libsolidity/syntaxTests/revertStatement/scoped.sol b/test/libsolidity/syntaxTests/revertStatement/scoped.sol new file mode 100644 index 000000000..ec8b6e601 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/scoped.sol @@ -0,0 +1,8 @@ +contract A { + error E(); +} +contract C { + function f() public pure { + revert A.E(); + } +} \ No newline at end of file diff --git a/test/libsolidity/syntaxTests/revertStatement/using_function.sol b/test/libsolidity/syntaxTests/revertStatement/using_function.sol new file mode 100644 index 000000000..e10384cd7 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/using_function.sol @@ -0,0 +1,9 @@ +error f(uint, uint); +contract C { + function f(uint) public { + revert f(10); + } +} +// ---- +// Warning 2519: (38-91): This declaration shadows an existing declaration. +// TypeError 1885: (79-80): Expression has to be an error. diff --git a/test/libsolidity/syntaxTests/revertStatement/using_struct.sol b/test/libsolidity/syntaxTests/revertStatement/using_struct.sol new file mode 100644 index 000000000..f04b0d598 --- /dev/null +++ b/test/libsolidity/syntaxTests/revertStatement/using_struct.sol @@ -0,0 +1,8 @@ +struct S { uint x; } +contract C { + function f() public { + revert S(10); + } +} +// ---- +// TypeError 1885: (75-76): Expression has to be an error.