Merge pull request #3839 from ethereum/unusedVariableWarningOrder

Static Analyzer: Fix non-deterministic order of unused variable warni…

3 files changed, 11 insertions, 8 deletions

diff --git a/Changelog.md b/Changelog.md
index d6860bdf..d1e199b7 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -27,6 +27,7 @@ Bugfixes:
  * Type System: Improve error message when attempting to shift by a fractional amount.
  * Type System: Make external library functions accessible.
  * Type System: Prevent encoding of weird types.
+ * Static Analyzer: Fix non-deterministic order of unused variable warnings.
 
 ### 0.4.21 (2018-03-07)
 
diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp
index d96f8748..33b0e296 100644
--- a/libsolidity/analysis/StaticAnalyzer.cpp
+++ b/libsolidity/analysis/StaticAnalyzer.cpp
@@ -78,13 +78,13 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&)
 	for (auto const& var: m_localVarUseCount)
 		if (var.second == 0)
 		{
-			if (var.first->isCallableParameter())
+			if (var.first.second->isCallableParameter())
 				m_errorReporter.warning(
-					var.first->location(),
+					var.first.second->location(),
 					"Unused function parameter. Remove or comment out the variable name to silence this warning."
 				);
 			else
-				m_errorReporter.warning(var.first->location(), "Unused local variable.");
+				m_errorReporter.warning(var.first.second->location(), "Unused local variable.");
 		}
 
 	m_localVarUseCount.clear();
@@ -97,7 +97,7 @@ bool StaticAnalyzer::visit(Identifier const& _identifier)
 		{
 			solAssert(!var->name().empty(), "");
 			if (var->isLocalVariable())
-				m_localVarUseCount[var] += 1;
+				m_localVarUseCount[make_pair(var->id(), var)] += 1;
 		}
 	return true;
 }
@@ -109,7 +109,7 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable)
 		solAssert(_variable.isLocalVariable(), "");
 		if (_variable.name() != "")
 			// This is not a no-op, the entry might pre-exist.
-			m_localVarUseCount[&_variable] += 0;
+			m_localVarUseCount[make_pair(_variable.id(), &_variable)] += 0;
 	}
 	else if (_variable.isStateVariable())
 	{
@@ -132,7 +132,7 @@ bool StaticAnalyzer::visit(Return const& _return)
 	if (m_currentFunction && _return.expression())
 		for (auto const& var: m_currentFunction->returnParameters())
 			if (!var->name().empty())
-				m_localVarUseCount[var.get()] += 1;
+				m_localVarUseCount[make_pair(var->id(), var.get())] += 1;
 	return true;
 }
 
@@ -224,7 +224,7 @@ bool StaticAnalyzer::visit(InlineAssembly const& _inlineAssembly)
 		{
 			solAssert(!var->name().empty(), "");
 			if (var->isLocalVariable())
-				m_localVarUseCount[var] += 1;
+				m_localVarUseCount[make_pair(var->id(), var)] += 1;
 		}
 	}
 
diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h
index 124c4e7c..0a806bbd 100644
--- a/libsolidity/analysis/StaticAnalyzer.h
+++ b/libsolidity/analysis/StaticAnalyzer.h
@@ -77,7 +77,9 @@ private:
 	bool m_nonPayablePublic = false;
 
 	/// Number of uses of each (named) local variable in a function, counter is initialized with zero.
-	std::map<VariableDeclaration const*, int> m_localVarUseCount;
+	/// Pairs of AST ids and pointers are used as keys to ensure a deterministic order
+	/// when traversing.
+	std::map<std::pair<size_t, VariableDeclaration const*>, int> m_localVarUseCount;
 
 	FunctionDefinition const* m_currentFunction = nullptr;