Merge pull request #2384 from ethereum/parseFunctionalInstructions

Enforce function arguments when parsing functional instructions.

Changelog.md

@@ -10,6 +10,7 @@ Features:
 
 Bugfixes:
  * Unused variable warnings no longer issued for variables used inside inline assembly.
+ * Inline Assembly: Enforce function arguments when parsing functional instructions.
 
 ### 0.4.11 (2017-05-03)
 

docs/assembly.rst

@@ -318,8 +318,10 @@ would be written as follows
 
     mstore(0x80, add(mload(0x80), 3))
 
-Functional style and instructional style can be mixed, but any opcode inside a
+Functional style expressions cannot use instructional style internally, i.e.
-functional style expression has to return exactly one stack slot (most of the opcodes do).
+``1 2 mstore(0x80, add)`` is not valid assembly, it has to be written as
+``mstore(0x80, add(2, 1))``. For opcodes that do not take arguments, the
+parentheses can be omitted.
 
 Note that the order of arguments is reversed in functional-style as opposed to the instruction-style
 way. If you use functional-style, the first argument will end up on the stack top.

libsolidity/inlineasm/AsmParser.cpp

@@ -174,6 +174,19 @@ assembly::Case Parser::parseCase()
 assembly::Statement Parser::parseExpression()
 {
 	Statement operation = parseElementaryOperation(true);
+	if (operation.type() == typeid(Instruction))
+	{
+		Instruction const& instr = boost::get<Instruction>(operation);
+		int args = instructionInfo(instr.instruction).args;
+		if (args > 0 && currentToken() != Token::LParen)
+			fatalParserError(string(
+				"Expected token \"(\" (\"" +
+				instructionNames().at(instr.instruction) +
+				"\" expects " +
+				boost::lexical_cast<string>(args) +
+				" arguments)"
+			));
+	}
 	if (currentToken() == Token::LParen)
 		return parseCall(std::move(operation));
 	else

test/libsolidity/InlineAssembly.cpp

@@ -213,6 +213,16 @@ BOOST_AUTO_TEST_CASE(functional)
 	BOOST_CHECK(successParse("{ let x := 2 add(7, mul(6, x)) mul(7, 8) add =: x }"));
 }
 
+BOOST_AUTO_TEST_CASE(functional_partial)
+{
+	CHECK_PARSE_ERROR("{ let x := byte }", ParserError, "Expected token \"(\"");
+}
+
+BOOST_AUTO_TEST_CASE(functional_partial_success)
+{
+	BOOST_CHECK(successParse("{ let x := byte(1, 2) }"));
+}
+
 BOOST_AUTO_TEST_CASE(functional_assignment)
 {
 	BOOST_CHECK(successParse("{ let x := 2 x := 7 }"));
@@ -258,7 +268,7 @@ BOOST_AUTO_TEST_CASE(switch_duplicate_case)
 BOOST_AUTO_TEST_CASE(switch_invalid_expression)
 {
 	CHECK_PARSE_ERROR("{ switch {} default {} }", ParserError, "Literal, identifier or instruction expected.");
-	CHECK_PARSE_ERROR("{ 1 2 switch mul default {} }", ParserError, "Instructions are not supported as expressions for switch.");
+	CHECK_PARSE_ERROR("{ switch calldatasize default {} }", ParserError, "Instructions are not supported as expressions for switch.");
 }
 
 BOOST_AUTO_TEST_CASE(switch_default_before_case)