From 9f8d49e3589019b3b10048458393c67277bb3881 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Tue, 17 Dec 2019 15:18:58 +0000 Subject: [PATCH 1/3] Move modifier context check to PostTypeChecker refs #7566 --- libsolidity/analysis/PostTypeChecker.cpp | 48 +++++++++++++++++++++++- libsolidity/analysis/PostTypeChecker.h | 4 ++ libsolidity/analysis/TypeChecker.cpp | 15 +------- libsolidity/analysis/TypeChecker.h | 3 -- 4 files changed, 52 insertions(+), 18 deletions(-) diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp index a83c51d3b..58eb9d79a 100644 --- a/libsolidity/analysis/PostTypeChecker.cpp +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -67,6 +67,15 @@ bool PostTypeChecker::visit(Identifier const& _identifier) return callVisit(_identifier); } +bool PostTypeChecker::visit(ModifierInvocation const& _modifierInvocation) +{ + return callVisit(_modifierInvocation); +} + +void PostTypeChecker::endVisit(ModifierInvocation const& _modifierInvocation) +{ + callEndVisit(_modifierInvocation); +} namespace { @@ -183,11 +192,48 @@ struct OverrideSpecifierChecker: public PostTypeChecker::Checker } } }; -} +struct ModifierContextChecker: public PostTypeChecker::Checker +{ + ModifierContextChecker(ErrorReporter& _errorReporter): + Checker(_errorReporter) {} + + bool visit(ModifierInvocation const&) override + { + m_insideModifierInvocation = true; + + return true; + } + + void endVisit(ModifierInvocation const&) override + { + m_insideModifierInvocation = false; + } + + bool visit(Identifier const& _identifier) override + { + if (m_insideModifierInvocation) + return true; + + if (ModifierType const* type = dynamic_cast(_identifier.annotation().type)) + { + m_errorReporter.typeError( + _identifier.location(), + "Modifier can only be referenced in function headers." + ); + } + + return false; + } +private: + /// Flag indicating whether we are currently inside the invocation of a modifier + bool m_insideModifierInvocation = false; +}; +} 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)); } diff --git a/libsolidity/analysis/PostTypeChecker.h b/libsolidity/analysis/PostTypeChecker.h index a772643f8..325d01ebb 100644 --- a/libsolidity/analysis/PostTypeChecker.h +++ b/libsolidity/analysis/PostTypeChecker.h @@ -36,6 +36,7 @@ namespace solidity::frontend * This module performs analyses on the AST that are done after type checking and assignments of types: * - whether there are circular references in constant state variables * - whether override specifiers are actually contracts + * - whether a modifier is in a function header * * When adding a new checker, make sure a visitor that forwards calls that your * checker uses exists in PostTypeChecker. Add missing ones. @@ -69,6 +70,9 @@ private: bool visit(Identifier const& _identifier) override; + bool visit(ModifierInvocation const& _modifierInvocation) override; + void endVisit(ModifierInvocation const& _modifierInvocation) override; + template bool callVisit(T const& _node) { diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 8f167e875..ea0c976e0 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -547,11 +547,7 @@ void TypeChecker::visitManually( for (ASTPointer const& argument: arguments) argument->accept(*this); - { - m_insideModifierInvocation = true; - ScopeGuard resetFlag{[&] () { m_insideModifierInvocation = false; }}; - _modifier.name()->accept(*this); - } + _modifier.name()->accept(*this); auto const* declaration = &dereference(*_modifier.name()); vector> emptyParameterList; @@ -2704,15 +2700,6 @@ bool TypeChecker::visit(Identifier const& _identifier) ); } - if (!m_insideModifierInvocation) - if (ModifierType const* type = dynamic_cast(_identifier.annotation().type)) - { - m_errorReporter.typeError( - _identifier.location(), - "Modifier can only be referenced in function headers." - ); - } - return false; } diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index ae83559d5..455b60fb8 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -170,9 +170,6 @@ private: /// Flag indicating whether we are currently inside a StructDefinition. bool m_insideStruct = false; - /// Flag indicating whether we are currently inside the invocation of a modifier - bool m_insideModifierInvocation = false; - langutil::ErrorReporter& m_errorReporter; }; From 21844aa545164b52570889a15f6c131937775641 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Tue, 17 Dec 2019 16:04:18 +0000 Subject: [PATCH 2/3] Move event-outside-emit check to PostTypeChecker refs #7566 --- libsolidity/analysis/PostTypeChecker.cpp | 54 ++++++++++++++++++++++++ libsolidity/analysis/PostTypeChecker.h | 6 +++ libsolidity/analysis/TypeChecker.cpp | 8 ---- libsolidity/analysis/TypeChecker.h | 4 -- 4 files changed, 60 insertions(+), 12 deletions(-) diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp index 58eb9d79a..a18ee4320 100644 --- a/libsolidity/analysis/PostTypeChecker.cpp +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -62,6 +62,21 @@ void PostTypeChecker::endVisit(VariableDeclaration const& _variable) callEndVisit(_variable); } +bool PostTypeChecker::visit(EmitStatement const& _emit) +{ + return callVisit(_emit); +} + +void PostTypeChecker::endVisit(EmitStatement const& _emit) +{ + callEndVisit(_emit); +} + +bool PostTypeChecker::visit(FunctionCall const& _functionCall) +{ + return callVisit(_functionCall); +} + bool PostTypeChecker::visit(Identifier const& _identifier) { return callVisit(_identifier); @@ -229,6 +244,44 @@ private: /// Flag indicating whether we are currently inside the invocation of a modifier bool m_insideModifierInvocation = false; }; + +struct EventOutsideEmitChecker: public PostTypeChecker::Checker +{ + EventOutsideEmitChecker(ErrorReporter& _errorReporter): + Checker(_errorReporter) {} + + bool visit(EmitStatement const&) override + { + m_insideEmitStatement = true; + return true; + } + + void endVisit(EmitStatement const&) override + { + m_insideEmitStatement = true; + } + + 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( + _functionCall.location(), + "Event invocations have to be prefixed by \"emit\"." + ); + + return true; + } + +private: + /// Flag indicating whether we are currently inside an EmitStatement. + bool m_insideEmitStatement = false; +}; + } PostTypeChecker::PostTypeChecker(langutil::ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) @@ -236,4 +289,5 @@ PostTypeChecker::PostTypeChecker(langutil::ErrorReporter& _errorReporter): m_err 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 325d01ebb..7a48dfb22 100644 --- a/libsolidity/analysis/PostTypeChecker.h +++ b/libsolidity/analysis/PostTypeChecker.h @@ -37,6 +37,7 @@ namespace solidity::frontend * - whether there are circular references in constant state variables * - whether override specifiers are actually contracts * - whether a modifier is in a function header + * - whether an event is used outside of an emit statement * * When adding a new checker, make sure a visitor that forwards calls that your * checker uses exists in PostTypeChecker. Add missing ones. @@ -68,6 +69,11 @@ private: bool visit(VariableDeclaration const& _variable) override; void endVisit(VariableDeclaration const& _variable) override; + bool visit(EmitStatement const& _emit) override; + void endVisit(EmitStatement const& _emit) override; + + bool visit(FunctionCall const& _functionCall) override; + bool visit(Identifier const& _identifier) override; bool visit(ModifierInvocation const& _modifierInvocation) override; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index ea0c976e0..6315ac15f 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -981,7 +981,6 @@ void TypeChecker::endVisit(EmitStatement const& _emit) dynamic_cast(*type(_emit.eventCall().expression())).kind() != FunctionType::Kind::Event ) m_errorReporter.typeError(_emit.eventCall().expression().location(), "Expression has to be an event invocation."); - m_insideEmitStatement = false; } namespace @@ -1715,13 +1714,6 @@ void TypeChecker::typeCheckFunctionCall( "\"staticcall\" is not supported by the VM version." ); - // Check for event outside of emit statement - if (!m_insideEmitStatement && _functionType->kind() == FunctionType::Kind::Event) - m_errorReporter.typeError( - _functionCall.location(), - "Event invocations have to be prefixed by \"emit\"." - ); - // Perform standard function call type checking typeCheckFunctionGeneralChecks(_functionCall, _functionType); } diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 455b60fb8..1e30ac1b5 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -126,7 +126,6 @@ private: bool visit(WhileStatement const& _whileStatement) override; bool visit(ForStatement const& _forStatement) override; void endVisit(Return const& _return) override; - bool visit(EmitStatement const&) override { m_insideEmitStatement = true; return true; } void endVisit(EmitStatement const& _emit) override; bool visit(VariableDeclarationStatement const& _variable) override; void endVisit(ExpressionStatement const& _statement) override; @@ -164,9 +163,6 @@ private: langutil::EVMVersion m_evmVersion; - /// Flag indicating whether we are currently inside an EmitStatement. - bool m_insideEmitStatement = false; - /// Flag indicating whether we are currently inside a StructDefinition. bool m_insideStruct = false; From 21795627854a1160a404dcf827a3019023117f60 Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Tue, 17 Dec 2019 16:58:53 +0000 Subject: [PATCH 3/3] Move variables-in-interfaces checker to PostTypeChecker refs #7566 --- libsolidity/analysis/PostTypeChecker.cpp | 60 ++++++++++++++++++++++++ libsolidity/analysis/PostTypeChecker.h | 4 ++ libsolidity/analysis/TypeChecker.cpp | 13 ----- libsolidity/analysis/TypeChecker.h | 3 -- 4 files changed, 64 insertions(+), 16 deletions(-) diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp index a18ee4320..6ed306269 100644 --- a/libsolidity/analysis/PostTypeChecker.cpp +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -82,6 +82,16 @@ bool PostTypeChecker::visit(Identifier const& _identifier) return callVisit(_identifier); } +bool PostTypeChecker::visit(StructDefinition const& _struct) +{ + return callVisit(_struct); +} + +void PostTypeChecker::endVisit(StructDefinition const& _struct) +{ + callEndVisit(_struct); +} + bool PostTypeChecker::visit(ModifierInvocation const& _modifierInvocation) { return callVisit(_modifierInvocation); @@ -282,6 +292,55 @@ private: bool m_insideEmitStatement = false; }; +struct NoVariablesInInterfaceChecker: public PostTypeChecker::Checker +{ + NoVariablesInInterfaceChecker(ErrorReporter& _errorReporter): + Checker(_errorReporter) + {} + + bool visit(VariableDeclaration const& _variable) override + { + // Forbid any variable declarations inside interfaces unless they are part of + // * a function's input/output parameters, + // * or inside of a struct definition. + if ( + m_scope && m_scope->isInterface() + && !_variable.isCallableOrCatchParameter() + && !m_insideStruct + ) + m_errorReporter.typeError(_variable.location(), "Variables cannot be declared in interfaces."); + + return true; + } + + bool visit(ContractDefinition const& _contract) override + { + m_scope = &_contract; + return true; + } + + void endVisit(ContractDefinition const&) override + { + m_scope = nullptr; + } + + bool visit(StructDefinition const&) override + { + solAssert(m_insideStruct >= 0, ""); + m_insideStruct++; + return true; + } + + void endVisit(StructDefinition const&) override + { + m_insideStruct--; + solAssert(m_insideStruct >= 0, ""); + } +private: + ContractDefinition const* m_scope = nullptr; + /// Flag indicating whether we are currently inside a StructDefinition. + int m_insideStruct = 0; +}; } PostTypeChecker::PostTypeChecker(langutil::ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) @@ -290,4 +349,5 @@ PostTypeChecker::PostTypeChecker(langutil::ErrorReporter& _errorReporter): m_err 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 7a48dfb22..0a3358553 100644 --- a/libsolidity/analysis/PostTypeChecker.h +++ b/libsolidity/analysis/PostTypeChecker.h @@ -38,6 +38,7 @@ namespace solidity::frontend * - whether override specifiers are actually contracts * - whether a modifier is in a function header * - whether an event is used outside of an emit statement + * - whether a variable is declared in a interface * * When adding a new checker, make sure a visitor that forwards calls that your * checker uses exists in PostTypeChecker. Add missing ones. @@ -76,6 +77,9 @@ private: bool visit(Identifier const& _identifier) override; + bool visit(StructDefinition const& _struct) override; + void endVisit(StructDefinition const& _struct) override; + bool visit(ModifierInvocation const& _modifierInvocation) override; void endVisit(ModifierInvocation const& _modifierInvocation) override; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 6315ac15f..eb1b6ddcc 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -317,10 +317,7 @@ bool TypeChecker::visit(StructDefinition const& _struct) if (CycleDetector(visitor).run(_struct) != nullptr) m_errorReporter.fatalTypeError(_struct.location(), "Recursive struct definition."); - bool insideStruct = true; - swap(insideStruct, m_insideStruct); ASTNode::listAccept(_struct.members(), *this); - m_insideStruct = insideStruct; return false; } @@ -451,16 +448,6 @@ bool TypeChecker::visit(FunctionDefinition const& _function) bool TypeChecker::visit(VariableDeclaration const& _variable) { - // Forbid any variable declarations inside interfaces unless they are part of - // * a function's input/output parameters, - // * or inside of a struct definition. - if ( - m_scope->isInterface() - && !_variable.isCallableOrCatchParameter() - && !m_insideStruct - ) - m_errorReporter.typeError(_variable.location(), "Variables cannot be declared in interfaces."); - if (_variable.typeName()) _variable.typeName()->accept(*this); diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 1e30ac1b5..c7443b29c 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -163,9 +163,6 @@ private: langutil::EVMVersion m_evmVersion; - /// Flag indicating whether we are currently inside a StructDefinition. - bool m_insideStruct = false; - langutil::ErrorReporter& m_errorReporter; };