Merge pull request #4508 from ethereum/nameCollisionCrash

[BREAKING] Don't exclude public state variables when looking for conflicting declarations.

9 files changed, 44 insertions, 12 deletions

diff --git a/Changelog.md b/Changelog.md
index e6956d5e..c21f0ae9 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -32,6 +32,7 @@ Breaking Changes:
  * General: Remove assembly instruction aliases ``sha3`` and ``suicide``
  * General: C99-style scoping rules are enforced now. This was already the case in the experimental 0.5.0 mode.
  * General: Disallow combining hex numbers with unit denominations (e.g. ``0x1e wei``). This was already the case in the experimental 0.5.0 mode.
+ * Name Resolver: Do not exclude public state variables when looking for conflicting declarations.
  * Optimizer: Remove the no-op ``PUSH1 0 NOT AND`` sequence.
  * Parser: Disallow trailing dots that are not followed by a number.
  * Parser: Remove ``constant`` as function state mutability modifer.
diff --git a/docs/solidity-by-example.rst b/docs/solidity-by-example.rst
index 3934d29d..3f054297 100644
--- a/docs/solidity-by-example.rst
+++ b/docs/solidity-by-example.rst
@@ -232,7 +232,7 @@ activate themselves.
         // absolute unix timestamps (seconds since 1970-01-01)
         // or time periods in seconds.
         address public beneficiary;
-        uint public auctionEnd;
+        uint public auctionEndTime;
 
         // Current state of the auction.
         address public highestBidder;
@@ -261,7 +261,7 @@ activate themselves.
             address _beneficiary
         ) public {
             beneficiary = _beneficiary;
-            auctionEnd = now + _biddingTime;
+            auctionEndTime = now + _biddingTime;
         }
 
         /// Bid on the auction with the value sent
@@ -278,7 +278,7 @@ activate themselves.
             // Revert the call if the bidding
             // period is over.
             require(
-                now <= auctionEnd,
+                now <= auctionEndTime,
                 "Auction already ended."
             );
 
@@ -337,7 +337,7 @@ activate themselves.
             // external contracts.
 
             // 1. Conditions
-            require(now >= auctionEnd, "Auction not yet ended.");
+            require(now >= auctionEndTime, "Auction not yet ended.");
             require(!ended, "auctionEnd has already been called.");
 
             // 2. Effects
diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp
index 9e2bf6d3..347daaf8 100644
--- a/libsolidity/analysis/DeclarationContainer.cpp
+++ b/libsolidity/analysis/DeclarationContainer.cpp
@@ -49,16 +49,10 @@ Declaration const* DeclarationContainer::conflictingDeclaration(
 		dynamic_cast<MagicVariableDeclaration const*>(&_declaration)
 	)
 	{
-		// check that all other declarations with the same name are functions or a public state variable or events.
-		// And then check that the signatures are different.
+		// check that all other declarations are of the same kind (in which
+		// case the type checker will ensure that the signatures are different)
 		for (Declaration const* declaration: declarations)
 		{
-			if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(declaration))
-			{
-				if (variableDeclaration->isStateVariable() && !variableDeclaration->isConstant() && variableDeclaration->isPublic())
-					continue;
-				return declaration;
-			}
 			if (
 				dynamic_cast<FunctionDefinition const*>(&_declaration) &&
 				!dynamic_cast<FunctionDefinition const*>(declaration)
diff --git a/test/libsolidity/syntaxTests/inheritance/override/function_state_variable.sol b/test/libsolidity/syntaxTests/inheritance/override/function_state_variable.sol
new file mode 100644
index 00000000..023a161a
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/function_state_variable.sol
@@ -0,0 +1,2 @@
+interface ERC20 { function x() external returns (uint); }
+contract C is ERC20 { uint public x; }
diff --git a/test/libsolidity/syntaxTests/inheritance/override/state_variable_function.sol b/test/libsolidity/syntaxTests/inheritance/override/state_variable_function.sol
new file mode 100644
index 00000000..0f05cc8e
--- /dev/null
+++ b/test/libsolidity/syntaxTests/inheritance/override/state_variable_function.sol
@@ -0,0 +1,8 @@
+contract A {
+	uint public x;
+}
+contract C is A {
+	function x() public returns (uint);
+}
+// ----
+// DeclarationError: (50-85): Identifier already declared.
diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/412_early_exit_on_fatal_errors.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/412_early_exit_on_fatal_errors.sol
index 56fc4051..d052dab5 100644
--- a/test/libsolidity/syntaxTests/nameAndTypeResolution/412_early_exit_on_fatal_errors.sol
+++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/412_early_exit_on_fatal_errors.sol
@@ -8,4 +8,5 @@ contract C {
     }
 }
 // ----
+// DeclarationError: (150-179): Identifier already declared.
 // DeclarationError: (114-120): Identifier not found or not unique.
diff --git a/test/libsolidity/syntaxTests/scoping/function_state_variable_conflict.sol b/test/libsolidity/syntaxTests/scoping/function_state_variable_conflict.sol
new file mode 100644
index 00000000..d717981b
--- /dev/null
+++ b/test/libsolidity/syntaxTests/scoping/function_state_variable_conflict.sol
@@ -0,0 +1,6 @@
+contract C {
+    function f(uint) public pure {}
+    uint public f = 0;
+}
+// ----
+// DeclarationError: (53-70): Identifier already declared.
diff --git a/test/libsolidity/syntaxTests/scoping/state_variable_function_conflict.sol b/test/libsolidity/syntaxTests/scoping/state_variable_function_conflict.sol
new file mode 100644
index 00000000..0c732f7f
--- /dev/null
+++ b/test/libsolidity/syntaxTests/scoping/state_variable_function_conflict.sol
@@ -0,0 +1,6 @@
+contract C {
+    uint public f = 0;
+    function f(uint) public pure {}
+}
+// ----
+// DeclarationError: (40-71): Identifier already declared.
diff --git a/test/libsolidity/syntaxTests/scoping/state_variable_function_conflict_former_crash.sol b/test/libsolidity/syntaxTests/scoping/state_variable_function_conflict_former_crash.sol
new file mode 100644
index 00000000..fb9180c6
--- /dev/null
+++ b/test/libsolidity/syntaxTests/scoping/state_variable_function_conflict_former_crash.sol
@@ -0,0 +1,14 @@
+// This used to crash with some compiler versions.
+contract SomeContract {
+
+  uint public balance = 0;
+
+  function balance(uint number) public {}
+
+  function doSomething() public {
+    balance(3);
+  }
+}
+// ----
+// DeclarationError: (106-145): Identifier already declared.
+// TypeError: (185-195): Type is not callable