Merge pull request #5582 from ethereum/yulErrorMessages

Improve error messages around invalid function argument count.

5 files changed, 26 insertions, 16 deletions

diff --git a/Changelog.md b/Changelog.md
index 7bdcd518..2acd60c8 100644
--- a/Changelog.md
+++ b/Changelog.md
@@ -4,6 +4,7 @@ Language Features:
 
 
 Compiler Features:
+ * Inline Assembly: Improve error messages around invalid function argument count.
 
 
 Bugfixes:
diff --git a/libyul/AsmAnalysis.cpp b/libyul/AsmAnalysis.cpp
index d3f6de84..d2efdd9f 100644
--- a/libyul/AsmAnalysis.cpp
+++ b/libyul/AsmAnalysis.cpp
@@ -244,9 +244,18 @@ bool AsmAnalyzer::operator()(VariableDeclaration const& _varDecl)
 	{
 		int const stackHeight = m_stackHeight;
 		success = boost::apply_visitor(*this, *_varDecl.value);
-		if ((m_stackHeight - stackHeight) != numVariables)
+		int numValues = m_stackHeight - stackHeight;
+		if (numValues != numVariables)
 		{
-			m_errorReporter.declarationError(_varDecl.location, "Variable count mismatch.");
+			m_errorReporter.declarationError(_varDecl.location,
+				"Variable count mismatch: " +
+				to_string(numVariables) +
+				" variables and " +
+				to_string(numValues) +
+				" values."
+			);
+			// Adjust stack height to avoid misleading additional errors.
+			m_stackHeight += numVariables - numValues;
 			return false;
 		}
 	}
@@ -288,7 +297,7 @@ bool AsmAnalyzer::operator()(FunctionCall const& _funCall)
 {
 	solAssert(!_funCall.functionName.name.empty(), "");
 	bool success = true;
-	size_t arguments = 0;
+	size_t parameters = 0;
 	size_t returns = 0;
 	if (!m_currentScope->lookup(_funCall.functionName.name, Scope::Visitor(
 		[&](Scope::Variable const&)
@@ -310,7 +319,7 @@ bool AsmAnalyzer::operator()(FunctionCall const& _funCall)
 		[&](Scope::Function const& _fun)
 		{
 			/// TODO: compare types too
-			arguments = _fun.arguments.size();
+			parameters = _fun.arguments.size();
 			returns = _fun.returns.size();
 		}
 	)))
@@ -319,21 +328,23 @@ bool AsmAnalyzer::operator()(FunctionCall const& _funCall)
 		success = false;
 	}
 	if (success)
-	{
-		if (_funCall.arguments.size() != arguments)
+		if (_funCall.arguments.size() != parameters)
 		{
 			m_errorReporter.typeError(
 				_funCall.functionName.location,
-				"Expected " + to_string(arguments) + " arguments but got " +
+				"Function expects " +
+				to_string(parameters) +
+				" arguments but got " +
 				to_string(_funCall.arguments.size()) + "."
 			);
 			success = false;
 		}
-	}
+
 	for (auto const& arg: _funCall.arguments | boost::adaptors::reversed)
 		if (!expectExpression(arg))
 			success = false;
-	m_stackHeight += int(returns) - int(arguments);
+	// Use argument size instead of parameter count to avoid misleading errors.
+	m_stackHeight += int(returns) - int(_funCall.arguments.size());
 	m_info.stackHeightInfo[&_funCall] = m_stackHeight;
 	return success;
 }
diff --git a/test/libsolidity/InlineAssembly.cpp b/test/libsolidity/InlineAssembly.cpp
index 11d4c59f..b6986041 100644
--- a/test/libsolidity/InlineAssembly.cpp
+++ b/test/libsolidity/InlineAssembly.cpp
@@ -433,8 +433,7 @@ BOOST_AUTO_TEST_CASE(variable_access_cross_functions)
 
 BOOST_AUTO_TEST_CASE(invalid_tuple_assignment)
 {
-	/// The push(42) is added here to silence the unbalanced stack error, so that there's only one error reported.
-	CHECK_PARSE_ERROR("{ 42 let x, y := 1 }", DeclarationError, "Variable count mismatch.");
+	CHECK_PARSE_ERROR("{ let x, y := 1 }", DeclarationError, "Variable count mismatch: 2 variables and 1 values");
 }
 
 BOOST_AUTO_TEST_CASE(instruction_too_few_arguments)
diff --git a/test/libsolidity/syntaxTests/inlineAssembly/function_call_invalid_argument_count.sol b/test/libsolidity/syntaxTests/inlineAssembly/function_call_invalid_argument_count.sol
index ac1f541e..2d36bedd 100644
--- a/test/libsolidity/syntaxTests/inlineAssembly/function_call_invalid_argument_count.sol
+++ b/test/libsolidity/syntaxTests/inlineAssembly/function_call_invalid_argument_count.sol
@@ -10,7 +10,5 @@ contract C {
   }
 }
 // ----
-// TypeError: (87-88): Expected 1 arguments but got 0.
-// SyntaxError: (87-90): Top-level expressions are not supposed to return values (this expression returns -1 values). Use ``pop()`` or assign them.
-// TypeError: (108-109): Expected 1 arguments but got 2.
-// SyntaxError: (108-115): Top-level expressions are not supposed to return values (this expression returns 1 value). Use ``pop()`` or assign them.
+// TypeError: (87-88): Function expects 1 arguments but got 0.
+// TypeError: (108-109): Function expects 1 arguments but got 2.
diff --git a/test/libyul/Parser.cpp b/test/libyul/Parser.cpp
index caaf2719..2d9c8904 100644
--- a/test/libyul/Parser.cpp
+++ b/test/libyul/Parser.cpp
@@ -78,7 +78,8 @@ boost::optional<Error> parseAndReturnFirstError(string const& _source, bool _all
 	ErrorReporter errorReporter(errors);
 	if (!parse(_source, errorReporter))
 	{
-		BOOST_REQUIRE_EQUAL(errors.size(), 1);
+		BOOST_REQUIRE(!errors.empty());
+		BOOST_CHECK_EQUAL(errors.size(), 1);
 		return *errors.front();
 	}
 	else