No write access to parameters of external functions.

libsolidity/AST.cpp

@@ -288,12 +288,23 @@ string FunctionDefinition::getCanonicalSignature() const
 	return FunctionType(*this).getCanonicalSignature(getName());
 }
 
-Declaration::LValueType VariableDeclaration::getLValueType() const
+bool VariableDeclaration::isLValue() const
 {
-	if (dynamic_cast<FunctionDefinition const*>(getScope()) || dynamic_cast<ModifierDefinition const*>(getScope()))
+	if (auto const* function = dynamic_cast<FunctionDefinition const*>(getScope()))
-		return Declaration::LValueType::Local;
+		if (function->getVisibility() == Declaration::Visibility::External && isFunctionParameter())
-	else
+			return false;
-		return Declaration::LValueType::Storage;
+	return true;
+}
+
+bool VariableDeclaration::isFunctionParameter() const
+{
+	auto const* function = dynamic_cast<FunctionDefinition const*>(getScope());
+	if (!function)
+		return false;
+	for (auto const& variable: function->getParameters())
+		if (variable.get() == this)
+			return true;
+	return false;
 }
 
 TypePointer ModifierDefinition::getType(ContractDefinition const*) const
@@ -586,8 +597,7 @@ void MemberAccess::checkTypeRequirements()
 	if (!m_type)
 		BOOST_THROW_EXCEPTION(createTypeError("Member \"" + *m_memberName + "\" not found or not "
 											  "visible in " + type.toString()));
-	//@todo later, this will not always be STORAGE
+	m_isLValue = (type.getCategory() == Type::Category::Struct);
-	m_lvalue = type.getCategory() == Type::Category::Struct ? Declaration::LValueType::Storage : Declaration::LValueType::None;
 }
 
 void IndexAccess::checkTypeRequirements()
@@ -599,14 +609,14 @@ void IndexAccess::checkTypeRequirements()
 	MappingType const& type = dynamic_cast<MappingType const&>(*m_base->getType());
 	m_index->expectType(*type.getKeyType());
 	m_type = type.getValueType();
-	m_lvalue = Declaration::LValueType::Storage;
+	m_isLValue = true;
 }
 
 void Identifier::checkTypeRequirements()
 {
 	solAssert(m_referencedDeclaration, "Identifier not resolved.");
 
-	m_lvalue = m_referencedDeclaration->getLValueType();
+	m_isLValue = m_referencedDeclaration->isLValue();
 	m_type = m_referencedDeclaration->getType(m_currentContract);
 	if (!m_type)
 		BOOST_THROW_EXCEPTION(createTypeError("Declaration referenced before type could be determined."));

libsolidity/AST.h

@@ -132,7 +132,6 @@ private:
 class Declaration: public ASTNode
 {
 public:
-	enum class LValueType { None, Local, Storage };
 	/// Visibility ordered from restricted to unrestricted.
 	enum class Visibility { Default, Private, Protected, Public, External };
 
@@ -156,8 +155,7 @@ public:
 	/// The current contract has to be given since this context can change the type, especially of
 	/// contract types.
 	virtual TypePointer getType(ContractDefinition const* m_currentContract = nullptr) const = 0;
-	/// @returns the lvalue type of expressions referencing this declaration
+	virtual bool isLValue() const { return false; }
-	virtual LValueType getLValueType() const { return LValueType::None; }
 
 protected:
 	virtual Visibility getDefaultVisibility() const { return Visibility::Public; }
@@ -448,8 +446,9 @@ public:
 	TypePointer getType(ContractDefinition const* = nullptr) const { return m_type; }
 	void setType(std::shared_ptr<Type const> const& _type) { m_type = _type; }
 
-	virtual LValueType getLValueType() const override;
+	virtual bool isLValue() const override;
 	bool isLocalVariable() const { return !!dynamic_cast<FunctionDefinition const*>(getScope()); }
+	bool isFunctionParameter() const;
 	bool isStateVariable() const { return m_isStateVariable; }
 	bool isIndexed() const { return m_isIndexed; }
 
@@ -887,8 +886,7 @@ public:
 	virtual void checkTypeRequirements() = 0;
 
 	std::shared_ptr<Type const> const& getType() const { return m_type; }
-	bool isLValue() const { return m_lvalue != Declaration::LValueType::None; }
+	bool isLValue() const { return m_isLValue; }
-	bool isLocalLValue() const { return m_lvalue == Declaration::LValueType::Local; }
 
 	/// Helper function, infer the type via @ref checkTypeRequirements and then check that it
 	/// is implicitly convertible to @a _expectedType. If not, throw exception.
@@ -903,9 +901,9 @@ public:
 protected:
 	//! Inferred type of the expression, only filled after a call to checkTypeRequirements().
 	std::shared_ptr<Type const> m_type;
-	//! If this expression is an lvalue (i.e. something that can be assigned to) and is stored
+	//! If this expression is an lvalue (i.e. something that can be assigned to).
-	//! locally or in storage. This is set during calls to @a checkTypeRequirements()
+	//! This is set during calls to @a checkTypeRequirements()
-	Declaration::LValueType m_lvalue = Declaration::LValueType::None;
+	bool m_isLValue = false;
 	//! Whether the outer expression requested the address (true) or the value (false) of this expression.
 	bool m_lvalueRequested = false;
 };

libsolidity/ASTJsonConverter.cpp

@@ -118,11 +118,7 @@ bool ASTJsonConverter::visit(FunctionDefinition const& _node)
 
 bool ASTJsonConverter::visit(VariableDeclaration const& _node)
 {
-	bool isLocalVariable = (_node.getLValueType() == VariableDeclaration::LValueType::Local);
+	addJsonNode("VariableDeclaration", { make_pair("name", _node.getName()) }, true);
-	addJsonNode("VariableDeclaration",
-				{   make_pair("name", _node.getName()),
-					make_pair("local", boost::lexical_cast<std::string>(isLocalVariable))},
-				true);
 	return true;
 }
 
@@ -216,11 +212,12 @@ bool ASTJsonConverter::visit(ExpressionStatement const&)
 
 bool ASTJsonConverter::visit(Expression const& _node)
 {
-	addJsonNode("Expression",
+	addJsonNode(
-				{ make_pair("type", getType(_node)),
+		"Expression",
-					make_pair("lvalue", boost::lexical_cast<std::string>(_node.isLValue())),
+		{ make_pair("type", getType(_node)),
-					make_pair("local_lvalue", boost::lexical_cast<std::string>(_node.isLocalLValue())) },
+			make_pair("lvalue", boost::lexical_cast<std::string>(_node.isLValue())) },
-				true);
+		true
+	);
 	return true;
 }
 

libsolidity/Compiler.cpp

@@ -255,12 +255,22 @@ bool Compiler::visit(FunctionDefinition const& _function)
 	// stack upon entry: [return address] [arg0] [arg1] ... [argn]
 	// reserve additional slots: [retarg0] ... [retargm] [localvar0] ... [localvarp]
 
-	unsigned parametersSize = CompilerUtils::getSizeOnStack(_function.getParameters());
+	if (_function.getVisibility() != Declaration::Visibility::External)
-	m_context.adjustStackOffset(parametersSize);
-	for (ASTPointer<VariableDeclaration const> const& variable: _function.getParameters())
 	{
-		m_context.addVariable(*variable, parametersSize);
+		unsigned parametersSize = CompilerUtils::getSizeOnStack(_function.getParameters());
-		parametersSize -= variable->getType()->getSizeOnStack();
+		m_context.adjustStackOffset(parametersSize);
+		for (ASTPointer<VariableDeclaration const> const& variable: _function.getParameters())
+		{
+			m_context.addVariable(*variable, parametersSize);
+			parametersSize -= variable->getType()->getSizeOnStack();
+		}
+	}
+	else
+	{
+		unsigned calldataPos = CompilerUtils::dataStartOffset;
+		// calldatapos is _always_ dynamic.
+		for (ASTPointer<VariableDeclaration const> const& variable: _function.getParameters())
+			m_context.addCalldataVariable(*variable, calldataPos);
 	}
 	for (ASTPointer<VariableDeclaration const> const& variable: _function.getReturnParameters())
 		m_context.addAndInitializeVariable(*variable);
@@ -277,7 +287,8 @@ bool Compiler::visit(FunctionDefinition const& _function)
 	// Note that the fact that the return arguments are of increasing index is vital for this
 	// algorithm to work.
 
-	unsigned const c_argumentsSize = CompilerUtils::getSizeOnStack(_function.getParameters());
+	unsigned const c_argumentsSize = (_function.getVisibility() == Declaration::Visibility::External
+		? 0 : CompilerUtils::getSizeOnStack(_function.getParameters()));
 	unsigned const c_returnValuesSize = CompilerUtils::getSizeOnStack(_function.getReturnParameters());
 	unsigned const c_localVariablesSize = CompilerUtils::getSizeOnStack(_function.getLocalVariables());
 

test/SolidityEndToEndTest.cpp

@@ -2534,6 +2534,21 @@ BOOST_AUTO_TEST_CASE(constructing_enums_from_ints)
 	BOOST_CHECK(callContractFunction("test()") == encodeArgs(1));
 }
 
+BOOST_AUTO_TEST_CASE(external_function)
+{
+	char const* sourceCode = R"(
+		contract c {
+			function f(uint a) returns (uint) { return a; }
+			function test(uint a, uint b) external returns (uint r_a, uint r_b) {
+				r_a = f(a + 7);
+				r_b = b;
+			}
+		}
+	)";
+	compileAndRun(sourceCode);
+	BOOST_CHECK(callContractFunction("test(uint256,uint256)", 2, 3) == encodeArgs(2, 3+7));
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }

test/SolidityNameAndTypeResolution.cpp

@@ -1133,6 +1133,36 @@ BOOST_AUTO_TEST_CASE(external_base_visibility)
 	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError);
 }
 
+BOOST_AUTO_TEST_CASE(external_argument_assign)
+{
+	char const* sourceCode = R"(
+		contract c {
+			function f(uint a) external { a = 1; }
+		}
+		)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError);
+}
+
+BOOST_AUTO_TEST_CASE(external_argument_increment)
+{
+	char const* sourceCode = R"(
+		contract c {
+			function f(uint a) external { a++; }
+		}
+		)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError);
+}
+
+BOOST_AUTO_TEST_CASE(external_argument_delete)
+{
+	char const* sourceCode = R"(
+		contract c {
+			function f(uint a) external { delete a; }
+		}
+		)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }