Merge pull request #7647 from ethereum/virtual-5424

Implement virtual keyword

libsolidity/analysis/ContractLevelChecker.cpp

@@ -245,15 +245,14 @@ void ContractLevelChecker::checkIllegalOverrides(ContractDefinition const& _cont
 
 	for (FunctionDefinition const* function: _contract.definedFunctions())
 	{
+		if (function->isConstructor())
+			continue;
+
 		if (contains_if(modSet, MatchByName{function->name()}))
 			m_errorReporter.typeError(function->location(), "Override changes modifier to function.");
 
-		// Skip if not overridable
-		if (!function->isOverridable())
-			continue;
-
 		// No inheriting functions found
-		if (funcSet.find(function) == funcSet.cend() && function->overrides())
+		if (!funcSet.count(function) && function->overrides())
 			m_errorReporter.typeError(
 				function->overrides()->location(),
 				"Function has override specified but does not override anything."
@@ -279,6 +278,12 @@ bool ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _func
 		success = false;
 	}
 
+	if (!_super.virtualSemantics())
+	{
+		overrideError( _super, _function, "Trying to override non-virtual function. Did you forget to add \"virtual\"?", "Overriding function is here:");
+		success = false;
+	}
+
 	if (!functionType->hasEqualReturnTypes(*superType))
 	{
 		overrideError(_function, _super, "Overriding function return types differ.");
@@ -343,11 +348,11 @@ void ContractLevelChecker::overrideListError(FunctionDefinition const& function,
 	);
 }
 
-void ContractLevelChecker::overrideError(CallableDeclaration const& function, CallableDeclaration const& super, string message)
+void ContractLevelChecker::overrideError(CallableDeclaration const& function, CallableDeclaration const& super, string message, string secondaryMsg)
 {
 	m_errorReporter.typeError(
 		function.location(),
-		SecondarySourceLocation().append("Overridden function is here:", super.location()),
+		SecondarySourceLocation().append(secondaryMsg, super.location()),
 		message
 	);
 }
@@ -653,10 +658,10 @@ void ContractLevelChecker::checkAmbiguousOverrides(ContractDefinition const& _co
 			continue;
 
 		// Not an overridable function
-		if (!(*it)->isOverridable())
+		if ((*it)->isConstructor())
 		{
 			for (begin++; begin != end; begin++)
-				solAssert(!(*begin)->isOverridable(), "All functions in range expected to be non-overridable!");
+				solAssert((*begin)->isConstructor(), "All functions in range expected to be constructors!");
 			continue;
 		}
 
@@ -785,8 +790,7 @@ void ContractLevelChecker::checkOverrideList(FunctionMultiSet const& _funcSet, F
 	for (auto [begin, end] = _funcSet.equal_range(&_function); begin != end; begin++)
 	{
 		// Validate the override
-		if (!checkFunctionOverride(_function, **begin))
-			break;
+		checkFunctionOverride(_function, **begin);
 
 		expectedContracts.insert((*begin)->annotation().contract);
 	}

libsolidity/analysis/ContractLevelChecker.h

@@ -75,7 +75,7 @@ private:
 	/// Also stores the direct super function in the AST annotations.
 	bool checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super);
 	void overrideListError(FunctionDefinition const& function, std::set<ContractDefinition const*, LessFunction> _secondary, std::string const& _message1, std::string const& _message2);
-	void overrideError(CallableDeclaration const& function, CallableDeclaration const& super, std::string message);
+	void overrideError(CallableDeclaration const& function, CallableDeclaration const& super, std::string message, std::string secondaryMsg = "Overridden function is here:");
 	void checkAbstractFunctions(ContractDefinition const& _contract);
 	/// Checks that the base constructor arguments are properly provided.
 	/// Fills the list of unimplemented functions in _contract's annotations.

libsolidity/analysis/TypeChecker.cpp

@@ -327,6 +327,9 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
 {
 	bool isLibraryFunction = _function.inContractKind() == ContractDefinition::ContractKind::Library;
 
+	if (_function.markedVirtual() && _function.annotation().contract->isInterface())
+		m_errorReporter.warning(_function.location(), "Interface functions are implicitly \"virtual\"");
+
 	if (_function.isPayable())
 	{
 		if (isLibraryFunction)

libsolidity/ast/AST.h

@@ -604,13 +604,15 @@ public:
 		ASTPointer<ASTString> const& _name,
 		Declaration::Visibility _visibility,
 		ASTPointer<ParameterList> const& _parameters,
+		bool _isVirtual = false,
 		ASTPointer<OverrideSpecifier> const& _overrides = nullptr,
 		ASTPointer<ParameterList> const& _returnParameters = ASTPointer<ParameterList>()
 	):
 		Declaration(_location, _name, _visibility),
 		m_parameters(_parameters),
 		m_overrides(_overrides),
-		m_returnParameters(_returnParameters)
+		m_returnParameters(_returnParameters),
+		m_isVirtual(_isVirtual)
 	{
 	}
 
@@ -619,11 +621,14 @@ public:
 	std::vector<ASTPointer<VariableDeclaration>> const& returnParameters() const { return m_returnParameters->parameters(); }
 	ParameterList const& parameterList() const { return *m_parameters; }
 	ASTPointer<ParameterList> const& returnParameterList() const { return m_returnParameters; }
+	bool markedVirtual() const { return m_isVirtual; }
+	virtual bool virtualSemantics() const { return markedVirtual(); }
 
 protected:
 	ASTPointer<ParameterList> m_parameters;
 	ASTPointer<OverrideSpecifier> m_overrides;
 	ASTPointer<ParameterList> m_returnParameters;
+	bool m_isVirtual = false;
 };
 
 /**
@@ -661,6 +666,7 @@ public:
 		Declaration::Visibility _visibility,
 		StateMutability _stateMutability,
 		Token _kind,
+		bool _isVirtual,
 		ASTPointer<OverrideSpecifier> const& _overrides,
 		ASTPointer<ASTString> const& _documentation,
 		ASTPointer<ParameterList> const& _parameters,
@@ -668,7 +674,7 @@ public:
 		ASTPointer<ParameterList> const& _returnParameters,
 		ASTPointer<Block> const& _body
 	):
-		CallableDeclaration(_location, _name, _visibility, _parameters, _overrides, _returnParameters),
+		CallableDeclaration(_location, _name, _visibility, _parameters, _isVirtual, _overrides, _returnParameters),
 		Documented(_documentation),
 		ImplementationOptional(_body != nullptr),
 		m_stateMutability(_stateMutability),
@@ -688,7 +694,6 @@ public:
 	bool isFallback() const { return m_kind == Token::Fallback; }
 	bool isReceive() const { return m_kind == Token::Receive; }
 	Token kind() const { return m_kind; }
-	bool isOverridable() const { return !isConstructor(); }
 	bool isPayable() const { return m_stateMutability == StateMutability::Payable; }
 	std::vector<ASTPointer<ModifierInvocation>> const& modifiers() const { return m_functionModifiers; }
 	Block const& body() const { solAssert(m_body, ""); return *m_body; }
@@ -713,6 +718,12 @@ public:
 
 	FunctionDefinitionAnnotation& annotation() const override;
 
+	bool virtualSemantics() const override
+	{
+		return
+			CallableDeclaration::virtualSemantics() ||
+			annotation().contract->isInterface();
+	}
 private:
 	StateMutability m_stateMutability;
 	Token const m_kind;
@@ -750,6 +761,7 @@ public:
 		m_overrides(_overrides),
 		m_location(_referenceLocation) {}
 
+
 	void accept(ASTVisitor& _visitor) override;
 	void accept(ASTConstVisitor& _visitor) const override;
 
@@ -811,11 +823,11 @@ private:
 	/// Initially assigned value, can be missing. For local variables, this is stored inside
 	/// VariableDeclarationStatement and not here.
 	ASTPointer<Expression> m_value;
-	bool m_isStateVariable; ///< Whether or not this is a contract state variable
-	bool m_isIndexed; ///< Whether this is an indexed variable (used by events).
-	bool m_isConstant; ///< Whether the variable is a compile-time constant.
+	bool m_isStateVariable = false; ///< Whether or not this is a contract state variable
+	bool m_isIndexed = false; ///< Whether this is an indexed variable (used by events).
+	bool m_isConstant = false; ///< Whether the variable is a compile-time constant.
 	ASTPointer<OverrideSpecifier> m_overrides; ///< Contains the override specifier node
-	Location m_location; ///< Location of the variable if it is of reference type.
+	Location m_location = Location::Unspecified; ///< Location of the variable if it is of reference type.
 };
 
 /**
@@ -829,10 +841,11 @@ public:
 		ASTPointer<ASTString> const& _name,
 		ASTPointer<ASTString> const& _documentation,
 		ASTPointer<ParameterList> const& _parameters,
+		bool _isVirtual,
 		ASTPointer<OverrideSpecifier> const& _overrides,
 		ASTPointer<Block> const& _body
 	):
-		CallableDeclaration(_location, _name, Visibility::Internal, _parameters, _overrides),
+		CallableDeclaration(_location, _name, Visibility::Internal, _parameters, _isVirtual, _overrides),
 		Documented(_documentation),
 		m_body(_body)
 	{

libsolidity/parsing/Parser.cpp

@@ -466,8 +466,6 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _isStateVari
 	RecursionGuard recursionGuard(*this);
 	FunctionHeaderParserResult result;
 
-	result.overrides = nullptr;
-
 	VarDeclParserOptions options;
 	options.allowLocationSpecifier = true;
 	result.parameters = parseParameterList(options);
@@ -515,6 +513,14 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _isStateVari
 
 			result.overrides = parseOverrideSpecifier();
 		}
+		else if (!_isStateVariable && token == Token::Virtual)
+		{
+			if (result.isVirtual)
+				parserError("Virtual already specified.");
+
+			result.isVirtual = true;
+			m_scanner->next();
+		}
 		else
 			break;
 	}
@@ -591,6 +597,7 @@ ASTPointer<ASTNode> Parser::parseFunctionDefinition()
 		header.visibility,
 		header.stateMutability,
 		kind,
+		header.isVirtual,
 		header.overrides,
 		docstring,
 		header.parameters,
@@ -804,13 +811,32 @@ ASTPointer<ModifierDefinition> Parser::parseModifierDefinition()
 		parameters = createEmptyParameterList();
 
 	ASTPointer<OverrideSpecifier> overrides;
+	bool isVirtual = false;
+
+	while (true)
+	{
+		if (m_scanner->currentToken() == Token::Override)
+		{
+			if (overrides)
+				parserError("Override already specified.");
+			overrides = parseOverrideSpecifier();
+		}
+		else if (m_scanner->currentToken() == Token::Virtual)
+		{
+			if (isVirtual)
+				parserError("Virtual already specified.");
+
+			isVirtual = true;
+			m_scanner->next();
+		}
+		else
+			break;
+	}
 
-	if (m_scanner->currentToken() == Token::Override)
-		overrides = parseOverrideSpecifier();
 
 	ASTPointer<Block> block = parseBlock();
 	nodeFactory.setEndPositionFromNode(block);
-	return nodeFactory.createNode<ModifierDefinition>(name, docstring, parameters, overrides, block);
+	return nodeFactory.createNode<ModifierDefinition>(name, docstring, parameters, isVirtual, overrides, block);
 }
 
 ASTPointer<EventDefinition> Parser::parseEventDefinition()

libsolidity/parsing/Parser.h

@@ -70,6 +70,7 @@ private:
 	/// This struct is shared for parsing a function header and a function type.
 	struct FunctionHeaderParserResult
 	{
+		bool isVirtual = false;
 		ASTPointer<OverrideSpecifier> overrides;
 		ASTPointer<ParameterList> parameters;
 		ASTPointer<ParameterList> returnParameters;