mirror of
https://github.com/ethereum/solidity
synced 2023-10-03 13:03:40 +00:00
Relax restrictions on immutable initialization
This commit is contained in:
@@ -18,8 +18,6 @@
|
||||
|
||||
#include <libsolidity/analysis/ImmutableValidator.h>
|
||||
|
||||
#include <libsolutil/CommonData.h>
|
||||
|
||||
#include <range/v3/view/reverse.hpp>
|
||||
|
||||
using namespace solidity::frontend;
|
||||
@@ -27,275 +25,44 @@ using namespace solidity::langutil;
|
||||
|
||||
void ImmutableValidator::analyze()
|
||||
{
|
||||
m_inCreationContext = true;
|
||||
|
||||
auto linearizedContracts = m_mostDerivedContract.annotation().linearizedBaseContracts | ranges::views::reverse;
|
||||
|
||||
for (ContractDefinition const* contract: linearizedContracts)
|
||||
for (VariableDeclaration const* stateVar: contract->stateVariables())
|
||||
if (stateVar->value())
|
||||
stateVar->value()->accept(*this);
|
||||
|
||||
for (ContractDefinition const* contract: linearizedContracts)
|
||||
for (std::shared_ptr<InheritanceSpecifier> const& inheritSpec: contract->baseContracts())
|
||||
if (auto args = inheritSpec->arguments())
|
||||
ASTNode::listAccept(*args, *this);
|
||||
|
||||
for (ContractDefinition const* contract: linearizedContracts)
|
||||
{
|
||||
for (VariableDeclaration const* stateVar: contract->stateVariables())
|
||||
if (stateVar->value())
|
||||
m_initializedStateVariables.emplace(stateVar);
|
||||
for (FunctionDefinition const* function: contract->definedFunctions())
|
||||
function->accept(*this);
|
||||
|
||||
if (contract->constructor())
|
||||
visitCallableIfNew(*contract->constructor());
|
||||
for (ModifierDefinition const* modifier: contract->functionModifiers())
|
||||
modifier->accept(*this);
|
||||
}
|
||||
|
||||
m_inCreationContext = false;
|
||||
|
||||
for (ContractDefinition const* contract: linearizedContracts)
|
||||
{
|
||||
for (auto funcDef: contract->definedFunctions())
|
||||
visitCallableIfNew(*funcDef);
|
||||
|
||||
for (auto modDef: contract->functionModifiers())
|
||||
visitCallableIfNew(*modDef);
|
||||
}
|
||||
|
||||
checkAllVariablesInitialized(m_mostDerivedContract.location());
|
||||
}
|
||||
|
||||
bool ImmutableValidator::visit(Assignment const& _assignment)
|
||||
{
|
||||
// Need to visit values first (rhs) as they might access other immutables.
|
||||
_assignment.rightHandSide().accept(*this);
|
||||
_assignment.leftHandSide().accept(*this);
|
||||
return false;
|
||||
}
|
||||
|
||||
|
||||
bool ImmutableValidator::visit(FunctionDefinition const& _functionDefinition)
|
||||
{
|
||||
return analyseCallable(_functionDefinition);
|
||||
return !_functionDefinition.isConstructor();
|
||||
}
|
||||
|
||||
bool ImmutableValidator::visit(ModifierDefinition const& _modifierDefinition)
|
||||
void ImmutableValidator::endVisit(MemberAccess const& _memberAccess)
|
||||
{
|
||||
return analyseCallable(_modifierDefinition);
|
||||
}
|
||||
|
||||
bool ImmutableValidator::visit(MemberAccess const& _memberAccess)
|
||||
{
|
||||
_memberAccess.expression().accept(*this);
|
||||
|
||||
if (auto contractType = dynamic_cast<ContractType const*>(_memberAccess.expression().annotation().type))
|
||||
if (!contractType->isSuper())
|
||||
// external access, no analysis needed.
|
||||
return false;
|
||||
|
||||
if (auto varDecl = dynamic_cast<VariableDeclaration const*>(_memberAccess.annotation().referencedDeclaration))
|
||||
analyseVariableReference(*varDecl, _memberAccess);
|
||||
else if (auto funcType = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type))
|
||||
if (funcType->kind() == FunctionType::Kind::Internal && funcType->hasDeclaration())
|
||||
visitCallableIfNew(funcType->declaration());
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
bool ImmutableValidator::visit(IfStatement const& _ifStatement)
|
||||
{
|
||||
bool prevInBranch = m_inBranch;
|
||||
|
||||
_ifStatement.condition().accept(*this);
|
||||
|
||||
m_inBranch = true;
|
||||
_ifStatement.trueStatement().accept(*this);
|
||||
|
||||
if (auto falseStatement = _ifStatement.falseStatement())
|
||||
falseStatement->accept(*this);
|
||||
|
||||
m_inBranch = prevInBranch;
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
bool ImmutableValidator::visit(WhileStatement const& _whileStatement)
|
||||
{
|
||||
bool prevInLoop = m_inLoop;
|
||||
m_inLoop = true;
|
||||
|
||||
_whileStatement.condition().accept(*this);
|
||||
_whileStatement.body().accept(*this);
|
||||
|
||||
m_inLoop = prevInLoop;
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
bool ImmutableValidator::visit(TryStatement const& _tryStatement)
|
||||
{
|
||||
ScopedSaveAndRestore constructorGuard{m_inTryStatement, true};
|
||||
|
||||
_tryStatement.externalCall().accept(*this);
|
||||
|
||||
for (auto&& clause: _tryStatement.clauses())
|
||||
if (clause)
|
||||
clause->accept(*this);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
void ImmutableValidator::endVisit(IdentifierPath const& _identifierPath)
|
||||
{
|
||||
if (auto const callableDef = dynamic_cast<CallableDeclaration const*>(_identifierPath.annotation().referencedDeclaration))
|
||||
visitCallableIfNew(
|
||||
*_identifierPath.annotation().requiredLookup == VirtualLookup::Virtual ?
|
||||
callableDef->resolveVirtual(m_mostDerivedContract) :
|
||||
*callableDef
|
||||
);
|
||||
|
||||
solAssert(!dynamic_cast<VariableDeclaration const*>(_identifierPath.annotation().referencedDeclaration), "");
|
||||
analyseVariableReference(_memberAccess.annotation().referencedDeclaration, _memberAccess);
|
||||
}
|
||||
|
||||
void ImmutableValidator::endVisit(Identifier const& _identifier)
|
||||
{
|
||||
if (auto const callableDef = dynamic_cast<CallableDeclaration const*>(_identifier.annotation().referencedDeclaration))
|
||||
visitCallableIfNew(*_identifier.annotation().requiredLookup == VirtualLookup::Virtual ? callableDef->resolveVirtual(m_mostDerivedContract) : *callableDef);
|
||||
if (auto const varDecl = dynamic_cast<VariableDeclaration const*>(_identifier.annotation().referencedDeclaration))
|
||||
analyseVariableReference(*varDecl, _identifier);
|
||||
analyseVariableReference(_identifier.annotation().referencedDeclaration, _identifier);
|
||||
}
|
||||
|
||||
void ImmutableValidator::endVisit(Return const& _return)
|
||||
void ImmutableValidator::analyseVariableReference(Declaration const* _reference, Expression const& _expression)
|
||||
{
|
||||
if (m_currentConstructor != nullptr)
|
||||
checkAllVariablesInitialized(_return.location());
|
||||
}
|
||||
|
||||
bool ImmutableValidator::analyseCallable(CallableDeclaration const& _callableDeclaration)
|
||||
{
|
||||
ScopedSaveAndRestore constructorGuard{m_currentConstructor, {}};
|
||||
ScopedSaveAndRestore constructorContractGuard{m_currentConstructorContract, {}};
|
||||
|
||||
if (FunctionDefinition const* funcDef = dynamic_cast<decltype(funcDef)>(&_callableDeclaration))
|
||||
{
|
||||
ASTNode::listAccept(funcDef->modifiers(), *this);
|
||||
|
||||
if (funcDef->isConstructor())
|
||||
{
|
||||
m_currentConstructorContract = funcDef->annotation().contract;
|
||||
m_currentConstructor = funcDef;
|
||||
}
|
||||
|
||||
if (funcDef->isImplemented())
|
||||
funcDef->body().accept(*this);
|
||||
}
|
||||
else if (ModifierDefinition const* modDef = dynamic_cast<decltype(modDef)>(&_callableDeclaration))
|
||||
if (modDef->isImplemented())
|
||||
modDef->body().accept(*this);
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
void ImmutableValidator::analyseVariableReference(VariableDeclaration const& _variableReference, Expression const& _expression)
|
||||
{
|
||||
if (!_variableReference.isStateVariable() || !_variableReference.immutable())
|
||||
auto const* variable = dynamic_cast<VariableDeclaration const*>(_reference);
|
||||
if (!variable || !variable->isStateVariable() || !variable->immutable())
|
||||
return;
|
||||
|
||||
// If this is not an ordinary assignment, we write and read at the same time.
|
||||
bool write = _expression.annotation().willBeWrittenTo;
|
||||
bool read = !_expression.annotation().willBeWrittenTo || !_expression.annotation().lValueOfOrdinaryAssignment;
|
||||
if (write)
|
||||
{
|
||||
if (!m_currentConstructor)
|
||||
m_errorReporter.typeError(
|
||||
1581_error,
|
||||
_expression.location(),
|
||||
"Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor."
|
||||
);
|
||||
else if (m_currentConstructor->annotation().contract->id() != _variableReference.annotation().contract->id())
|
||||
m_errorReporter.typeError(
|
||||
7484_error,
|
||||
_expression.location(),
|
||||
"Cannot write to immutable here: Immutable variables must be initialized in the constructor of the contract they are defined in."
|
||||
);
|
||||
else if (m_inLoop)
|
||||
m_errorReporter.typeError(
|
||||
6672_error,
|
||||
_expression.location(),
|
||||
"Cannot write to immutable here: Immutable variables cannot be initialized inside a loop."
|
||||
);
|
||||
else if (m_inBranch)
|
||||
m_errorReporter.typeError(
|
||||
4599_error,
|
||||
_expression.location(),
|
||||
"Cannot write to immutable here: Immutable variables cannot be initialized inside an if statement."
|
||||
);
|
||||
else if (m_inTryStatement)
|
||||
m_errorReporter.typeError(
|
||||
4130_error,
|
||||
_expression.location(),
|
||||
"Cannot write to immutable here: Immutable variables cannot be initialized inside a try/catch statement."
|
||||
);
|
||||
else if (m_initializedStateVariables.count(&_variableReference))
|
||||
{
|
||||
if (!read)
|
||||
m_errorReporter.typeError(
|
||||
1574_error,
|
||||
_expression.location(),
|
||||
"Immutable state variable already initialized."
|
||||
);
|
||||
else
|
||||
m_errorReporter.typeError(
|
||||
2718_error,
|
||||
_expression.location(),
|
||||
"Immutable variables cannot be modified after initialization."
|
||||
);
|
||||
}
|
||||
else if (read)
|
||||
m_errorReporter.typeError(
|
||||
3969_error,
|
||||
_expression.location(),
|
||||
"Immutable variables must be initialized using an assignment."
|
||||
);
|
||||
m_initializedStateVariables.emplace(&_variableReference);
|
||||
}
|
||||
if (
|
||||
read &&
|
||||
m_inCreationContext &&
|
||||
!m_initializedStateVariables.count(&_variableReference)
|
||||
)
|
||||
if (_expression.annotation().willBeWrittenTo)
|
||||
m_errorReporter.typeError(
|
||||
7733_error,
|
||||
1581_error,
|
||||
_expression.location(),
|
||||
"Immutable variables cannot be read before they are initialized."
|
||||
"Cannot write to immutable here: Immutable variables can only be initialized inline or assigned directly in the constructor."
|
||||
);
|
||||
}
|
||||
|
||||
void ImmutableValidator::checkAllVariablesInitialized(solidity::langutil::SourceLocation const& _location)
|
||||
{
|
||||
for (ContractDefinition const* contract: m_mostDerivedContract.annotation().linearizedBaseContracts | ranges::views::reverse)
|
||||
{
|
||||
for (VariableDeclaration const* varDecl: contract->stateVariables())
|
||||
if (varDecl->immutable())
|
||||
if (!util::contains(m_initializedStateVariables, varDecl))
|
||||
m_errorReporter.typeError(
|
||||
2658_error,
|
||||
_location,
|
||||
solidity::langutil::SecondarySourceLocation().append("Not initialized: ", varDecl->location()),
|
||||
"Construction control flow ends without initializing all immutable state variables."
|
||||
);
|
||||
|
||||
// Don't check further than the current c'tors contract
|
||||
if (contract == m_currentConstructorContract)
|
||||
break;
|
||||
}
|
||||
}
|
||||
|
||||
void ImmutableValidator::visitCallableIfNew(Declaration const& _declaration)
|
||||
{
|
||||
CallableDeclaration const* _callable = dynamic_cast<CallableDeclaration const*>(&_declaration);
|
||||
solAssert(_callable != nullptr, "");
|
||||
|
||||
if (m_visitedCallables.emplace(_callable).second)
|
||||
_declaration.accept(*this);
|
||||
}
|
||||
|
||||
@@ -21,25 +21,15 @@
|
||||
#include <libsolidity/ast/ASTVisitor.h>
|
||||
#include <liblangutil/ErrorReporter.h>
|
||||
|
||||
#include <memory>
|
||||
|
||||
namespace solidity::frontend
|
||||
{
|
||||
|
||||
/**
|
||||
* Validates access and initialization of immutable variables:
|
||||
* must be directly initialized in their respective c'tor or inline
|
||||
* cannot be read before being initialized
|
||||
* cannot be read when initializing state variables inline
|
||||
* must be initialized outside loops (only one initialization)
|
||||
* must be initialized outside ifs (must be initialized unconditionally)
|
||||
* must be initialized exactly once (no multiple statements)
|
||||
* must be initialized exactly once (no early return to skip initialization)
|
||||
* must be directly initialized in a c'tor or inline
|
||||
*/
|
||||
class ImmutableValidator: private ASTConstVisitor
|
||||
{
|
||||
using CallableDeclarationSet = std::set<CallableDeclaration const*, ASTNode::CompareByID>;
|
||||
|
||||
public:
|
||||
ImmutableValidator(langutil::ErrorReporter& _errorReporter, ContractDefinition const& _contractDefinition):
|
||||
m_mostDerivedContract(_contractDefinition),
|
||||
@@ -49,37 +39,15 @@ public:
|
||||
void analyze();
|
||||
|
||||
private:
|
||||
bool visit(Assignment const& _assignment);
|
||||
bool visit(FunctionDefinition const& _functionDefinition);
|
||||
bool visit(ModifierDefinition const& _modifierDefinition);
|
||||
bool visit(MemberAccess const& _memberAccess);
|
||||
bool visit(IfStatement const& _ifStatement);
|
||||
bool visit(WhileStatement const& _whileStatement);
|
||||
bool visit(TryStatement const& _tryStatement);
|
||||
void endVisit(IdentifierPath const& _identifierPath);
|
||||
void endVisit(MemberAccess const& _memberAccess);
|
||||
void endVisit(Identifier const& _identifier);
|
||||
void endVisit(Return const& _return);
|
||||
|
||||
bool analyseCallable(CallableDeclaration const& _callableDeclaration);
|
||||
void analyseVariableReference(VariableDeclaration const& _variableReference, Expression const& _expression);
|
||||
|
||||
void checkAllVariablesInitialized(langutil::SourceLocation const& _location);
|
||||
|
||||
void visitCallableIfNew(Declaration const& _declaration);
|
||||
void analyseVariableReference(Declaration const* _variableReference, Expression const& _expression);
|
||||
|
||||
ContractDefinition const& m_mostDerivedContract;
|
||||
|
||||
CallableDeclarationSet m_visitedCallables;
|
||||
|
||||
std::set<VariableDeclaration const*, ASTNode::CompareByID> m_initializedStateVariables;
|
||||
langutil::ErrorReporter& m_errorReporter;
|
||||
|
||||
FunctionDefinition const* m_currentConstructor = nullptr;
|
||||
ContractDefinition const* m_currentConstructorContract = nullptr;
|
||||
bool m_inLoop = false;
|
||||
bool m_inBranch = false;
|
||||
bool m_inCreationContext = true;
|
||||
bool m_inTryStatement = false;
|
||||
};
|
||||
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user