Merge pull request #4306 from ethereum/remove_050_workaround_scoping

[BREAKING] C99 scoping rules by default (remove 050 workaround)

4 files changed, 23 insertions, 36 deletions

diff --git a/libsolidity/analysis/DeclarationContainer.h b/libsolidity/analysis/DeclarationContainer.h
index e4b3320a..a3e0bd0a 100644
--- a/libsolidity/analysis/DeclarationContainer.h
+++ b/libsolidity/analysis/DeclarationContainer.h
@@ -58,7 +58,7 @@ public:
 	/// @returns whether declaration is valid, and if not also returns previous declaration.
 	Declaration const* conflictingDeclaration(Declaration const& _declaration, ASTString const* _name = nullptr) const;
 
-	/// Activates a previously inactive (invisible) variable. To be used in C99 scpoing for
+	/// Activates a previously inactive (invisible) variable. To be used in C99 scoping for
 	/// VariableDeclarationStatements.
 	void activateVariable(ASTString const& _name);
 
diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp
index 0a356f04..b856544a 100644
--- a/libsolidity/analysis/NameAndTypeResolver.cpp
+++ b/libsolidity/analysis/NameAndTypeResolver.cpp
@@ -54,11 +54,10 @@ NameAndTypeResolver::NameAndTypeResolver(
 
 bool NameAndTypeResolver::registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope)
 {
-	bool useC99Scoping = _sourceUnit.annotation().experimentalFeatures.count(ExperimentalFeature::V050);
 	// The helper registers all declarations in m_scopes as a side-effect of its construction.
 	try
 	{
-		DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, useC99Scoping, m_errorReporter, _currentScope);
+		DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, m_errorReporter, _currentScope);
 	}
 	catch (FatalError const&)
 	{
@@ -449,11 +448,9 @@ string NameAndTypeResolver::similarNameSuggestions(ASTString const& _name) const
 DeclarationRegistrationHelper::DeclarationRegistrationHelper(
 	map<ASTNode const*, shared_ptr<DeclarationContainer>>& _scopes,
 	ASTNode& _astRoot,
-	bool _useC99Scoping,
 	ErrorReporter& _errorReporter,
 	ASTNode const* _currentScope
 ):
-	m_useC99Scoping(_useC99Scoping),
 	m_scopes(_scopes),
 	m_currentScope(_currentScope),
 	m_errorReporter(_errorReporter)
@@ -629,29 +626,25 @@ void DeclarationRegistrationHelper::endVisit(ModifierDefinition&)
 bool DeclarationRegistrationHelper::visit(Block& _block)
 {
 	_block.setScope(m_currentScope);
-	if (m_useC99Scoping)
-		enterNewSubScope(_block);
+	enterNewSubScope(_block);
 	return true;
 }
 
 void DeclarationRegistrationHelper::endVisit(Block&)
 {
-	if (m_useC99Scoping)
-		closeCurrentScope();
+	closeCurrentScope();
 }
 
 bool DeclarationRegistrationHelper::visit(ForStatement& _for)
 {
 	_for.setScope(m_currentScope);
-	if (m_useC99Scoping)
-		enterNewSubScope(_for);
+	enterNewSubScope(_for);
 	return true;
 }
 
 void DeclarationRegistrationHelper::endVisit(ForStatement&)
 {
-	if (m_useC99Scoping)
-		closeCurrentScope();
+	closeCurrentScope();
 }
 
 void DeclarationRegistrationHelper::endVisit(VariableDeclarationStatement& _variableDeclarationStatement)
@@ -716,9 +709,8 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio
 		if (fun->isConstructor())
 			warnAboutShadowing = false;
 
-	// Register declaration as inactive if we are in block scope and C99 mode.
+	// Register declaration as inactive if we are in block scope.
 	bool inactive =
-		m_useC99Scoping &&
 		(dynamic_cast<Block const*>(m_currentScope) || dynamic_cast<ForStatement const*>(m_currentScope));
 
 	registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, inactive, m_errorReporter);
diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h
index 3d10fbd8..8178ee17 100644
--- a/libsolidity/analysis/NameAndTypeResolver.h
+++ b/libsolidity/analysis/NameAndTypeResolver.h
@@ -69,7 +69,7 @@ public:
 	/// that create their own scope.
 	/// @returns false in case of error.
 	bool updateDeclaration(Declaration const& _declaration);
-	/// Activates a previously inactive (invisible) variable. To be used in C99 scpoing for
+	/// Activates a previously inactive (invisible) variable. To be used in C99 scoping for
 	/// VariableDeclarationStatements.
 	void activateVariable(std::string const& _name);
 
@@ -142,7 +142,6 @@ public:
 	DeclarationRegistrationHelper(
 		std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes,
 		ASTNode& _astRoot,
-		bool _useC99Scoping,
 		ErrorReporter& _errorReporter,
 		ASTNode const* _currentScope = nullptr
 	);
@@ -190,7 +189,6 @@ private:
 	/// @returns the canonical name of the current scope.
 	std::string currentCanonicalName() const;
 
-	bool m_useC99Scoping = false;
 	std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& m_scopes;
 	ASTNode const* m_currentScope = nullptr;
 	VariableScope* m_currentFunction = nullptr;
diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp
index a051d7f9..58b659f7 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/libsolidity/analysis/ReferencesResolver.cpp
@@ -48,9 +48,7 @@ bool ReferencesResolver::visit(Block const& _block)
 	if (!m_resolveInsideCode)
 		return false;
 	m_experimental050Mode = _block.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
-	// C99-scoped variables
-	if (m_experimental050Mode)
-		m_resolver.setScope(&_block);
+	m_resolver.setScope(&_block);
 	return true;
 }
 
@@ -59,9 +57,7 @@ void ReferencesResolver::endVisit(Block const& _block)
 	if (!m_resolveInsideCode)
 		return;
 
-	// C99-scoped variables
-	if (m_experimental050Mode)
-		m_resolver.setScope(_block.scope());
+	m_resolver.setScope(_block.scope());
 }
 
 bool ReferencesResolver::visit(ForStatement const& _for)
@@ -69,9 +65,7 @@ bool ReferencesResolver::visit(ForStatement const& _for)
 	if (!m_resolveInsideCode)
 		return false;
 	m_experimental050Mode = _for.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050);
-	// C99-scoped variables
-	if (m_experimental050Mode)
-		m_resolver.setScope(&_for);
+	m_resolver.setScope(&_for);
 	return true;
 }
 
@@ -79,18 +73,16 @@ void ReferencesResolver::endVisit(ForStatement const& _for)
 {
 	if (!m_resolveInsideCode)
 		return;
-	if (m_experimental050Mode)
-		m_resolver.setScope(_for.scope());
+	m_resolver.setScope(_for.scope());
 }
 
 void ReferencesResolver::endVisit(VariableDeclarationStatement const& _varDeclStatement)
 {
 	if (!m_resolveInsideCode)
 		return;
-	if (m_experimental050Mode)
-		for (auto const& var: _varDeclStatement.declarations())
-			if (var)
-				m_resolver.activateVariable(var->name());
+	for (auto const& var: _varDeclStatement.declarations())
+		if (var)
+			m_resolver.activateVariable(var->name());
 }
 
 bool ReferencesResolver::visit(Identifier const& _identifier)
@@ -99,9 +91,14 @@ bool ReferencesResolver::visit(Identifier const& _identifier)
 	if (declarations.empty())
 	{
 		string suggestions = m_resolver.similarNameSuggestions(_identifier.name());
-		string errorMessage =
-			"Undeclared identifier." +
-			(suggestions.empty()? "": " Did you mean " + std::move(suggestions) + "?");
+		string errorMessage = "Undeclared identifier.";
+		if (!suggestions.empty())
+		{
+			if ("\"" + _identifier.name() + "\"" == suggestions)
+				errorMessage += " " + std::move(suggestions) + " is not (or not yet) visible at this point.";
+			else
+				errorMessage += " Did you mean " + std::move(suggestions) + "?";
+		}
 		declarationError(_identifier.location(), errorMessage);
 	}
 	else if (declarations.size() == 1)