Error on duplicated super constructor calls

7 files changed, 55 insertions, 9 deletions

diff --git a/Changelog.md b/Changelog.md
index d1e199b7..94703f7d 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -28,6 +28,7 @@ Bugfixes:
  * Type System: Make external library functions accessible.
  * Type System: Prevent encoding of weird types.
  * Static Analyzer: Fix non-deterministic order of unused variable warnings.
+ * Static Analyzer: Error on duplicated super constructor calls.
 
 ### 0.4.21 (2018-03-07)
 
diff --git a/docs/contracts.rst b/docs/contracts.rst
index f8a44fb3..0dd9845c 100644
--- a/docs/contracts.rst
+++ b/docs/contracts.rst
@@ -1034,9 +1034,12 @@ the base constructors. This can be done in two ways::
         constructor(uint _x) public { x = _x; }
     }
 
-    contract Derived is Base(7) {
-        constructor(uint _y) Base(_y * _y) public {
-        }
+    contract Derived1 is Base(7) {
+        constructor(uint _y) public {}
+    }
+
+    contract Derived2 is Base {
+        constructor(uint _y) Base(_y * _y) public {}
     }
 
 One way is directly in the inheritance list (``is Base(7)``).  The other is in
@@ -1046,8 +1049,9 @@ do it is more convenient if the constructor argument is a
 constant and defines the behaviour of the contract or
 describes it. The second way has to be used if the
 constructor arguments of the base depend on those of the
-derived contract. If, as in this silly example, both places
-are used, the modifier-style argument takes precedence.
+derived contract. Arguments have to be given either in the
+inheritance list or in modifier-style in the derived constuctor.
+Specifying arguments in both places is an error.
 
 .. index:: ! inheritance;multiple, ! linearization, ! C3 linearization
 
diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp
index 33b0e296..6c70ba6b 100644
--- a/libsolidity/analysis/StaticAnalyzer.cpp
+++ b/libsolidity/analysis/StaticAnalyzer.cpp
@@ -90,6 +90,38 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&)
 	m_localVarUseCount.clear();
 }
 
+bool modifierOverridesInheritanceSpecifier(
+		ContractDefinition const* _contract,
+		ModifierInvocation const& _modifier,
+		InheritanceSpecifier const& _specifier
+)
+{
+	auto parent = _specifier.name().annotation().referencedDeclaration;
+	return _contract == parent && (!_specifier.arguments().empty() || _modifier.arguments().empty());
+}
+
+bool StaticAnalyzer::visit(ModifierInvocation const& _modifier)
+{
+	if (!m_constructor)
+		return true;
+
+	if (auto contract = dynamic_cast<ContractDefinition const*>(_modifier.name()->annotation().referencedDeclaration))
+		for (auto const& specifier: m_currentContract->baseContracts())
+			if (modifierOverridesInheritanceSpecifier(contract, _modifier, *specifier))
+			{
+				SecondarySourceLocation ssl;
+				ssl.append("Overriden constructor call is here:", specifier->location());
+
+				m_errorReporter.declarationError(
+					_modifier.location(),
+					ssl,
+					"Duplicated super constructor call."
+				);
+			}
+
+	return true;
+}
+
 bool StaticAnalyzer::visit(Identifier const& _identifier)
 {
 	if (m_currentFunction)
diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h
index 0a806bbd..e68325bc 100644
--- a/libsolidity/analysis/StaticAnalyzer.h
+++ b/libsolidity/analysis/StaticAnalyzer.h
@@ -57,6 +57,7 @@ private:
 
 	virtual bool visit(FunctionDefinition const& _function) override;
 	virtual void endVisit(FunctionDefinition const& _function) override;
+	virtual bool visit(ModifierInvocation const& _modifier) override;
 
 	virtual bool visit(ExpressionStatement const& _statement) override;
 	virtual bool visit(VariableDeclaration const& _variable) override;
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index beeae786..5ed53a2f 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -5191,7 +5191,7 @@ BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base)
 			}
 			uint public m_i;
 		}
-		contract Derived is Base(2) {
+		contract Derived is Base {
 			function Derived(uint i) Base(i)
 			{}
 		}
@@ -5211,10 +5211,10 @@ BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base_base)
 			}
 			uint public m_i;
 		}
-		contract Base1 is Base(3) {
+		contract Base1 is Base {
 			function Base1(uint k) Base(k*k) {}
 		}
-		contract Derived is Base(3), Base1(2) {
+		contract Derived is Base, Base1 {
 			function Derived(uint i) Base(i) Base1(i)
 			{}
 		}
@@ -5235,7 +5235,7 @@ BOOST_AUTO_TEST_CASE(pass_dynamic_arguments_to_the_base_base_with_gap)
 			uint public m_i;
 		}
 		contract Base1 is Base(3) {}
-		contract Derived is Base(2), Base1 {
+		contract Derived is Base, Base1 {
 			function Derived(uint i) Base(i) {}
 		}
 		contract Final is Derived(4) {
diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call.sol
new file mode 100644
index 00000000..95df1040
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call.sol
@@ -0,0 +1,4 @@
+contract A { constructor(uint) public { } }
+contract B is A(2) { constructor() A(3) public {  } }
+// ----
+// DeclarationError: Duplicated super constructor call.
diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call_empty.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call_empty.sol
new file mode 100644
index 00000000..f8024ad6
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call_empty.sol
@@ -0,0 +1,4 @@
+contract A { constructor() public { } }
+contract B is A { constructor() A() public {  } }
+// ----
+// DeclarationError: Duplicated super constructor call.