Merge pull request ethereum#2692 from ethereum/shadowing-overload

Do not mark overloaded functions as shadowing
gabod2000 · Aug 4, 2017 · f3af014 · f3af014
2 parents dc0f85c + eacc67c
commit f3af014
Show file tree

Hide file tree

Showing 5 changed files with 48 additions and 7 deletions.
diff --git a/Changelog.md b/Changelog.md
@@ -5,6 +5,7 @@ Features:
 Bugfixes:
  * Code Generator: ``.delegatecall()`` should always return execution outcome.
  * Code Generator: Provide "new account gas" for low-level ``callcode`` and ``delegatecall``.
+ * Type Checker: Do not mark overloaded functions as shadowing other functions.
  * Type Checker: Disallow the ``.gas()`` modifier on ``ecrecover``, ``sha256`` and ``ripemd160``.
 
 ### 0.4.14 (2017-07-31)

diff --git a/libsolidity/analysis/DeclarationContainer.h b/libsolidity/analysis/DeclarationContainer.h
@@ -53,6 +53,7 @@ class DeclarationContainer
 	bool registerDeclaration(Declaration const& _declaration, ASTString const* _name = nullptr, bool _invisible = false, bool _update = false);
 	std::vector<Declaration const*> resolveName(ASTString const& _name, bool _recursive = false) const;
 	ASTNode const* enclosingNode() const { return m_enclosingNode; }
+	DeclarationContainer const* enclosingContainer() const { return m_enclosingContainer; }
 	std::map<ASTString, std::vector<Declaration const*>> const& declarations() const { return m_declarations; }
 	/// @returns whether declaration is valid, and if not also returns previous declaration.
 	Declaration const* conflictingDeclaration(Declaration const& _declaration, ASTString const* _name = nullptr) const;

diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp
@@ -452,13 +452,9 @@ bool DeclarationRegistrationHelper::registerDeclaration(
 		_errorLocation = &_declaration.location();
 
 	Declaration const* shadowedDeclaration = nullptr;
-	if (_warnOnShadow && !_declaration.name().empty())
-		for (auto const* decl: _container.resolveName(_declaration.name(), true))
-			if (decl != &_declaration)
-			{
-				shadowedDeclaration = decl;
-				break;
-			}
+	if (_warnOnShadow && !_declaration.name().empty() && _container.enclosingContainer())
+		for (auto const* decl: _container.enclosingContainer()->resolveName(_declaration.name(), true))
+			shadowedDeclaration = decl;
 
 	if (!_container.registerDeclaration(_declaration, _name, !_declaration.isVisibleInContract()))
 	{

diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h
@@ -169,6 +169,8 @@ class DeclarationRegistrationHelper: private ASTVisitor
 	void closeCurrentScope();
 	void registerDeclaration(Declaration& _declaration, bool _opensScope);
 
+	static bool isOverloadedFunction(Declaration const& _declaration1, Declaration const& _declaration2);
+
 	/// @returns the canonical name of the current scope.
 	std::string currentCanonicalName() const;
 

diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -6134,6 +6134,25 @@ BOOST_AUTO_TEST_CASE(shadowing_builtins_with_variables)
 	CHECK_WARNING(text, "shadows a builtin symbol");
 }
 
+BOOST_AUTO_TEST_CASE(shadowing_builtins_with_storage_variables)
+{
+	char const* text = R"(
+		contract C {
+			uint msg;
+		}
+	)";
+	CHECK_WARNING(text, "shadows a builtin symbol");
+}
+
+BOOST_AUTO_TEST_CASE(shadowing_builtin_at_global_scope)
+{
+	char const* text = R"(
+		contract msg {
+		}
+	)";
+	CHECK_WARNING(text, "shadows a builtin symbol");
+}
+
 BOOST_AUTO_TEST_CASE(shadowing_builtins_with_parameters)
 {
 	char const* text = R"(
@@ -6190,6 +6209,28 @@ BOOST_AUTO_TEST_CASE(shadowing_builtins_ignores_constructor)
 	CHECK_SUCCESS_NO_WARNINGS(text);
 }
 
+BOOST_AUTO_TEST_CASE(function_overload_is_not_shadowing)
+{
+	char const* text = R"(
+		contract C {
+			function f() {}
+			function f(uint) {}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
+BOOST_AUTO_TEST_CASE(function_override_is_not_shadowing)
+{
+	char const* text = R"(
+		contract D { function f() {} }
+		contract C is D {
+			function f(uint) {}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
 BOOST_AUTO_TEST_CASE(callable_crash)
 {
 	char const* text = R"(