Only active variables at the point of their declaration.

8 files changed, 176 insertions, 33 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 40021771..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,9 +166,9 @@ vector<Declaration const*> NameAndTypeResolver::resolveName(ASTString const& _na
 	return iterator->second->resolveName(_name, false);
 }
 
-vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name) const
+vector<Declaration const*> NameAndTypeResolver::nameFromCurrentScope(ASTString const& _name, bool _includeInvisibles) const
 {
-	return m_currentScope->resolveName(_name, true);
+	return m_currentScope->resolveName(_name, true, _includeInvisibles);
 }
 
 Declaration const* NameAndTypeResolver::pathFromCurrentScope(vector<ASTString> const& _path) const
@@ -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, "");
@@ -439,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)
@@ -456,6 +465,7 @@ bool DeclarationRegistrationHelper::registerDeclaration(
 	string const* _name,
 	SourceLocation const* _errorLocation,
 	bool _warnOnShadow,
+	bool _inactive,
 	ErrorReporter& _errorReporter
 )
 {
@@ -465,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;
@@ -613,32 +626,28 @@ void DeclarationRegistrationHelper::endVisit(ModifierDefinition&)
 bool DeclarationRegistrationHelper::visit(Block& _block)
 {
 	_block.setScope(m_currentScope);
-	// Enable C99-scoped variables.
-	if (_block.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050))
+	if (m_useC99Scoping)
 		enterNewSubScope(_block);
 	return true;
 }
 
-void DeclarationRegistrationHelper::endVisit(Block& _block)
+void DeclarationRegistrationHelper::endVisit(Block&)
 {
-	// Enable C99-scoped variables.
-	if (_block.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050))
+	if (m_useC99Scoping)
 		closeCurrentScope();
 }
 
 bool DeclarationRegistrationHelper::visit(ForStatement& _for)
 {
 	_for.setScope(m_currentScope);
-	if (_for.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050))
-		// TODO special scoping rules for the init statement - if it is a block, then it should
-		// not open its own scope.
+	if (m_useC99Scoping)
 		enterNewSubScope(_for);
 	return true;
 }
 
-void DeclarationRegistrationHelper::endVisit(ForStatement& _for)
+void DeclarationRegistrationHelper::endVisit(ForStatement&)
 {
-	if (_for.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050))
+	if (m_useC99Scoping)
 		closeCurrentScope();
 }
 
@@ -704,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 7a6e813e..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,6 +69,9 @@ 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).
@@ -78,7 +81,7 @@ public:
 
 	/// 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) const;
+	std::vector<Declaration const*> nameFromCurrentScope(ASTString const& _name, bool _includeInvisibles = false) const;
 
 	/// Resolves a path starting from the "current" scope, but also searches parent scopes.
 	/// Should only be called during the initial resolving phase.
@@ -139,6 +142,7 @@ public:
 	DeclarationRegistrationHelper(
 		std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes,
 		ASTNode& _astRoot,
+		bool _useC99Scoping,
 		ErrorReporter& _errorReporter,
 		ASTNode const* _currentScope = nullptr
 	);
@@ -149,6 +153,7 @@ public:
 		std::string const* _name,
 		SourceLocation const* _errorLocation,
 		bool _warnOnShadow,
+		bool _inactive,
 		ErrorReporter& _errorReporter
 	);
 
@@ -185,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 4d919f2b..985c44d0 100644
--- a/libsolidity/analysis/ReferencesResolver.cpp
+++ b/libsolidity/analysis/ReferencesResolver.cpp
@@ -83,6 +83,16 @@ void ReferencesResolver::endVisit(ForStatement const& _for)
 		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 ab7c987e..4e8f54b5 100644
--- a/libsolidity/analysis/ReferencesResolver.h
+++ b/libsolidity/analysis/ReferencesResolver.h
@@ -61,6 +61,7 @@ private:
 	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;
diff --git a/test/libsolidity/SolidityEndToEndTest.cpp b/test/libsolidity/SolidityEndToEndTest.cpp
index 3882e4ea..7bae3cd6 100644
--- a/test/libsolidity/SolidityEndToEndTest.cpp
+++ b/test/libsolidity/SolidityEndToEndTest.cpp
@@ -284,6 +284,26 @@ BOOST_AUTO_TEST_CASE(conditional_expression_functions)
 	ABI_CHECK(callContractFunction("f(bool)", false), encodeArgs(u256(2)));
 }
 
+BOOST_AUTO_TEST_CASE(C99_scoping_activation)
+{
+	char const* sourceCode = R"(
+		pragma experimental "v0.5.0";
+		contract test {
+			function f() pure public returns (uint) {
+				uint x = 7;
+				{
+					x = 3; // This should still assign to the outer variable
+					uint x;
+					x = 4; // This should assign to the new one
+				}
+				return x;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	ABI_CHECK(callContractFunction("f()"), encodeArgs(3));
+}
+
 BOOST_AUTO_TEST_CASE(recursive_calls)
 {
 	char const* sourceCode = R"(
diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp
index 8586ebf8..e5bd0103 100644
--- a/test/libsolidity/SolidityNameAndTypeResolution.cpp
+++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp
@@ -135,19 +135,59 @@ BOOST_AUTO_TEST_CASE(scoping)
 	CHECK_ERROR(text, DeclarationError, "Undeclared identifier");
 }
 
-BOOST_AUTO_TEST_CASE(scoping_for)
+BOOST_AUTO_TEST_CASE(scoping_activation_old)
+{
+	char const* text = R"(
+		contract test {
+			function f() pure public {
+				x = 3;
+				uint x;
+			}
+		}
+	)";
+	CHECK_SUCCESS_NO_WARNINGS(text);
+}
+
+BOOST_AUTO_TEST_CASE(scoping_activation)
+{
+	char const* text = R"(
+		pragma experimental "v0.5.0";
+		contract test {
+			function f() pure public {
+				x = 3;
+				uint x;
+			}
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Undeclared identifier");
+}
+
+BOOST_AUTO_TEST_CASE(scoping_self_use)
 {
 	char const* text = R"(
 		pragma experimental "v0.5.0";
 		contract test {
 			function f() public {
+				uint a = a;
+			}
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Undeclared identifier");
+}
+
+BOOST_AUTO_TEST_CASE(scoping_for)
+{
+	char const* text = R"(
+		pragma experimental "v0.5.0";
+		contract test {
+			function f() pure public {
 				for (uint x = 0; x < 10; x ++){
 					x = 2;
 				}
 			}
 		}
 	)";
-	CHECK_SUCCESS(text);
+	CHECK_WARNING(text, "Experimental features");
 }
 
 BOOST_AUTO_TEST_CASE(scoping_for2)
@@ -155,7 +195,21 @@ BOOST_AUTO_TEST_CASE(scoping_for2)
 	char const* text = R"(
 		pragma experimental "v0.5.0";
 		contract test {
-			function f() public {
+			function f() pure public {
+				for (uint x = 0; x < 10; x ++)
+					x = 2;
+			}
+		}
+	)";
+	CHECK_WARNING(text, "Experimental features");
+}
+
+BOOST_AUTO_TEST_CASE(scoping_for3)
+{
+	char const* text = R"(
+		pragma experimental "v0.5.0";
+		contract test {
+			function f() pure public {
 				for (uint x = 0; x < 10; x ++){
 					x = 2;
 				}
@@ -166,6 +220,21 @@ BOOST_AUTO_TEST_CASE(scoping_for2)
 	CHECK_ERROR(text, DeclarationError, "Undeclared identifier");
 }
 
+BOOST_AUTO_TEST_CASE(scoping_for_decl_in_body)
+{
+	char const* text = R"(
+		pragma experimental "v0.5.0";
+		contract test {
+			function f() pure public {
+				for (;; y++){
+					uint y = 3;
+				}
+			}
+		}
+	)";
+	CHECK_ERROR(text, DeclarationError, "Undeclared identifier");
+}
+
 BOOST_AUTO_TEST_CASE(name_shadowing)
 {
 	char const* text = R"(