Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert statement. #11037

Merged
merged 7 commits into from
Mar 30, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion docs/grammar/Solidity.g4
Original file line number Diff line number Diff line change
Expand Up @@ -381,7 +381,7 @@ inlineArrayExpression: LBrack (expression ( Comma expression)* ) RBrack;
/**
* Besides regular non-keyword Identifiers, some keywords like 'from' and 'error' can also be used as identifiers.
*/
identifier: Identifier | From | Error;
identifier: Identifier | From | Error | Revert;

literal: stringLiteral | numberLiteral | booleanLiteral | hexStringLiteral | unicodeStringLiteral;
booleanLiteral: True | False;
Expand Down Expand Up @@ -422,6 +422,7 @@ statement:
| tryStatement
| returnStatement
| emitStatement
| revertStatement
| assemblyStatement
;

Expand Down Expand Up @@ -459,6 +460,10 @@ returnStatement: Return expression? Semicolon;
* An emit statement. The contained expression needs to refer to an event.
*/
emitStatement: Emit expression callArgumentList Semicolon;
/**
* A revert statement. The contained expression needs to refer to an error.
*/
revertStatement: Revert expression callArgumentList Semicolon;
/**
* An inline assembly block.
* The contents of an inline assembly block use a separate scanner/lexer, i.e. the set of keywords and
Expand Down
1 change: 1 addition & 0 deletions docs/grammar/SolidityLexer.g4
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ Else: 'else';
Emit: 'emit';
Enum: 'enum';
Error: 'error'; // not a real keyword
Revert: 'revert'; // not a real keyword
Event: 'event';
External: 'external';
Fallback: 'fallback';
Expand Down
2 changes: 2 additions & 0 deletions libsolidity/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ set(sources
analysis/OverrideChecker.h
analysis/PostTypeChecker.cpp
analysis/PostTypeChecker.h
analysis/PostTypeContractLevelChecker.cpp
analysis/PostTypeContractLevelChecker.h
analysis/ReferencesResolver.cpp
analysis/ReferencesResolver.h
analysis/Scoper.cpp
Expand Down
18 changes: 0 additions & 18 deletions libsolidity/analysis/ContractLevelChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -433,24 +433,6 @@ void ContractLevelChecker::checkHashCollisions(ContractDefinition const& _contra
);
hashes.insert(hash);
}

map<uint32_t, SourceLocation> errorHashes;
for (ErrorDefinition const* error: _contract.interfaceErrors())
{
if (!error->functionType(true)->interfaceFunctionType())
// This will result in an error later on, so we can ignore it here.
continue;
uint32_t hash = selectorFromSignature32(error->functionType(true)->externalSignature());
if (errorHashes.count(hash))
m_errorReporter.typeError(
4883_error,
_contract.location(),
SecondarySourceLocation{}.append("This error has the same selector: "s, errorHashes[hash]),
"Error signature hash collision for " + error->functionType(true)->externalSignature()
);
else
errorHashes[hash] = error->location();
}
}

void ContractLevelChecker::checkLibraryRequirements(ContractDefinition const& _contract)
Expand Down
10 changes: 10 additions & 0 deletions libsolidity/analysis/ControlFlowBuilder.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,16 @@ bool ControlFlowBuilder::visit(Throw const& _throw)
return false;
}

bool ControlFlowBuilder::visit(RevertStatement const& _revert)
{
solAssert(!!m_currentNode, "");
solAssert(!!m_revertNode, "");
visitNode(_revert);
connect(m_currentNode, m_revertNode);
m_currentNode = newLabel();
return false;
}

bool ControlFlowBuilder::visit(PlaceholderStatement const&)
{
solAssert(!!m_currentNode, "");
Expand Down
1 change: 1 addition & 0 deletions libsolidity/analysis/ControlFlowBuilder.h
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class ControlFlowBuilder: private ASTConstVisitor, private yul::ASTWalker
bool visit(Break const&) override;
bool visit(Continue const&) override;
bool visit(Throw const&) override;
bool visit(RevertStatement const&) override;
bool visit(PlaceholderStatement const&) override;
bool visit(FunctionCall const& _functionCall) override;
bool visit(ModifierInvocation const& _modifierInvocation) override;
Expand Down
2 changes: 2 additions & 0 deletions libsolidity/analysis/FunctionCallGraph.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,8 @@ bool FunctionCallGraphBuilder::visit(FunctionCall const& _functionCall)
// change at runtime). All we can do is to add an edge to the dispatch which in turn has
// edges to all functions could possibly be called.
add(m_currentNode, CallGraph::SpecialNode::InternalDispatch);
else if (functionType->kind() == FunctionType::Kind::Error)
m_graph.usedErrors.insert(&dynamic_cast<ErrorDefinition const&>(functionType->declaration()));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe we should add an assert here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean as else? Not really. There are other case possible here, the graph just skips them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a check and I would say it is highly unlikely for this assert to trigger...


return true;
}
Expand Down
4 changes: 2 additions & 2 deletions libsolidity/analysis/FunctionCallGraph.h
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,8 @@ class FunctionCallGraphBuilder: private ASTConstVisitor

private:
FunctionCallGraphBuilder(ContractDefinition const& _contract):
m_contract(_contract),
m_graph{{}, {}, {}} {}
m_contract(_contract)
{}

bool visit(FunctionCall const& _functionCall) override;
bool visit(EmitStatement const& _emitStatement) override;
Expand Down
67 changes: 48 additions & 19 deletions libsolidity/analysis/PostTypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -281,42 +291,59 @@ struct ModifierContextChecker: public PostTypeChecker::Checker
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(FunctionCall const& _functionCall) override
bool visit(RevertStatement const& _revertStatement) override
{
if (*_functionCall.annotation().kind != FunctionCallKind::FunctionCall)
return true;
m_currentStatement = &_revertStatement;
return true;
}

if (FunctionTypePointer const functionType = dynamic_cast<FunctionTypePointer const>(_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\"."
);
void endVisit(RevertStatement const&) override
{
m_currentStatement = nullptr;
}

bool visit(FunctionCall const& _functionCall) override
{
if (*_functionCall.annotation().kind == FunctionCallKind::FunctionCall)
if (auto const* functionType = dynamic_cast<FunctionType const*>(_functionCall.expression().annotation().type))
{
// Check for event outside of emit statement
if (!dynamic_cast<EmitStatement const*>(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<RevertStatement const*>(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
Expand Down Expand Up @@ -395,14 +422,16 @@ struct ReservedErrorSelector: public PostTypeChecker::Checker
}
}
};

}


PostTypeChecker::PostTypeChecker(langutil::ErrorReporter& _errorReporter): m_errorReporter(_errorReporter)
{
m_checkers.push_back(make_shared<ConstStateVarCircularReferenceChecker>(_errorReporter));
m_checkers.push_back(make_shared<OverrideSpecifierChecker>(_errorReporter));
m_checkers.push_back(make_shared<ModifierContextChecker>(_errorReporter));
m_checkers.push_back(make_shared<EventOutsideEmitChecker>(_errorReporter));
m_checkers.push_back(make_shared<EventOutsideEmitErrorOutsideRevertChecker>(_errorReporter));
m_checkers.push_back(make_shared<NoVariablesInInterfaceChecker>(_errorReporter));
m_checkers.push_back(make_shared<ReservedErrorSelector>(_errorReporter));
}
3 changes: 3 additions & 0 deletions libsolidity/analysis/PostTypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ class PostTypeChecker: private ASTConstVisitor
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;
Expand Down
72 changes: 72 additions & 0 deletions libsolidity/analysis/PostTypeContractLevelChecker.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
This file is part of solidity.

solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
// SPDX-License-Identifier: GPL-3.0
/**
* Component that verifies overloads, abstract contracts, function clashes and others
* checks at contract or function level.
*/

#include <libsolidity/analysis/PostTypeContractLevelChecker.h>

#include <libsolidity/ast/AST.h>
#include <libsolutil/FunctionSelector.h>
#include <liblangutil/ErrorReporter.h>

using namespace std;
using namespace solidity;
using namespace solidity::langutil;
using namespace solidity::frontend;

bool PostTypeContractLevelChecker::check(SourceUnit const& _sourceUnit)
{
bool noErrors = true;
for (auto* contract: ASTNode::filteredNodes<ContractDefinition>(_sourceUnit.nodes()))
if (!check(*contract))
noErrors = false;
return noErrors;
}

bool PostTypeContractLevelChecker::check(ContractDefinition const& _contract)
{
solAssert(
_contract.annotation().creationCallGraph.set() &&
_contract.annotation().deployedCallGraph.set(),
""
);

map<uint32_t, map<string, SourceLocation>> errorHashes;
for (ErrorDefinition const* error: _contract.interfaceErrors())
{
string signature = error->functionType(true)->externalSignature();
uint32_t hash = selectorFromSignature32(signature);
// Fail if there is a different signature for the same hash.
if (!errorHashes[hash].empty() && !errorHashes[hash].count(signature))
cameel marked this conversation as resolved.
Show resolved Hide resolved
{
SourceLocation& otherLocation = errorHashes[hash].begin()->second;
m_errorReporter.typeError(
4883_error,
error->nameLocation(),
SecondarySourceLocation{}.append("This error has a different signature but the same hash: ", otherLocation),
"Error signature hash collision for " + error->functionType(true)->externalSignature()
);
}
else
errorHashes[hash][signature] = error->location();
}

return Error::containsOnlyWarnings(m_errorReporter.errors());
}
56 changes: 56 additions & 0 deletions libsolidity/analysis/PostTypeContractLevelChecker.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
/*
This file is part of solidity.

solidity is free software: you can redistribute it and/or modify
it under the terms of the GNU General Public License as published by
the Free Software Foundation, either version 3 of the License, or
(at your option) any later version.

solidity is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

You should have received a copy of the GNU General Public License
along with solidity. If not, see <http://www.gnu.org/licenses/>.
*/
// SPDX-License-Identifier: GPL-3.0
/**
* Component that verifies properties at contract level, after the type checker has run.
*/

#pragma once

#include <libsolidity/ast/ASTForward.h>

namespace solidity::langutil
{
class ErrorReporter;
}

namespace solidity::frontend
{

/**
* Component that verifies properties at contract level, after the type checker has run.
*/
class PostTypeContractLevelChecker
{
public:
explicit PostTypeContractLevelChecker(langutil::ErrorReporter& _errorReporter):
m_errorReporter(_errorReporter)
{}

/// Performs checks on the given source ast.
/// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings
bool check(SourceUnit const& _sourceUnit);

private:
/// Performs checks on the given contract.
/// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings
bool check(ContractDefinition const& _contract);

langutil::ErrorReporter& m_errorReporter;
};

}
12 changes: 12 additions & 0 deletions libsolidity/analysis/TypeChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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<FunctionType const&>(*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())
Expand Down
1 change: 1 addition & 0 deletions libsolidity/analysis/TypeChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,7 @@ class TypeChecker: private ASTConstVisitor
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;
Expand Down
Loading