Merge pull request #8040 from ethereum/stateless-7566

Move states from the TypeChecker to the PostTypeChecker

libsolidity/analysis/PostTypeChecker.cpp

@@ -62,11 +62,45 @@ 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);
 }
 
+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);
+}
+
+void PostTypeChecker::endVisit(ModifierInvocation const& _modifierInvocation)
+{
+	callEndVisit(_modifierInvocation);
+}
 
 namespace
 {
@@ -183,11 +217,137 @@ 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<decltype(type)>(_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;
+};
+
+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<FunctionTypePointer const>(_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;
+};
+
+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)
 {
 	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<NoVariablesInInterfaceChecker>(_errorReporter));
 }

libsolidity/analysis/PostTypeChecker.h

@@ -36,6 +36,9 @@ 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
+ *  - 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.
@@ -67,8 +70,19 @@ 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(StructDefinition const& _struct) override;
+	void endVisit(StructDefinition const& _struct) override;
+
+	bool visit(ModifierInvocation const& _modifierInvocation) override;
+	void endVisit(ModifierInvocation const& _modifierInvocation) override;
+
 	template <class T>
 	bool callVisit(T const& _node)
 	{

libsolidity/analysis/TypeChecker.cpp

@@ -317,10 +317,7 @@ bool TypeChecker::visit(StructDefinition const& _struct)
 	if (CycleDetector<StructDefinition>(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);
 
@@ -547,11 +534,7 @@ void TypeChecker::visitManually(
 	for (ASTPointer<Expression> const& argument: arguments)
 		argument->accept(*this);
 
-	{
+	_modifier.name()->accept(*this);
-		m_insideModifierInvocation = true;
-		ScopeGuard resetFlag{[&] () { m_insideModifierInvocation = false; }};
-		_modifier.name()->accept(*this);
-	}
 
 	auto const* declaration = &dereference(*_modifier.name());
 	vector<ASTPointer<VariableDeclaration>> emptyParameterList;
@@ -985,7 +968,6 @@ void TypeChecker::endVisit(EmitStatement const& _emit)
 		dynamic_cast<FunctionType const&>(*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
@@ -1719,13 +1701,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);
 }
@@ -2704,15 +2679,6 @@ bool TypeChecker::visit(Identifier const& _identifier)
 			);
 	}
 
-	if (!m_insideModifierInvocation)
-		if (ModifierType const* type = dynamic_cast<decltype(type)>(_identifier.annotation().type))
-		{
-			m_errorReporter.typeError(
-				_identifier.location(),
-				"Modifier can only be referenced in function headers."
-			);
-		}
-
 	return false;
 }
 

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,15 +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;
-
-	/// Flag indicating whether we are currently inside the invocation of a modifier
-	bool m_insideModifierInvocation = false;
-
 	langutil::ErrorReporter& m_errorReporter;
 };