Warn if modifiers are applied to functions without implementation.

5 files changed, 35 insertions, 4 deletions

diff --git a/Changelog.md b/Changelog.md
index db080b05..e2174cfd 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -14,6 +14,7 @@ Features:
  * Optimizer: Optimize across ``mload`` if ``msize()`` is not used.
  * Static Analyzer: Error on duplicated super constructor calls as experimental 0.5.0 feature.
  * Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature).
+ * Syntax Checker: Warn about modifiers on functions without implementation (this will turn into an error with version 0.5.0).
  * Syntax Tests: Add source locations to syntax test expectations.
  * General: Introduce new constructor syntax using the ``constructor`` keyword as experimental 0.5.0 feature.
  * Inheritance: Error when using empty parentheses for base class constructors that require arguments as experimental 0.5.0 feature.
diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp
index 343b4ba8..f648e5b4 100644
--- a/libsolidity/analysis/SyntaxChecker.cpp
+++ b/libsolidity/analysis/SyntaxChecker.cpp
@@ -232,6 +232,13 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function)
 				"Use \"constructor(...) { ... }\" instead."
 			);
 	}
+	if (!_function.isImplemented() && !_function.modifiers().empty())
+	{
+		if (v050)
+			m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers.");
+		else
+			m_errorReporter.warning( _function.location(), "Modifiers of functions without implementation are ignored." );
+	}
 	return true;
 }
 
diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp
index bdb46ad6..ea092a74 100644
--- a/libsolidity/parsing/Parser.cpp
+++ b/libsolidity/parsing/Parser.cpp
@@ -365,12 +365,12 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(
 		Token::Value token = m_scanner->currentToken();
 		if (_allowModifiers && token == Token::Identifier)
 		{
-			// This can either be a modifier (function declaration) or the name of the
-			// variable (function type name plus variable).
-			if (
+			// If the name is empty, then this can either be a modifier (fallback function declaration)
+			// or the name of the state variable (function type name plus variable).
+			if (result.name->empty() && (
 				m_scanner->peekNextToken() == Token::Semicolon ||
 				m_scanner->peekNextToken() == Token::Assign
-			)
+			))
 				// Variable declaration, break here.
 				break;
 			else
diff --git a/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_050.sol b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_050.sol
new file mode 100644
index 00000000..af1babbc
--- /dev/null
+++ b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_050.sol
@@ -0,0 +1,10 @@
+pragma experimental "v0.5.0";
+contract C
+{
+    modifier only_owner() { _; }
+    function foo() only_owner public;
+    function bar() public only_owner;
+}
+// ----
+// SyntaxError: (80-113): Functions without implementation cannot have modifiers.
+// SyntaxError: (118-151): Functions without implementation cannot have modifiers.
diff --git a/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_no_parser_error.sol b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_no_parser_error.sol
new file mode 100644
index 00000000..e18c5cf9
--- /dev/null
+++ b/test/libsolidity/syntaxTests/modifiers/modifiers_on_abstract_functions_no_parser_error.sol
@@ -0,0 +1,13 @@
+// Previous versions of Solidity turned this
+// into a parser error (they wrongly recognized
+// these functions as state variables of
+// function type).
+contract C
+{
+    modifier only_owner() { _; }
+    function foo() only_owner public;
+    function bar() public only_owner;
+}
+// ----
+// Warning: (203-236): Modifiers of functions without implementation are ignored.
+// Warning: (241-274): Modifiers of functions without implementation are ignored.