From 7bc6645218bac2642ef0a0b255e31be48895026f Mon Sep 17 00:00:00 2001
From: Mathias Baumann <git@supradigital.org>
Date: Wed, 27 Nov 2019 14:02:52 +0100
Subject: [PATCH] Disallow use of virtual and private together

---
 Changelog.md                                             | 3 ++-
 docs/contracts/inheritance.rst                           | 9 +++++++--
 libsolidity/analysis/TypeChecker.cpp                     | 9 +++++++--
 .../syntaxTests/inheritance/override/virtual_private.sol | 8 ++++++++
 4 files changed, 24 insertions(+), 5 deletions(-)
 create mode 100644 test/libsolidity/syntaxTests/inheritance/override/virtual_private.sol

diff --git a/Changelog.md b/Changelog.md
index c537e1b19..5b866f9ec 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -21,6 +21,7 @@ Breaking changes:
  * Natspec JSON Interface: Properly support multiple ``@return`` statements in ``@dev`` documentation and enforce named return parameters to be mentioned documentation.
  * Source mappings: Add "modifier depth" as a fifth field in the source mappings.
  * AST: Inline assembly is exported as structured JSON instead of plain string.
+ * General: ``private`` cannot be used together with ``virtual``.
 
 Language Features:
  * Allow global enums and structs.
@@ -28,7 +29,7 @@ Language Features:
  * Allow explicit conversions from ``address`` to ``address payable`` via ``payable(...)``.
  * Introduce syntax for array slices and implement them for dynamic calldata arrays.
  * Introduce ``push()`` for dynamic storage arrays. It returns a reference to the newly allocated element, if applicable.
- * Introduce ``virtual`` and ``override`` keywords
+ * Introduce ``virtual`` and ``override`` keywords.
  * Modify ``push(element)`` for dynamic storage arrays such that it does not return the new length anymore.
  * Yul: Introduce ``leave`` statement that exits the current function.
 
diff --git a/docs/contracts/inheritance.rst b/docs/contracts/inheritance.rst
index 9e7c17d5f..81ad06f44 100644
--- a/docs/contracts/inheritance.rst
+++ b/docs/contracts/inheritance.rst
@@ -181,8 +181,8 @@ Function Overriding
 ===================
 
 Base functions can be overridden by inheriting contracts to change their
-behavior. The overriding function must then use the ``override`` keyword in the
-function header as shown in this example:
+behavior if they are marked as ``virtual``. The overriding function must then
+use the ``override`` keyword in the function header as shown in this example:
 
 ::
 
@@ -240,6 +240,11 @@ overridden when used with multiple inheritance:
     // No explicit override required
     contract D is B, C {}
 
+.. note::
+
+  Functions with the ``private`` visibility cannot be ``virtual``.
+
+
 .. _modifier-overriding:
 
 .. index:: ! overriding;modifier
diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp
index 912a568b5..d32da45b4 100644
--- a/libsolidity/analysis/TypeChecker.cpp
+++ b/libsolidity/analysis/TypeChecker.cpp
@@ -327,8 +327,13 @@ 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.markedVirtual())
+	{
+		if (_function.annotation().contract->isInterface())
+			m_errorReporter.warning(_function.location(), "Interface functions are implicitly \"virtual\"");
+		if (_function.visibility() == Declaration::Visibility::Private)
+			m_errorReporter.typeError(_function.location(), "\"virtual\" and \"private\" cannot be used together.");
+	}
 
 	if (_function.isPayable())
 	{
diff --git a/test/libsolidity/syntaxTests/inheritance/override/virtual_private.sol b/test/libsolidity/syntaxTests/inheritance/override/virtual_private.sol
new file mode 100644
index 000000000..95690e7db
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/virtual_private.sol
@@ -0,0 +1,8 @@
+abstract contract A {
+	function test() private virtual returns (uint256);
+}
+abstract contract X is A {
+	function test() private override returns (uint256);
+}
+// ----
+// TypeError: (23-73): "virtual" and "private" cannot be used together.