Merge pull request #3476 from ethereum/scoping

C99/C++ scoping rules

8 files changed, 236 insertions, 71 deletions

diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp
index 7508ad9e..c7ba78d6 100644
--- a/libsolidity/analysis/DeclarationContainer.cpp
+++ b/libsolidity/analysis/DeclarationContainer.cpp
@@ -79,6 +79,17 @@ Declaration const* DeclarationContainer::conflictingDeclaration(
 	return nullptr;
 }
 
+void DeclarationContainer::activateVariable(ASTString const& _name)
+{
+	solAssert(
+		m_invisibleDeclarations.count(_name) && m_invisibleDeclarations.at(_name).size() == 1,
+		"Tried to activate a non-inactive variable or multiple inactive variables with the same name."
+	);
+	solAssert(m_declarations.count(_name) == 0 || m_declarations.at(_name).empty(), "");
+	m_declarations[_name].emplace_back(m_invisibleDeclarations.at(_name).front());
+	m_invisibleDeclarations.erase(_name);
+}
+
 bool DeclarationContainer::registerDeclaration(
 	Declaration const& _declaration,
 	ASTString const* _name,
@@ -106,15 +117,17 @@ bool DeclarationContainer::registerDeclaration(
 	return true;
 }
 
-vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _name, bool _recursive) const
+vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _name, bool _recursive, bool _alsoInvisible) const
 {
 	solAssert(!_name.empty(), "Attempt to resolve empty name.");
-	auto result = m_declarations.find(_name);
-	if (result != m_declarations.end())
-		return result->second;
-	if (_recursive && m_enclosingContainer)
-		return m_enclosingContainer->resolveName(_name, true);
-	return vector<Declaration const*>({});
+	vector<Declaration const*> result;
+	if (m_declarations.count(_name))
+		result = m_declarations.at(_name);
+	if (_alsoInvisible && m_invisibleDeclarations.count(_name))
+		result += m_invisibleDeclarations.at(_name);
+	if (result.empty() && _recursive && m_enclosingContainer)
+		result = m_enclosingContainer->resolveName(_name, true, _alsoInvisible);
+	return result;
 }
 
 vector<ASTString> DeclarationContainer::similarNames(ASTString const& _name) const
@@ -129,6 +142,12 @@ vector<ASTString> DeclarationContainer::similarNames(ASTString const& _name) con
 		if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE))
 			similar.push_back(declarationName);
 	}
+	for (auto const& declaration: m_invisibleDeclarations)
+	{
+		string const& declarationName = declaration.first;
+		if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE))
+			similar.push_back(declarationName);
+	}
 
 	if (m_enclosingContainer)
 		similar += m_enclosingContainer->similarNames(_name);
diff --git a/libsolidity/analysis/DeclarationContainer.h b/libsolidity/analysis/DeclarationContainer.h
index f9b1bda4..e4b3320a 100644
--- a/libsolidity/analysis/DeclarationContainer.h
+++ b/libsolidity/analysis/DeclarationContainer.h
@@ -51,13 +51,17 @@ public:
 	/// @param _update if true, replaces a potential declaration that is already present
 	/// @returns false if the name was already declared.
 	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;
+	std::vector<Declaration const*> resolveName(ASTString const& _name, bool _recursive = false, bool _alsoInvisible = 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;
 
+	/// Activates a previously inactive (invisible) variable. To be used in C99 scpoing for
+	/// VariableDeclarationStatements.
+	void activateVariable(ASTString const& _name);
+
 	/// @returns existing declaration names similar to @a _name.
 	/// Searches this and all parent containers.
 	std::vector<ASTString> similarNames(ASTString const& _name) const;
diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp
index 662792a3..2f675135 100644
--- a/libsolidity/analysis/NameAndTypeResolver.cpp
+++ b/libsolidity/analysis/NameAndTypeResolver.cpp
@@ -50,12 +50,13 @@ NameAndTypeResolver::NameAndTypeResolver(
 		m_scopes[nullptr]->registerDeclaration(*declaration);
 }
 
-bool NameAndTypeResolver::registerDeclarations(ASTNode& _sourceUnit, ASTNode const* _currentScope)
+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, m_errorReporter, _currentScope);
+		DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, useC99Scoping, m_errorReporter, _currentScope);
 	}
 	catch (FatalError const&)
 	{
@@ -106,7 +107,7 @@ bool NameAndTypeResolver::performImports(SourceUnit& _sourceUnit, map<string, So
 					else
 						for (Declaration const* declaration: declarations)
 							if (!DeclarationRegistrationHelper::registerDeclaration(
-								target, *declaration, alias.second.get(), &imp->location(), true, m_errorReporter
+								target, *declaration, alias.second.get(), &imp->location(), true, false, m_errorReporter
 							))
 								error = true;
 				}
@@ -114,7 +115,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, m_errorReporter
+							target, *declaration, &nameAndDeclaration.first, &imp->location(), true, false, m_errorReporter
 						))
 							error =  true;
 		}
@@ -151,6 +152,12 @@ bool NameAndTypeResolver::updateDeclaration(Declaration const& _declaration)
 	return true;
 }
 
+void NameAndTypeResolver::activateVariable(string const& _name)
+{
+	solAssert(m_currentScope, "");
+	m_currentScope->activateVariable(_name);
+}
+
 vector<Declaration const*> NameAndTypeResolver::resolveName(ASTString const& _name, ASTNode const* _scope) const
 {
 	auto iterator = m_scopes.find(_scope);
@@ -159,15 +166,15 @@ vector<Declaration const*> NameAndTypeResolver::resolveName(ASTString const& _na
 	return iterator->second->resolveName(_name, false);
 }
 
-vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name, bool _recursive) const
+vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name, bool _includeInvisibles) const
 {
-	return m_currentScope->resolveName(_name, _recursive);
+	return m_currentScope->resolveName(_name, true, _includeInvisibles);
 }
 
-Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> const& _path, bool _recursive) const
+Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> const& _path) const
 {
 	solAssert(!_path.empty(), "");
-	vector<Declaration const*> candidates = m_currentScope->resolveName(_path.front(), _recursive);
+	vector<Declaration const*> candidates = m_currentScope->resolveName(_path.front(), true);
 	for (size_t i = 1; i < _path.size() && candidates.size() == 1; i++)
 	{
 		if (!m_scopes.count(candidates.front()))
@@ -229,7 +236,7 @@ void NameAndTypeResolver::warnVariablesNamedLikeInstructions()
 	for (auto const& instruction: c_instructions)
 	{
 		string const instructionName{boost::algorithm::to_lower_copy(instruction.first)};
-		auto declarations = nameFromCurrentScope(instructionName);
+		auto declarations = nameFromCurrentScope(instructionName, true);
 		for (Declaration const* const declaration: declarations)
 		{
 			solAssert(!!declaration, "");
@@ -244,19 +251,24 @@ void NameAndTypeResolver::warnVariablesNamedLikeInstructions()
 	}
 }
 
+void NameAndTypeResolver::setScope(ASTNode const* _node)
+{
+	m_currentScope = m_scopes[_node].get();
+}
+
 bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode)
 {
 	if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(&_node))
 	{
 		bool success = true;
-		m_currentScope = m_scopes[contract->scope()].get();
+		setScope(contract->scope());
 		solAssert(!!m_currentScope, "");
 
 		for (ASTPointer<InheritanceSpecifier> const& baseContract: contract->baseContracts())
 			if (!resolveNamesAndTypes(*baseContract, true))
 				success = false;
 
-		m_currentScope = m_scopes[contract].get();
+		setScope(contract);
 
 		if (success)
 		{
@@ -273,7 +285,7 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res
 		// these can contain code, only resolve parameters for now
 		for (ASTPointer<ASTNode> const& node: contract->subNodes())
 		{
-			m_currentScope = m_scopes[contract].get();
+			setScope(contract);
 			if (!resolveNamesAndTypes(*node, false))
 			{
 				success = false;
@@ -287,12 +299,12 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res
 		if (!_resolveInsideCode)
 			return success;
 
-		m_currentScope = m_scopes[contract].get();
+		setScope(contract);
 
 		// now resolve references inside the code
 		for (ASTPointer<ASTNode> const& node: contract->subNodes())
 		{
-			m_currentScope = m_scopes[contract].get();
+			setScope(contract);
 			if (!resolveNamesAndTypes(*node, true))
 				success = false;
 		}
@@ -301,7 +313,7 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res
 	else
 	{
 		if (m_scopes.count(&_node))
-			m_currentScope = m_scopes[&_node].get();
+			setScope(&_node);
 		return ReferencesResolver(m_errorReporter, *this, _resolveInsideCode).resolve(_node);
 	}
 }
@@ -434,9 +446,11 @@ 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)
@@ -451,6 +465,7 @@ bool DeclarationRegistrationHelper::registerDeclaration(
 	string const* _name,
 	SourceLocation const* _errorLocation,
 	bool _warnOnShadow,
+	bool _inactive,
 	ErrorReporter& _errorReporter
 )
 {
@@ -460,10 +475,13 @@ bool DeclarationRegistrationHelper::registerDeclaration(
 	string name = _name ? *_name : _declaration.name();
 	Declaration const* shadowedDeclaration = nullptr;
 	if (_warnOnShadow && !name.empty() && _container.enclosingContainer())
-		for (auto const* decl: _container.enclosingContainer()->resolveName(name, true))
+		for (auto const* decl: _container.enclosingContainer()->resolveName(name, true, true))
 			shadowedDeclaration = decl;
 
-	if (!_container.registerDeclaration(_declaration, _name, !_declaration.isVisibleInContract()))
+	// We use "invisible" for both inactive variables in blocks and for members invisible in contracts.
+	// They cannot both be true at the same time.
+	solAssert(!(_inactive && !_declaration.isVisibleInContract()), "");
+	if (!_container.registerDeclaration(_declaration, _name, !_declaration.isVisibleInContract() || _inactive))
 	{
 		SourceLocation firstDeclarationLocation;
 		SourceLocation secondDeclarationLocation;
@@ -605,6 +623,34 @@ void DeclarationRegistrationHelper::endVisit(ModifierDefinition&)
 	closeCurrentScope();
 }
 
+bool DeclarationRegistrationHelper::visit(Block& _block)
+{
+	_block.setScope(m_currentScope);
+	if (m_useC99Scoping)
+		enterNewSubScope(_block);
+	return true;
+}
+
+void DeclarationRegistrationHelper::endVisit(Block&)
+{
+	if (m_useC99Scoping)
+		closeCurrentScope();
+}
+
+bool DeclarationRegistrationHelper::visit(ForStatement& _for)
+{
+	_for.setScope(m_currentScope);
+	if (m_useC99Scoping)
+		enterNewSubScope(_for);
+	return true;
+}
+
+void DeclarationRegistrationHelper::endVisit(ForStatement&)
+{
+	if (m_useC99Scoping)
+		closeCurrentScope();
+}
+
 void DeclarationRegistrationHelper::endVisit(VariableDeclarationStatement& _variableDeclarationStatement)
 {
 	// Register the local variables with the function
@@ -632,14 +678,14 @@ void DeclarationRegistrationHelper::endVisit(EventDefinition&)
 	closeCurrentScope();
 }
 
-void DeclarationRegistrationHelper::enterNewSubScope(Declaration const& _declaration)
+void DeclarationRegistrationHelper::enterNewSubScope(ASTNode& _subScope)
 {
 	map<ASTNode const*, shared_ptr<DeclarationContainer>>::iterator iter;
 	bool newlyAdded;
 	shared_ptr<DeclarationContainer> container(new DeclarationContainer(m_currentScope, m_scopes[m_currentScope].get()));
-	tie(iter, newlyAdded) = m_scopes.emplace(&_declaration, move(container));
+	tie(iter, newlyAdded) = m_scopes.emplace(&_subScope, move(container));
 	solAssert(newlyAdded, "Unable to add new scope.");
-	m_currentScope = &_declaration;
+	m_currentScope = &_subScope;
 }
 
 void DeclarationRegistrationHelper::closeCurrentScope()
@@ -667,7 +713,12 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio
 		if (fun->isConstructor())
 			warnAboutShadowing = false;
 
-	registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, m_errorReporter);
+	// Register declaration as inactive if we are in block scope and C99 mode.
+	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);
 
 	_declaration.setScope(m_currentScope);
 	if (_opensScope)
diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h
index 9aea07ab..3d10fbd8 100644
--- a/libsolidity/analysis/NameAndTypeResolver.h
+++ b/libsolidity/analysis/NameAndTypeResolver.h
@@ -56,7 +56,7 @@ public:
 	/// @returns false in case of error.
 	/// @param _currentScope should be nullptr but can be used to inject new declarations into
 	/// existing scopes, used by the snippets feature.
-	bool registerDeclarations(ASTNode& _sourceUnit, ASTNode const* _currentScope = nullptr);
+	bool registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope = nullptr);
 	/// Applies the effect of import directives.
 	bool performImports(SourceUnit& _sourceUnit, std::map<std::string, SourceUnit const*> const& _sourceUnits);
 	/// Resolves all names and types referenced from the given AST Node.
@@ -69,20 +69,24 @@ 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
+	/// VariableDeclarationStatements.
+	void activateVariable(std::string const& _name);
 
 	/// Resolves the given @a _name inside the scope @a _scope. If @a _scope is omitted,
 	/// the global scope is used (i.e. the one containing only the pre-defined global variables).
 	/// @returns a pointer to the declaration on success or nullptr on failure.
+	/// SHOULD only be used for testing.
 	std::vector<Declaration const*> resolveName(ASTString const& _name, ASTNode const* _scope = nullptr) const;
 
-	/// Resolves a name in the "current" scope. Should only be called during the initial
-	/// resolving phase.
-	std::vector<Declaration const*> nameFromCurrentScope(ASTString const& _name, bool _recursive = true) const;
+	/// Resolves a name in the "current" scope, but also searches parent scopes.
+	/// Should only be called during the initial resolving phase.
+	std::vector<Declaration const*> nameFromCurrentScope(ASTString const& _name, bool _includeInvisibles = false) const;
 
-	/// Resolves a path starting from the "current" scope. Should only be called during the initial
-	/// resolving phase.
+	/// Resolves a path starting from the "current" scope, but also searches parent scopes.
+	/// Should only be called during the initial resolving phase.
 	/// @note Returns a null pointer if any component in the path was not unique or not found.
-	Declaration const* pathFromCurrentScope(std::vector<ASTString> const& _path, bool _recursive = true) const;
+	Declaration const* pathFromCurrentScope(std::vector<ASTString> const& _path) const;
 
 	/// returns the vector of declarations without repetitions
 	std::vector<Declaration const*> cleanedDeclarations(
@@ -96,6 +100,9 @@ public:
 	/// @returns a list of similar identifiers in the current and enclosing scopes. May return empty string if no suggestions.
 	std::string similarNameSuggestions(ASTString const& _name) const;
 
+	/// Sets the current scope.
+	void setScope(ASTNode const* _node);
+
 private:
 	/// Internal version of @a resolveNamesAndTypes (called from there) throws exceptions on fatal errors.
 	bool resolveNamesAndTypesInternal(ASTNode& _node, bool _resolveInsideCode = true);
@@ -135,6 +142,7 @@ public:
 	DeclarationRegistrationHelper(
 		std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes,
 		ASTNode& _astRoot,
+		bool _useC99Scoping,
 		ErrorReporter& _errorReporter,
 		ASTNode const* _currentScope = nullptr
 	);
@@ -145,6 +153,7 @@ public:
 		std::string const* _name,
 		SourceLocation const* _errorLocation,
 		bool _warnOnShadow,
+		bool _inactive,
 		ErrorReporter& _errorReporter
 	);
 
@@ -163,12 +172,16 @@ private:
 	void endVisit(FunctionDefinition& _function) override;
 	bool visit(ModifierDefinition& _modifier) override;
 	void endVisit(ModifierDefinition& _modifier) override;
+	bool visit(Block& _block) override;
+	void endVisit(Block& _block) override;
+	bool visit(ForStatement& _forLoop) override;
+	void endVisit(ForStatement& _forLoop) override;
 	void endVisit(VariableDeclarationStatement& _variableDeclarationStatement) override;
 	bool visit(VariableDeclaration& _declaration) override;
 	bool visit(EventDefinition& _event) override;
 	void endVisit(EventDefinition& _event) override;
 
-	void enterNewSubScope(Declaration const& _declaration);
+	void enterNewSubScope(ASTNode& _subScope);
 	void closeCurrentScope();
 	void registerDeclaration(Declaration& _declaration, bool _opensScope);
 
@@ -177,6 +190,7 @@ 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 0bb5e3fe..985c44d0 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/libsolidity/analysis/ReferencesResolver.cpp
@@ -43,6 +43,56 @@ bool ReferencesResolver::resolve(ASTNode const& _root)
 	return !m_errorOccurred;
 }
 
+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);
+	return true;
+}
+
+void ReferencesResolver::endVisit(Block const& _block)
+{
+	if (!m_resolveInsideCode)
+		return;
+
+	// C99-scoped variables
+	if (m_experimental050Mode)
+		m_resolver.setScope(_block.scope());
+}
+
+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);
+	return true;
+}
+
+void ReferencesResolver::endVisit(ForStatement const& _for)
+{
+	if (!m_resolveInsideCode)
+		return;
+	if (m_experimental050Mode)
+		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());
+}
+
 bool ReferencesResolver::visit(Identifier const& _identifier)
 {
 	auto declarations = m_resolver.nameFromCurrentScope(_identifier.name());
diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h
index fef2e73f..4e8f54b5 100644
--- a/libsolidity/analysis/ReferencesResolver.h
+++ b/libsolidity/analysis/ReferencesResolver.h
@@ -57,7 +57,11 @@ public:
 	bool resolve(ASTNode const& _root);
 
 private:
-	virtual bool visit(Block const&) override { return m_resolveInsideCode; }
+	virtual bool visit(Block const& _block) override;
+	virtual void endVisit(Block const& _block) override;
+	virtual bool visit(ForStatement const& _for) override;
+	virtual void endVisit(ForStatement const& _for) override;
+	virtual void endVisit(VariableDeclarationStatement const& _varDeclStatement) override;
 	virtual bool visit(Identifier const& _identifier) override;
 	virtual bool visit(ElementaryTypeName const& _typeName) override;
 	virtual bool visit(FunctionDefinition const& _functionDefinition) override;
@@ -90,6 +94,7 @@ private:
 	std::vector<ParameterList const*> m_returnParameters;
 	bool const m_resolveInsideCode;
 	bool m_errorOccurred = false;
+	bool m_experimental050Mode = false;
 };
 
 }
diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp
index 8da6964e..27220b1f 100644
--- a/libsolidity/ast/AST.cpp
+++ b/libsolidity/ast/AST.cpp
@@ -96,20 +96,6 @@ set<SourceUnit const*> SourceUnit::referencedSourceUnits(bool _recurse, set<Sour
 	return sourceUnits;
 }
 
-SourceUnit const& Declaration::sourceUnit() const
-{
-	solAssert(!!m_scope, "");
-	ASTNode const* scope = m_scope;
-	while (dynamic_cast<Declaration const*>(scope) && dynamic_cast<Declaration const*>(scope)->m_scope)
-		scope = dynamic_cast<Declaration const*>(scope)->m_scope;
-	return dynamic_cast<SourceUnit const&>(*scope);
-}
-
-string Declaration::sourceUnitName() const
-{
-	return sourceUnit().annotation().path;
-}
-
 ImportAnnotation& ImportDirective::annotation() const
 {
 	if (!m_annotation)
@@ -408,12 +394,36 @@ UserDefinedTypeNameAnnotation& UserDefinedTypeName::annotation() const
 	return dynamic_cast<UserDefinedTypeNameAnnotation&>(*m_annotation);
 }
 
+SourceUnit const& Scopable::sourceUnit() const
+{
+	ASTNode const* s = scope();
+	solAssert(s, "");
+	// will not always be a declaratoion
+	while (dynamic_cast<Scopable const*>(s) && dynamic_cast<Scopable const*>(s)->scope())
+		s = dynamic_cast<Scopable const*>(s)->scope();
+	return dynamic_cast<SourceUnit const&>(*s);
+}
+
+string Scopable::sourceUnitName() const
+{
+	return sourceUnit().annotation().path;
+}
+
 bool VariableDeclaration::isLValue() const
 {
 	// External function parameters and constant declared variables are Read-Only
 	return !isExternalCallableParameter() && !m_isConstant;
 }
 
+bool VariableDeclaration::isLocalVariable() const
+{
+	auto s = scope();
+	return
+		dynamic_cast<CallableDeclaration const*>(s) ||
+		dynamic_cast<Block const*>(s) ||
+		dynamic_cast<ForStatement const*>(s);
+}
+
 bool VariableDeclaration::isCallableParameter() const
 {
 	auto const* callable = dynamic_cast<CallableDeclaration const*>(scope());
@@ -459,8 +469,7 @@ bool VariableDeclaration::isExternalCallableParameter() const
 
 bool VariableDeclaration::canHaveAutoType() const
 {
-	auto const* callable = dynamic_cast<CallableDeclaration const*>(scope());
-	return (!!callable && !isCallableParameter());
+	return isLocalVariable() && !isCallableParameter();
 }
 
 TypePointer VariableDeclaration::type() const
diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h
index c0d55aec..863ad2fe 100644
--- a/libsolidity/ast/AST.h
+++ b/libsolidity/ast/AST.h
@@ -140,9 +140,32 @@ private:
 };
 
 /**
+ * Abstract class that is added to each AST node that is stored inside a scope
+ * (including scopes).
+ */
+class Scopable
+{
+public:
+	/// @returns the scope this declaration resides in. Can be nullptr if it is the global scope.
+	/// Available only after name and type resolution step.
+	ASTNode const* scope() const { return m_scope; }
+	void setScope(ASTNode const* _scope) { m_scope = _scope; }
+
+	/// @returns the source unit this scopable is present in.
+	SourceUnit const& sourceUnit() const;
+
+	/// @returns the source name this scopable is present in.
+	/// Can be combined with annotation().canonicalName (if present) to form a globally unique name.
+	std::string sourceUnitName() const;
+
+protected:
+	ASTNode const* m_scope = nullptr;
+};
+
+/**
  * Abstract AST class for a declaration (contract, function, struct, variable, import directive).
  */
-class Declaration: public ASTNode
+class Declaration: public ASTNode, public Scopable
 {
 public:
 	/// Visibility ordered from restricted to unrestricted.
@@ -171,7 +194,7 @@ public:
 		ASTPointer<ASTString> const& _name,
 		Visibility _visibility = Visibility::Default
 	):
-		ASTNode(_location), m_name(_name), m_visibility(_visibility), m_scope(nullptr) {}
+		ASTNode(_location), m_name(_name), m_visibility(_visibility) {}
 
 	/// @returns the declared name.
 	ASTString const& name() const { return *m_name; }
@@ -181,17 +204,6 @@ public:
 	virtual bool isVisibleInContract() const { return visibility() != Visibility::External; }
 	bool isVisibleInDerivedContracts() const { return isVisibleInContract() && visibility() >= Visibility::Internal; }
 
-	/// @returns the scope this declaration resides in. Can be nullptr if it is the global scope.
-	/// Available only after name and type resolution step.
-	ASTNode const* scope() const { return m_scope; }
-	void setScope(ASTNode const* _scope) { m_scope = _scope; }
-
-	/// @returns the source unit this declaration is present in.
-	SourceUnit const& sourceUnit() const;
-
-	/// @returns the source name this declaration is present in.
-	/// Can be combined with annotation().canonicalName to form a globally unique name.
-	std::string sourceUnitName() const;
 	std::string fullyQualifiedName() const { return sourceUnitName() + ":" + name(); }
 
 	virtual bool isLValue() const { return false; }
@@ -213,7 +225,6 @@ protected:
 private:
 	ASTPointer<ASTString> m_name;
 	Visibility m_visibility;
-	ASTNode const* m_scope;
 };
 
 /**
@@ -289,6 +300,8 @@ private:
 
 /**
  * Abstract class that is added to each AST node that can store local variables.
+ * Local variables in functions are always added to functions, even though they are not
+ * in scope for the whole function.
  */
 class VariableScope
 {
@@ -662,7 +675,7 @@ public:
 	virtual bool isLValue() const override;
 	virtual bool isPartOfExternalInterface() const override { return isPublic(); }
 
-	bool isLocalVariable() const { return !!dynamic_cast<CallableDeclaration const*>(scope()); }
+	bool isLocalVariable() const;
 	/// @returns true if this variable is a parameter or return parameter of a function.
 	bool isCallableParameter() const;
 	/// @returns true if this variable is a return parameter of a function.
@@ -1004,7 +1017,7 @@ private:
 /**
  * Brace-enclosed block containing zero or more statements.
  */
-class Block: public Statement
+class Block: public Statement, public Scopable
 {
 public:
 	Block(
@@ -1111,7 +1124,7 @@ private:
 /**
  * For loop statement
  */
-class ForStatement: public BreakableStatement
+class ForStatement: public BreakableStatement, public Scopable
 {
 public:
 	ForStatement(