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

Add modifier areas #3939

Closed
wants to merge 13 commits into from
9 changes: 7 additions & 2 deletions docs/grammar.txt
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ ContractDefinition = ( 'contract' | 'library' | 'interface' ) Identifier
'{' ContractPart* '}'

ContractPart = StateVariableDeclaration | UsingForDeclaration
| StructDefinition | ModifierDefinition | FunctionDefinition | EventDefinition | EnumDefinition
| StructDefinition | ModifierDefinition | FunctionDefinition | EventDefinition | EnumDefinition | ModifierArea
Copy link
Member

Choose a reason for hiding this comment

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

Maybe a line break may be good.


InheritanceSpecifier = UserDefinedTypeName ( '(' Expression ( ',' Expression )* ')' )?

Expand All @@ -24,14 +24,18 @@ StructDefinition = 'struct' Identifier '{'
ModifierDefinition = 'modifier' Identifier ParameterList? Block
ModifierInvocation = Identifier ( '(' ExpressionList? ')' )?

FunctionModifier = ModifierInvocation | StateMutability | Visibility
FunctionDefinition = 'function' Identifier? ParameterList
( ModifierInvocation | StateMutability | 'external' | 'public' | 'internal' | 'private' )*
FunctionModifier*
( 'returns' ParameterList )? ( ';' | Block )
EventDefinition = 'event' Identifier EventParameterList 'anonymous'? ';'

EnumValue = Identifier
EnumDefinition = 'enum' Identifier '{' EnumValue? (',' EnumValue)* '}'

ModifierArea = 'using modifier' FunctionModifier FunctionModifier*
'{' (ModifierArea | FunctionDefinition)* '}'

ParameterList = '(' ( Parameter (',' Parameter)* )? ')'
Parameter = TypeName StorageLocation? Identifier?

Expand Down Expand Up @@ -59,6 +63,7 @@ FunctionTypeName = 'function' FunctionTypeParameterList ( 'internal' | 'external
( 'returns' FunctionTypeParameterList )?
StorageLocation = 'memory' | 'storage' | 'calldata'
StateMutability = 'pure' | 'view' | 'payable'
Visibility = 'public' | 'external' | 'internal' | 'private'

Block = '{' Statement* '}'
Statement = IfStatement | WhileStatement | ForStatement | Block | InlineAssemblyStatement |
Expand Down
97 changes: 97 additions & 0 deletions docs/structure-of-a-contract.rst
Original file line number Diff line number Diff line change
Expand Up @@ -139,3 +139,100 @@ Enums can be used to create custom types with a finite set of 'constant values'
contract Purchase {
enum State { Created, Locked, Inactive } // Enum
}


.. _structure-modifier-areas:

Modifier Areas
==============

Modifier areas can be used to apply modifier invocations,
a visibility specifier, or a mutability specifier to an
entire group of functions. They take the following syntax:

::

pragma soliditiy [TBD];

contract Purchase {
private State currentState = Created;

enum State { Created, Locked, Inactive } // Enum

modifier inState(State requiredState) { require(currentState = requiredState); }

using modifier inState(State.Created) {
function lock() { currentState = State.Locked; }

// Other functions only callable in state Created
}

using modifier inState(State.Locked) {
function inactivate() { currentState = State.Inactive; }

// Other functions only callable in state Locked
}

using modifier inState(State.Inactive) {
// Functions only callable in state Inactive
}
}

Multiple modifier invocations can be used in a single modifier area:
`using modifier A, B { /* ... */ }` declares a modifier area
where both modifiers `A` and `B` are used on every function.

Modifier areas can be nested by declaring a modifier area
inside the scope of another. Inside the nested modifier area,
all modifiers from all parents apply in addition to those
declared on that modifier area.

::

contract C {
modifier A { /* ... */ }
modifier B { /* ... */ }

using modifier A {
// Functions where only A applies

using modifier B {
// Functions where A and B apply
}
}
}

Modifier areas can also apply a mutability or visiblity specifier to
a group of functions.

::

contract C {
using modifier public {
// Functions that are public
}

using modifier payable {
// Functions that are payable
}
}

State and mutability specifiers also nest, just like modifiers.

::

contract C {
using modifier public {
// Functions that are public

using modifier payable {
// Functions that are public and payable
}
}
}

It is **not** permissible to do any of the following:
1. Declare a nested modifier area with a different visibility or
mutability than any of its parents
2. Declare a function within a modifier area with a different
visibility or mutability than any of its parents.
27 changes: 27 additions & 0 deletions libsolidity/analysis/SyntaxChecker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,22 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma)
return true;
}

bool SyntaxChecker::visit(ModifierArea const& _modifierArea)
{
if (_modifierArea.parent())
{
auto parent = _modifierArea.parent();

if (parent->visibility() != Declaration::Visibility::Default && _modifierArea.visibility() != parent->visibility())
m_errorReporter.syntaxError(_modifierArea.location(), "Cannot override parent modifier area's visibility.");

if (parent->stateMutability() != StateMutability::NonPayable && _modifierArea.stateMutability() != parent->stateMutability())
m_errorReporter.syntaxError(_modifierArea.location(), "Cannot override parent modifier area's state mutability.");
}

return true;
}

bool SyntaxChecker::visit(ModifierDefinition const&)
{
m_placeholderFound = false;
Expand Down Expand Up @@ -222,6 +238,17 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function)
);
}

if (_function.modifierArea())
{
auto modifierArea = _function.modifierArea();

if (modifierArea->visibility() != Declaration::Visibility::Default && _function.visibility() != modifierArea->visibility())
m_errorReporter.syntaxError(_function.location(), "Cannot override modifier area's visibility.");

if (modifierArea->stateMutability() != StateMutability::NonPayable && _function.stateMutability() != modifierArea->stateMutability())
m_errorReporter.syntaxError(_function.location(), "Cannot override modifier area's state mutability.");
}

if (!_function.isImplemented() && !_function.modifiers().empty())
m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers.");

Expand Down
2 changes: 2 additions & 0 deletions libsolidity/analysis/SyntaxChecker.h
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ class SyntaxChecker: private ASTConstVisitor
virtual void endVisit(SourceUnit const& _sourceUnit) override;
virtual bool visit(PragmaDirective const& _pragma) override;

virtual bool visit(ModifierArea const& _modifierArea) override;

virtual bool visit(ModifierDefinition const& _modifier) override;
virtual void endVisit(ModifierDefinition const& _modifier) override;

Expand Down
68 changes: 68 additions & 0 deletions libsolidity/ast/AST.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@

#include <algorithm>
#include <functional>
#include <memory>

using namespace std;
using namespace dev;
Expand Down Expand Up @@ -252,6 +253,28 @@ ContractDefinitionAnnotation& ContractDefinition::annotation() const
return dynamic_cast<ContractDefinitionAnnotation&>(*m_annotation);
}

std::vector<FunctionDefinition const*> ContractDefinition::definedFunctions() const
{
std::vector<FunctionDefinition const*> totalFunctions = filteredNodes<FunctionDefinition>(m_subNodes);
std::vector<ModifierArea const*> currentGroup = modifierAreas();
std::vector<ModifierArea const*> nextGroup;

while (!currentGroup.empty())
{
for (auto const& modifierArea : currentGroup)
{
for (auto const& subFunction: modifierArea->definedFunctions())
totalFunctions.push_back(subFunction.get());
for (auto const& subArea: modifierArea->subAreas())
nextGroup.push_back(subArea.get());
}
currentGroup = nextGroup;
nextGroup.clear();
}

return totalFunctions;
}

TypeNameAnnotation& TypeName::annotation() const
{
if (!m_annotation)
Expand Down Expand Up @@ -354,6 +377,30 @@ FunctionDefinitionAnnotation& FunctionDefinition::annotation() const
return dynamic_cast<FunctionDefinitionAnnotation&>(*m_annotation);
}

FunctionDefinition::FunctionDefinition(
SourceLocation const &_location, ASTPointer<ASTString> const &_name,
Declaration::Visibility _visibility, StateMutability _stateMutability,
bool _isConstructor, ASTPointer<ASTString> const &_documentation,
ASTPointer<ParameterList> const &_parameters,
std::vector<ASTPointer<ModifierInvocation>> const &_modifiers,
ASTPointer<ParameterList> const &_returnParameters,
ASTPointer<Block> const &_body,
ModifierArea* const &_modifierArea
):
CallableDeclaration(_location, _name, _visibility, _parameters, _returnParameters),
Documented(_documentation),
ImplementationOptional(_body != nullptr),
m_stateMutability(_stateMutability),
m_isConstructor(_isConstructor),
m_body(_body),
m_modifierArea(_modifierArea)
{
if (_modifierArea)
m_functionModifiers = _modifierArea->totalModifiers() + _modifiers;
else
m_functionModifiers = _modifiers;
}

TypePointer ModifierDefinition::type() const
{
return make_shared<ModifierType>(*this);
Expand Down Expand Up @@ -500,6 +547,27 @@ VariableDeclarationAnnotation& VariableDeclaration::annotation() const
return dynamic_cast<VariableDeclarationAnnotation&>(*m_annotation);
}

ModifierArea::ModifierArea(SourceLocation const& _location,
std::vector<ASTPointer<ModifierInvocation>> const& _modifiers,
Declaration::Visibility const& _visibility,
StateMutability const& _mutability,
ASTPointer<std::vector<ASTPointer<FunctionDefinition>>> const& _functions,
ASTPointer<std::vector<ASTPointer<ModifierArea>>> const& _subAreas,
ModifierArea* _parent) :
ASTNode(_location),
m_declaredModifiers(_modifiers),
m_visibility(_visibility),
m_mutability(_mutability),
m_functions(_functions),
m_subAreas(_subAreas),
m_parent(_parent)
{
if (_parent)
m_totalModifiers = _parent->totalModifiers() + _modifiers;
else
m_totalModifiers = _modifiers;
}

StatementAnnotation& Statement::annotation() const
{
if (!m_annotation)
Expand Down
55 changes: 44 additions & 11 deletions libsolidity/ast/AST.h
Original file line number Diff line number Diff line change
Expand Up @@ -385,7 +385,8 @@ class ContractDefinition: public Declaration, public Documented
std::vector<EnumDefinition const*> definedEnums() const { return filteredNodes<EnumDefinition>(m_subNodes); }
std::vector<VariableDeclaration const*> stateVariables() const { return filteredNodes<VariableDeclaration>(m_subNodes); }
std::vector<ModifierDefinition const*> functionModifiers() const { return filteredNodes<ModifierDefinition>(m_subNodes); }
std::vector<FunctionDefinition const*> definedFunctions() const { return filteredNodes<FunctionDefinition>(m_subNodes); }
std::vector<FunctionDefinition const*> definedFunctions() const;
std::vector<ModifierArea const*> modifierAreas() const { return filteredNodes<ModifierArea>(m_subNodes); }
std::vector<EventDefinition const*> events() const { return filteredNodes<EventDefinition>(m_subNodes); }
std::vector<EventDefinition const*> const& interfaceEvents() const;
bool isLibrary() const { return m_contractKind == ContractKind::Library; }
Expand Down Expand Up @@ -599,16 +600,9 @@ class FunctionDefinition: public CallableDeclaration, public Documented, public
ASTPointer<ParameterList> const& _parameters,
std::vector<ASTPointer<ModifierInvocation>> const& _modifiers,
ASTPointer<ParameterList> const& _returnParameters,
ASTPointer<Block> const& _body
):
CallableDeclaration(_location, _name, _visibility, _parameters, _returnParameters),
Documented(_documentation),
ImplementationOptional(_body != nullptr),
m_stateMutability(_stateMutability),
m_isConstructor(_isConstructor),
m_functionModifiers(_modifiers),
m_body(_body)
{}
ASTPointer<Block> const& _body,
ModifierArea* const& _modifierArea = nullptr
);

virtual void accept(ASTVisitor& _visitor) override;
virtual void accept(ASTConstVisitor& _visitor) const override;
Expand All @@ -620,6 +614,8 @@ class FunctionDefinition: public CallableDeclaration, public Documented, public
std::vector<ASTPointer<ModifierInvocation>> const& modifiers() const { return m_functionModifiers; }
std::vector<ASTPointer<VariableDeclaration>> const& returnParameters() const { return m_returnParameters->parameters(); }
Block const& body() const { solAssert(m_body, ""); return *m_body; }
ModifierArea* modifierArea() const { return m_modifierArea; }

virtual bool isVisibleInContract() const override
{
return Declaration::isVisibleInContract() && !isConstructor() && !isFallback();
Expand All @@ -646,6 +642,7 @@ class FunctionDefinition: public CallableDeclaration, public Documented, public
bool m_isConstructor;
std::vector<ASTPointer<ModifierInvocation>> m_functionModifiers;
ASTPointer<Block> m_body;
ModifierArea* m_modifierArea;
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps use ASTPointer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was something @ekpyron mentioned. We can''t use a smart pointer here because then there is a dependency loop and neither ModifierArea nor FunctionDefinition would be destructed.

};

/**
Expand Down Expand Up @@ -723,6 +720,42 @@ class VariableDeclaration: public Declaration
Location m_location; ///< Location of the variable if it is of reference type.
};

class ModifierArea: public ASTNode
{
public:
Copy link
Member

Choose a reason for hiding this comment

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

I think indentation here is off.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I just copied it from surrounding classes

ModifierArea(
SourceLocation const& _location,
std::vector<ASTPointer<ModifierInvocation>> const& _modifiers,
Declaration::Visibility const& _visibility,
StateMutability const& _mutability,
ASTPointer<std::vector<ASTPointer<FunctionDefinition>>> const& _functions,
ASTPointer<std::vector<ASTPointer<ModifierArea>>> const& _subAreas,
ModifierArea* _parent = nullptr
);

virtual void accept(ASTVisitor& _visitor) override;
virtual void accept(ASTConstVisitor& _visitor) const override;

/// Returns only modifiers defined
std::vector<ASTPointer<ModifierInvocation>> const& declaredModifiers() const { return m_declaredModifiers; }
/// Returns all modifiers in effect (including ones from parents)
std::vector<ASTPointer<ModifierInvocation>> const& totalModifiers() const { return m_totalModifiers; }
Declaration::Visibility const& visibility() const { return m_visibility; }
StateMutability const& stateMutability() const { return m_mutability; }
std::vector<ASTPointer<FunctionDefinition>> const& definedFunctions() const { return *m_functions; }
std::vector<ASTPointer<ModifierArea>> const& subAreas() const { return *m_subAreas; }
ModifierArea const* parent() const { return m_parent; }

private:
std::vector<ASTPointer<ModifierInvocation>> m_declaredModifiers;
std::vector<ASTPointer<ModifierInvocation>> m_totalModifiers;
Declaration::Visibility m_visibility;
StateMutability m_mutability;
ASTPointer<std::vector<ASTPointer<FunctionDefinition>> const> m_functions;
ASTPointer<std::vector<ASTPointer<ModifierArea>> const> m_subAreas;
ModifierArea* m_parent;
};

/**
* Definition of a function modifier.
*/
Expand Down
1 change: 1 addition & 0 deletions libsolidity/ast/ASTForward.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ class EnumValue;
class ParameterList;
class FunctionDefinition;
class VariableDeclaration;
class ModifierArea;
class ModifierDefinition;
class ModifierInvocation;
class EventDefinition;
Expand Down
10 changes: 10 additions & 0 deletions libsolidity/ast/ASTJsonConverter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,16 @@ bool ASTJsonConverter::visit(VariableDeclaration const& _node)
return false;
}

bool ASTJsonConverter::visit(ModifierArea const& _node)
{
setJsonNode(_node, "ModifierArea", {
make_pair("modifiers", toJson(_node.declaredModifiers())),
make_pair("functions", toJson(_node.definedFunctions())),
make_pair("subAreas", toJson(_node.subAreas()))
});
return false;
}

bool ASTJsonConverter::visit(ModifierDefinition const& _node)
{
setJsonNode(_node, "ModifierDefinition", {
Expand Down
Loading