Simplify DeclarationRegistrationHelper

libsolidity/analysis/NameAndTypeResolver.cpp

@@ -109,7 +109,7 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So
 					else
 						for (Declaration const* declaration: declarations)
 							if (!DeclarationRegistrationHelper::registerDeclaration(
-								target, *declaration, alias.alias.get(), &alias.location, true, false, m_errorReporter
+								target, *declaration, alias.alias.get(), &alias.location, false, m_errorReporter
 							))
 								error = true;
 				}
@@ -117,7 +117,7 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So
 				for (auto const& nameAndDeclaration: scope->second->declarations())
 					for (auto const& declaration: nameAndDeclaration.second)
 						if (!DeclarationRegistrationHelper::registerDeclaration(
-							target, *declaration, &nameAndDeclaration.first, &imp->location(), true, false, m_errorReporter
+							target, *declaration, &nameAndDeclaration.first, &imp->location(), false, m_errorReporter
 						))
 							error =  true;
 		}
@@ -446,7 +446,6 @@ bool DeclarationRegistrationHelper::registerDeclaration(
 	Declaration const& _declaration,
 	string const* _name,
 	SourceLocation const* _errorLocation,
-	bool _warnOnShadow,
 	bool _inactive,
 	ErrorReporter& _errorReporter
 )
@@ -456,7 +455,11 @@ bool DeclarationRegistrationHelper::registerDeclaration(
 
 	string name = _name ? *_name : _declaration.name();
 	Declaration const* shadowedDeclaration = nullptr;
-	if (_warnOnShadow && !name.empty() && _container.enclosingContainer())
+	// Do not warn about shadowing for structs and enums because their members are
+	// not accessible without prefixes. Also do not warn about event parameters
+	// because they do not participate in any proper scope.
+	bool warnOnShadow = !_declaration.isStructMember() && !_declaration.isEnumValue() && !_declaration.isEventParameter();
+	if (warnOnShadow && !name.empty() && _container.enclosingContainer())
 		for (auto const* decl: _container.enclosingContainer()->resolveName(name, true, true))
 			shadowedDeclaration = decl;
 
@@ -623,23 +626,13 @@ void DeclarationRegistrationHelper::closeCurrentScope()
 void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaration)
 {
 	solAssert(m_currentScope && m_scopes.count(m_currentScope), "No current scope.");
-
+	solAssert(m_currentScope == _declaration.scope(), "Unexpected current scope.");
-	bool warnAboutShadowing = true;
-	// Do not warn about shadowing for structs and enums because their members are
-	// not accessible without prefixes. Also do not warn about event parameters
-	// because they don't participate in any proper scope.
-	if (
-		dynamic_cast<StructDefinition const*>(m_currentScope) ||
-		dynamic_cast<EnumDefinition const*>(m_currentScope) ||
-		dynamic_cast<EventDefinition const*>(m_currentScope)
-	)
-		warnAboutShadowing = false;
 
 	// Register declaration as inactive if we are in block scope.
 	bool inactive =
 		(dynamic_cast<Block const*>(m_currentScope) || dynamic_cast<ForStatement const*>(m_currentScope));
 
-	registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, inactive, m_errorReporter);
+	registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, inactive, m_errorReporter);
 
 	solAssert(_declaration.annotation().scope == m_currentScope, "");
 	solAssert(_declaration.annotation().contract == m_currentContract, "");

libsolidity/analysis/NameAndTypeResolver.h

@@ -152,7 +152,6 @@ public:
 		Declaration const& _declaration,
 		std::string const* _name,
 		langutil::SourceLocation const* _errorLocation,
-		bool _warnOnShadow,
 		bool _inactive,
 		langutil::ErrorReporter& _errorReporter
 	);

libsolidity/ast/AST.cpp

@@ -495,6 +495,24 @@ string Scopable::sourceUnitName() const
 	return *sourceUnit().annotation().path;
 }
 
+bool Declaration::isEnumValue() const
+{
+	solAssert(scope(), "");
+	return dynamic_cast<EnumDefinition const*>(scope());
+}
+
+bool Declaration::isStructMember() const
+{
+	solAssert(scope(), "");
+	return dynamic_cast<StructDefinition const*>(scope());
+}
+
+bool Declaration::isEventParameter() const
+{
+	solAssert(scope(), "");
+	return dynamic_cast<EventDefinition const*>(scope());
+}
+
 DeclarationAnnotation& Declaration::annotation() const
 {
 	return initAnnotation<DeclarationAnnotation>();
@@ -617,11 +635,6 @@ bool VariableDeclaration::isLibraryFunctionParameter() const
 	return false;
 }
 
-bool VariableDeclaration::isEventParameter() const
-{
-	return dynamic_cast<EventDefinition const*>(scope()) != nullptr;
-}
-
 bool VariableDeclaration::hasReferenceOrMappingType() const
 {
 	solAssert(typeName().annotation().type, "Can only be called after reference resolution");

libsolidity/ast/AST.h

@@ -258,10 +258,16 @@ public:
 	bool isVisibleAsLibraryMember() const { return visibility() >= Visibility::Internal; }
 	virtual bool isVisibleViaContractTypeAccess() const { return false; }
 
-
 	virtual bool isLValue() const { return false; }
 	virtual bool isPartOfExternalInterface() const { return false; }
 
+	/// @returns true if this is a declaration of an enum member.
+	bool isEnumValue() const;
+	/// @returns true if this is a declaration of a struct member.
+	bool isStructMember() const;
+	/// @returns true if this is a declaration of a parameter of an event.
+	bool isEventParameter() const;
+
 	/// @returns the type of expressions referencing this declaration.
 	/// This can only be called once types of variable declarations have already been resolved.
 	virtual TypePointer type() const = 0;
@@ -952,10 +958,6 @@ public:
 	bool isConstructorParameter() const;
 	/// @returns true iff this variable is a parameter(or return parameter of a library function
 	bool isLibraryFunctionParameter() const;
-	/// @returns true if the type of the variable does not need to be specified, i.e. it is declared
-	/// in the body of a function or modifier.
-	/// @returns true if this variable is a parameter of an event.
-	bool isEventParameter() const;
 	/// @returns true if the type of the variable is a reference or mapping type, i.e.
 	/// array, struct or mapping. These types can take a data location (and often require it).
 	/// Can only be called after reference resolution.