Merge pull request #1072 from chriseth/sol_external

bytes ABI type and external visibility specifier

15 files changed, 222 insertions, 117 deletions

diff --git a/AST.cpp b/AST.cpp
index 0dbad433..c6d8f5c5 100644
--- a/AST.cpp
+++ b/AST.cpp
@@ -274,6 +274,15 @@ TypePointer FunctionDefinition::getType(ContractDefinition const*) const
 
 void FunctionDefinition::checkTypeRequirements()
 {
+	// change all byte arrays parameters to point to calldata
+	if (getVisibility() == Visibility::External)
+		for (ASTPointer<VariableDeclaration> const& var: getParameters())
+		{
+			auto const& type = var->getType();
+			solAssert(!!type, "");
+			if (auto const* byteArrayType = dynamic_cast<ByteArrayType const*>(type.get()))
+				var->setType(byteArrayType->copyForLocation(ByteArrayType::Location::CallData));
+		}
 	for (ASTPointer<VariableDeclaration> const& var: getParameters() + getReturnParameters())
 		if (!var->getType()->canLiveOutsideStorage())
 			BOOST_THROW_EXCEPTION(var->createTypeError("Type is required to live outside storage."));
@@ -288,12 +297,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()))
-		return Declaration::LValueType::Local;
-	else
-		return Declaration::LValueType::Storage;
+	if (auto const* function = dynamic_cast<FunctionDefinition const*>(getScope()))
+		if (function->getVisibility() == Declaration::Visibility::External && isFunctionParameter())
+			return false;
+	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 +606,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_lvalue = type.getCategory() == Type::Category::Struct ? Declaration::LValueType::Storage : Declaration::LValueType::None;
+	m_isLValue = (type.getCategory() == Type::Category::Struct);
 }
 
 void IndexAccess::checkTypeRequirements()
@@ -599,14 +618,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."));
diff --git a/AST.h b/AST.h
index 51d8031a..af45934f 100644
--- a/AST.h
+++ b/AST.h
@@ -132,8 +132,8 @@ private:
 class Declaration: public ASTNode
 {
 public:
-	enum class LValueType { None, Local, Storage };
-	enum class Visibility { Default, Public, Protected, Private };
+	/// Visibility ordered from restricted to unrestricted.
+	enum class Visibility { Default, Private, Protected, Public, External };
 
 	Declaration(Location const& _location, ASTPointer<ASTString> const& _name,
 				Visibility _visibility = Visibility::Default):
@@ -142,7 +142,9 @@ public:
 	/// @returns the declared name.
 	ASTString const& getName() const { return *m_name; }
 	Visibility getVisibility() const { return m_visibility == Visibility::Default ? getDefaultVisibility() : m_visibility; }
-	bool isPublic() const { return getVisibility() == Visibility::Public; }
+	bool isPublic() const { return getVisibility() >= Visibility::Public; }
+	bool isVisibleInContract() const { return getVisibility() != Visibility::External; }
+	bool isVisibleInDerivedContracts() const { return isVisibleInContract() && getVisibility() >= Visibility::Protected; }
 
 	/// @returns the scope this declaration resides in. Can be nullptr if it is the global scope.
 	/// Available only after name and type resolution step.
@@ -153,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 LValueType getLValueType() const { return LValueType::None; }
+	virtual bool isLValue() const { return false; }
 
 protected:
 	virtual Visibility getDefaultVisibility() const { return Visibility::Public; }
@@ -445,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; }
 
@@ -884,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 isLocalLValue() const { return m_lvalue == Declaration::LValueType::Local; }
+	bool isLValue() const { return m_isLValue; }
 
 	/// Helper function, infer the type via @ref checkTypeRequirements and then check that it
 	/// is implicitly convertible to @a _expectedType. If not, throw exception.
@@ -900,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
-	//! locally or in storage. This is set during calls to @a checkTypeRequirements()
-	Declaration::LValueType m_lvalue = Declaration::LValueType::None;
+	//! If this expression is an lvalue (i.e. something that can be assigned to).
+	//! This is set during calls to @a checkTypeRequirements()
+	bool m_isLValue = false;
 	//! Whether the outer expression requested the address (true) or the value (false) of this expression.
 	bool m_lvalueRequested = false;
 };
diff --git a/ASTJsonConverter.cpp b/ASTJsonConverter.cpp
index a216a59a..04feafe2 100644
--- a/ASTJsonConverter.cpp
+++ b/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()),
-					make_pair("local", boost::lexical_cast<std::string>(isLocalVariable))},
-				true);
+	addJsonNode("VariableDeclaration", { make_pair("name", _node.getName()) }, true);
 	return true;
 }
 
@@ -216,11 +212,12 @@ bool ASTJsonConverter::visit(ExpressionStatement const&)
 
 bool ASTJsonConverter::visit(Expression const& _node)
 {
-	addJsonNode("Expression",
-				{ make_pair("type", getType(_node)),
-					make_pair("lvalue", boost::lexical_cast<std::string>(_node.isLValue())),
-					make_pair("local_lvalue", boost::lexical_cast<std::string>(_node.isLocalLValue())) },
-				true);
+	addJsonNode(
+		"Expression",
+		{ make_pair("type", getType(_node)),
+			make_pair("lvalue", boost::lexical_cast<std::string>(_node.isLValue())) },
+		true
+	);
 	return true;
 }
 
diff --git a/Compiler.cpp b/Compiler.cpp
index dad79bb0..14acc011 100644
--- a/Compiler.cpp
+++ b/Compiler.cpp
@@ -147,7 +147,7 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 
 	// retrieve the function signature hash from the calldata
 	if (!interfaceFunctions.empty())
-		CompilerUtils(m_context).loadFromMemory(0, 4, false, true);
+		CompilerUtils(m_context).loadFromMemory(0, IntegerType(CompilerUtils::dataStartOffset * 8), true);
 
 	// stack now is: 1 0 <funhash>
 	for (auto const& it: interfaceFunctions)
@@ -178,23 +178,46 @@ void Compiler::appendFunctionSelector(ContractDefinition const& _contract)
 	}
 }
 
-unsigned Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory)
+void Compiler::appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory)
 {
 	// We do not check the calldata size, everything is zero-padded.
-	unsigned dataOffset = CompilerUtils::dataStartOffset; // the 4 bytes of the function hash signature
-	//@todo this can be done more efficiently, saving some CALLDATALOAD calls
+	unsigned offset(CompilerUtils::dataStartOffset);
+	bool const c_padToWords = true;
+
+	unsigned dynamicParameterCount = 0;
 	for (TypePointer const& type: _typeParameters)
-	{
-		unsigned const c_numBytes = type->getCalldataEncodedSize();
-		if (c_numBytes > 32)
-			BOOST_THROW_EXCEPTION(CompilerError()
-								  << errinfo_comment("Type " + type->toString() + " not yet supported."));
-		bool const c_leftAligned = type->getCategory() == Type::Category::String;
-		bool const c_padToWords = true;
-		dataOffset += CompilerUtils(m_context).loadFromMemory(dataOffset, c_numBytes, c_leftAligned,
-															  !_fromMemory, c_padToWords);
-	}
-	return dataOffset;
+		if (type->isDynamicallySized())
+			dynamicParameterCount++;
+	offset += dynamicParameterCount * 32;
+	unsigned currentDynamicParameter = 0;
+	for (TypePointer const& type: _typeParameters)
+		if (type->isDynamicallySized())
+		{
+			// value on stack: [calldata_offset] (only if we are already in dynamic mode)
+			if (currentDynamicParameter == 0)
+				// switch from static to dynamic
+				m_context << u256(offset);
+			// retrieve length
+			CompilerUtils(m_context).loadFromMemory(
+				CompilerUtils::dataStartOffset + currentDynamicParameter * 32,
+				IntegerType(256), !_fromMemory, c_padToWords);
+			// stack: offset length
+			// add 32-byte padding to copy of length
+			m_context << u256(32) << eth::Instruction::DUP1 << u256(31)
+				<< eth::Instruction::DUP4 << eth::Instruction::ADD
+				<< eth::Instruction::DIV << eth::Instruction::MUL;
+			// stack: offset length padded_length
+			m_context << eth::Instruction::DUP3 << eth::Instruction::ADD;
+			currentDynamicParameter++;
+			// stack: offset length next_calldata_offset
+		}
+		else if (currentDynamicParameter == 0)
+			// we can still use static load
+			offset += CompilerUtils(m_context).loadFromMemory(offset, *type, !_fromMemory, c_padToWords);
+		else
+			CompilerUtils(m_context).loadFromMemoryDynamic(*type, !_fromMemory, c_padToWords);
+	if (dynamicParameterCount > 0)
+		m_context << eth::Instruction::POP;
 }
 
 void Compiler::appendReturnValuePacker(TypePointers const& _typeParameters)
diff --git a/Compiler.h b/Compiler.h
index b3eae5b1..5d5b6d47 100644
--- a/Compiler.h
+++ b/Compiler.h
@@ -52,8 +52,8 @@ private:
 	void appendConstructorCall(FunctionDefinition const& _constructor);
 	void appendFunctionSelector(ContractDefinition const& _contract);
 	/// Creates code that unpacks the arguments for the given function represented by a vector of TypePointers.
-	/// From memory if @a _fromMemory is true, otherwise from call data. @returns the size of the data in bytes.
-	unsigned appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory = false);
+	/// From memory if @a _fromMemory is true, otherwise from call data.
+	void appendCalldataUnpacker(TypePointers const& _typeParameters, bool _fromMemory = false);
 	void appendReturnValuePacker(TypePointers const& _typeParameters);
 
 	void registerStateVariables(ContractDefinition const& _contract);
diff --git a/CompilerUtils.cpp b/CompilerUtils.cpp
index dda1736d..047bc6d6 100644
--- a/CompilerUtils.cpp
+++ b/CompilerUtils.cpp
@@ -33,35 +33,26 @@ namespace solidity
 
 const unsigned int CompilerUtils::dataStartOffset = 4;
 
-unsigned CompilerUtils::loadFromMemory(unsigned _offset, unsigned _bytes, bool _leftAligned,
-									   bool _fromCalldata, bool _padToWordBoundaries)
+unsigned CompilerUtils::loadFromMemory(unsigned _offset, Type const& _type,
+	bool _fromCalldata, bool _padToWordBoundaries)
 {
-	if (_bytes == 0)
-	{
-		m_context << u256(0);
-		return 0;
-	}
-	eth::Instruction load = _fromCalldata ? eth::Instruction::CALLDATALOAD : eth::Instruction::MLOAD;
-	solAssert(_bytes <= 32, "Memory load of more than 32 bytes requested.");
-	if (_bytes == 32 || _padToWordBoundaries)
-	{
-		m_context << u256(_offset) << load;
-		return 32;
-	}
-	else
-	{
-		// load data and add leading or trailing zeros by dividing/multiplying depending on alignment
-		u256 shiftFactor = u256(1) << ((32 - _bytes) * 8);
-		m_context << shiftFactor;
-		if (_leftAligned)
-			m_context << eth::Instruction::DUP1;
-		m_context << u256(_offset) << load << eth::Instruction::DIV;
-		if (_leftAligned)
-			m_context << eth::Instruction::MUL;
-		return _bytes;
-	}
+	solAssert(_type.getCategory() != Type::Category::ByteArray, "Unable to statically load dynamic type.");
+	m_context << u256(_offset);
+	return loadFromMemoryHelper(_type, _fromCalldata, _padToWordBoundaries);
 }
 
+void CompilerUtils::loadFromMemoryDynamic(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries)
+{
+	solAssert(_type.getCategory() != Type::Category::ByteArray, "Byte arrays not yet implemented.");
+	m_context << eth::Instruction::DUP1;
+	unsigned numBytes = loadFromMemoryHelper(_type, _fromCalldata, _padToWordBoundaries);
+	// update memory counter
+	for (unsigned i = 0; i < _type.getSizeOnStack(); ++i)
+		m_context << eth::swapInstruction(1 + i);
+	m_context << u256(numBytes) << eth::Instruction::ADD;
+}
+
+
 unsigned CompilerUtils::storeInMemory(unsigned _offset, Type const& _type, bool _padToWordBoundaries)
 {
 	solAssert(_type.getCategory() != Type::Category::ByteArray, "Unable to statically store dynamic type.");
@@ -79,9 +70,12 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound
 
 		if (type.getLocation() == ByteArrayType::Location::CallData)
 		{
-			m_context << eth::Instruction::CALLDATASIZE << u256(0) << eth::Instruction::DUP3
-					  << eth::Instruction::CALLDATACOPY
-					  << eth::Instruction::CALLDATASIZE << eth::Instruction::ADD;
+			// stack: target source_offset source_len
+			m_context << eth::Instruction::DUP1 << eth::Instruction::DUP3 << eth::Instruction::DUP5
+				// stack: target source_offset source_len source_len source_offset target
+				<< eth::Instruction::CALLDATACOPY
+				<< eth::Instruction::DUP3 << eth::Instruction::ADD
+				<< eth::Instruction::SWAP2 << eth::Instruction::POP << eth::Instruction::POP;
 		}
 		else
 		{
@@ -120,6 +114,7 @@ void CompilerUtils::storeInMemoryDynamic(Type const& _type, bool _padToWordBound
 		unsigned numBytes = prepareMemoryStore(_type, _padToWordBoundaries);
 		if (numBytes > 0)
 		{
+			solAssert(_type.getSizeOnStack() == 1, "Memory store of types with stack size != 1 not implemented.");
 			m_context << eth::Instruction::DUP2 << eth::Instruction::MSTORE;
 			m_context << u256(numBytes) << eth::Instruction::ADD;
 		}
@@ -179,29 +174,32 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType,
 	{
 	case ByteArrayType::Location::CallData:
 	{
-		// @todo this does not take length into account. It also assumes that after "CALLDATALENGTH" we only have zeros.
+		// This also assumes that after "length" we only have zeros, i.e. it cannot be used to
+		// slice a byte array from calldata.
+
+		// stack: source_offset source_len target_ref
 		// fetch old length and convert to words
 		m_context << eth::Instruction::DUP1 << eth::Instruction::SLOAD;
 		m_context << u256(31) << eth::Instruction::ADD
 				  << u256(32) << eth::Instruction::SWAP1 << eth::Instruction::DIV;
-		// stack here: target_ref target_length_words
+		// stack here: source_offset source_len target_ref target_length_words
 		// actual array data is stored at SHA3(storage_offset)
 		m_context << eth::Instruction::DUP2;
 		CompilerUtils(m_context).computeHashStatic();
 		// compute target_data_end
 		m_context << eth::Instruction::DUP1 << eth::Instruction::SWAP2 << eth::Instruction::ADD
 				  << eth::Instruction::SWAP1;
-		// stack here: target_ref target_data_end target_data_ref
+		// stack here: source_offset source_len target_ref target_data_end target_data_ref
 		// store length (in bytes)
-		m_context << eth::Instruction::CALLDATASIZE;
-		m_context << eth::Instruction::DUP1 << eth::Instruction::DUP5 << eth::Instruction::SSTORE;
+		m_context << eth::Instruction::DUP4 << eth::Instruction::DUP1 << eth::Instruction::DUP5
+			<< eth::Instruction::SSTORE;
 		// jump to end if length is zero
 		m_context << eth::Instruction::ISZERO;
 		eth::AssemblyItem copyLoopEnd = m_context.newTag();
 		m_context.appendConditionalJumpTo(copyLoopEnd);
 		// store start offset
-		m_context << u256(0);
-		// stack now: target_ref target_data_end target_data_ref calldata_offset
+		m_context << eth::Instruction::DUP5;
+		// stack now: source_offset source_len target_ref target_data_end target_data_ref calldata_offset
 		eth::AssemblyItem copyLoopStart = m_context.newTag();
 		m_context << copyLoopStart
 				  // copy from calldata and store
@@ -212,16 +210,18 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType,
 				  // increment calldata_offset by 32
 				  << eth::Instruction::SWAP1 << u256(32) << eth::Instruction::ADD
 				  // check for loop condition
-				  << eth::Instruction::DUP1 << eth::Instruction::CALLDATASIZE << eth::Instruction::GT;
+				  << eth::Instruction::DUP1 << eth::Instruction::DUP6 << eth::Instruction::GT;
 		m_context.appendConditionalJumpTo(copyLoopStart);
 		m_context << eth::Instruction::POP;
 		m_context << copyLoopEnd;
 
 		// now clear leftover bytes of the old value
-		// stack now: target_ref target_data_end target_data_ref
+		// stack now: source_offset source_len target_ref target_data_end target_data_ref
 		clearStorageLoop();
+		// stack now: source_offset source_len target_ref target_data_end
 
-		m_context << eth::Instruction::POP;
+		m_context << eth::Instruction::POP << eth::Instruction::SWAP2
+			<< eth::Instruction::POP << eth::Instruction::POP;
 		break;
 	}
 	case ByteArrayType::Location::Storage:
@@ -289,6 +289,30 @@ void CompilerUtils::copyByteArrayToStorage(ByteArrayType const& _targetType,
 	}
 }
 
+unsigned CompilerUtils::loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries)
+{
+	unsigned _encodedSize = _type.getCalldataEncodedSize();
+	unsigned numBytes = _padToWordBoundaries ? getPaddedSize(_encodedSize) : _encodedSize;
+	bool leftAligned = _type.getCategory() == Type::Category::String;
+	if (numBytes == 0)
+		m_context << eth::Instruction::POP << u256(0);
+	else
+	{
+		solAssert(numBytes <= 32, "Static memory load of more than 32 bytes requested.");
+		m_context << (_fromCalldata ? eth::Instruction::CALLDATALOAD : eth::Instruction::MLOAD);
+		if (numBytes != 32)
+		{
+			// add leading or trailing zeros by dividing/multiplying depending on alignment
+			u256 shiftFactor = u256(1) << ((32 - numBytes) * 8);
+			m_context << shiftFactor << eth::Instruction::SWAP1 << eth::Instruction::DIV;
+			if (leftAligned)
+				m_context << shiftFactor << eth::Instruction::MUL;
+		}
+	}
+
+	return numBytes;
+}
+
 void CompilerUtils::clearByteArray(ByteArrayType const& _type) const
 {
 	solAssert(_type.getLocation() == ByteArrayType::Location::Storage, "");
diff --git a/CompilerUtils.h b/CompilerUtils.h
index fe28cead..5369d3bf 100644
--- a/CompilerUtils.h
+++ b/CompilerUtils.h
@@ -37,14 +37,16 @@ public:
 
 	/// Loads data from memory to the stack.
 	/// @param _offset offset in memory (or calldata)
-	/// @param _bytes number of bytes to load
-	/// @param _leftAligned if true, store left aligned on stack (otherwise right aligned)
+	/// @param _type data type to load
 	/// @param _fromCalldata if true, load from calldata, not from memory
 	/// @param _padToWordBoundaries if true, assume the data is padded to word (32 byte) boundaries
-	/// @returns the number of bytes consumed in memory (can be different from _bytes if
-	///          _padToWordBoundaries is true)
-	unsigned loadFromMemory(unsigned _offset, unsigned _bytes = 32, bool _leftAligned = false,
-							bool _fromCalldata = false, bool _padToWordBoundaries = false);
+	/// @returns the number of bytes consumed in memory.
+	unsigned loadFromMemory(unsigned _offset, Type const& _type = IntegerType(256),
+		bool _fromCalldata = false, bool _padToWordBoundaries = false);
+	/// Dynamic version of @see loadFromMemory, expects the memory offset on the stack.
+	/// Stack pre: memory_offset
+	/// Stack post: value... (memory_offset+length)
+	void loadFromMemoryDynamic(Type const& _type, bool _fromCalldata = false, bool _padToWordBoundaries = true);
 	/// Stores data from stack in memory.
 	/// @param _offset offset in memory
 	/// @param _type type of the data on the stack
@@ -93,6 +95,8 @@ public:
 private:
 	/// Prepares the given type for storing in memory by shifting it if necessary.
 	unsigned prepareMemoryStore(Type const& _type, bool _padToWordBoundaries) const;
+	/// Loads type from memory assuming memory offset is on stack top.
+	unsigned loadFromMemoryHelper(Type const& _type, bool _fromCalldata, bool _padToWordBoundaries);
 	/// Appends a loop that clears a sequence of storage slots (excluding end).
 	/// Stack pre: end_ref start_ref
 	/// Stack post: end_ref
diff --git a/DeclarationContainer.cpp b/DeclarationContainer.cpp
index 2e810a4c..2594d428 100644
--- a/DeclarationContainer.cpp
+++ b/DeclarationContainer.cpp
@@ -28,14 +28,19 @@ namespace dev
 namespace solidity
 {
 
-bool DeclarationContainer::registerDeclaration(Declaration const& _declaration, bool _update)
+bool DeclarationContainer::registerDeclaration(Declaration const& _declaration, bool _invisible, bool _update)
 {
-	if (_declaration.getName().empty())
+	ASTString const& name(_declaration.getName());
+	if (name.empty())
 		return true;
 
-	if (!_update && m_declarations.find(_declaration.getName()) != m_declarations.end())
+	if (!_update && (m_declarations.count(name) || m_invisibleDeclarations.count(name)))
 		return false;
-	m_declarations[_declaration.getName()] = &_declaration;
+
+	if (_invisible)
+		m_invisibleDeclarations.insert(name);
+	else
+		m_declarations[name] = &_declaration;
 	return true;
 }
 
diff --git a/DeclarationContainer.h b/DeclarationContainer.h
index 1216fcef..f70881f5 100644
--- a/DeclarationContainer.h
+++ b/DeclarationContainer.h
@@ -23,6 +23,7 @@
 #pragma once
 
 #include <map>
+#include <set>
 #include <boost/noncopyable.hpp>
 
 #include <libsolidity/ASTForward.h>
@@ -43,8 +44,10 @@ public:
 								  DeclarationContainer const* _enclosingContainer = nullptr):
 		m_enclosingDeclaration(_enclosingDeclaration), m_enclosingContainer(_enclosingContainer) {}
 	/// Registers the declaration in the scope unless its name is already declared or the name is empty.
+	/// @param _invisible if true, registers the declaration, reports name clashes but does not return it in @a resolveName
+	/// @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, bool _update = false);
+	bool registerDeclaration(Declaration const& _declaration, bool _invisible = false, bool _update = false);
 	Declaration const* resolveName(ASTString const& _name, bool _recursive = false) const;
 	Declaration const* getEnclosingDeclaration() const { return m_enclosingDeclaration; }
 	std::map<ASTString, Declaration const*> const& getDeclarations() const { return m_declarations; }
@@ -53,6 +56,7 @@ private:
 	Declaration const* m_enclosingDeclaration;
 	DeclarationContainer const* m_enclosingContainer;
 	std::map<ASTString, Declaration const*> m_declarations;
+	std::set<ASTString> m_invisibleDeclarations;
 };
 
 }
diff --git a/ExpressionCompiler.cpp b/ExpressionCompiler.cpp
index 0f0e94f2..3bf1c8c9 100644
--- a/ExpressionCompiler.cpp
+++ b/ExpressionCompiler.cpp
@@ -475,9 +475,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 		else if (member == "gasprice")
 			m_context << eth::Instruction::GASPRICE;
 		else if (member == "data")
-		{
-			// nothing to store on the stack
-		}
+			m_context << u256(0) << eth::Instruction::CALLDATASIZE;
 		else
 			BOOST_THROW_EXCEPTION(InternalCompilerError() << errinfo_comment("Unknown magic member."));
 		break;
@@ -510,6 +508,7 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 					m_context << m_context.getFunctionEntryLabel(*function).pushTag();
 					return;
 				}
+			solAssert(false, "Function not found in member access.");
 		}
 		else if (auto enumType = dynamic_cast<EnumType const*>(type.getActualType().get()))
 			m_context << enumType->getMemberValue(_memberAccess.getMemberName());
@@ -518,7 +517,19 @@ void ExpressionCompiler::endVisit(MemberAccess const& _memberAccess)
 	case Type::Category::ByteArray:
 	{
 		solAssert(member == "length", "Illegal bytearray member.");
-		m_context << eth::Instruction::SLOAD;
+		auto const& type = dynamic_cast<ByteArrayType const&>(*_memberAccess.getExpression().getType());
+		switch (type.getLocation())
+		{
+		case ByteArrayType::Location::CallData:
+			m_context << eth::Instruction::SWAP1 << eth::Instruction::POP;
+			break;
+		case ByteArrayType::Location::Storage:
+			m_context << eth::Instruction::SLOAD;
+			break;
+		default:
+			solAssert(false, "Unsupported byte array location.");
+			break;
+		}
 		break;
 	}
 	default:
@@ -885,11 +896,8 @@ void ExpressionCompiler::appendExternalFunctionCall(FunctionType const& _functio
 		m_context << eth::Instruction::POP;
 	m_context << eth::Instruction::POP; // pop contract address
 
-	if (retSize > 0)
-	{
-		bool const c_leftAligned = firstType->getCategory() == Type::Category::String;
-		CompilerUtils(m_context).loadFromMemory(0, retSize, c_leftAligned, false, true);
-	}
+	if (firstType)
+		CompilerUtils(m_context).loadFromMemory(0, *firstType, false, true);
 }
 
 void ExpressionCompiler::appendArgumentsCopyToMemory(vector<ASTPointer<Expression const>> const& _arguments,
diff --git a/NameAndTypeResolver.cpp b/NameAndTypeResolver.cpp
index dbe5693a..ea70b65b 100644
--- a/NameAndTypeResolver.cpp
+++ b/NameAndTypeResolver.cpp
@@ -86,7 +86,7 @@ void NameAndTypeResolver::checkTypeRequirements(ContractDefinition& _contract)
 
 void NameAndTypeResolver::updateDeclaration(Declaration const& _declaration)
 {
-	m_scopes[nullptr].registerDeclaration(_declaration, true);
+	m_scopes[nullptr].registerDeclaration(_declaration, false, true);
 	solAssert(_declaration.getScope() == nullptr, "Updated declaration outside global scope.");
 }
 
@@ -110,8 +110,9 @@ void NameAndTypeResolver::importInheritedScope(ContractDefinition const& _base)
 	for (auto const& nameAndDeclaration: iterator->second.getDeclarations())
 	{
 		Declaration const* declaration = nameAndDeclaration.second;
-		// Import if it was declared in the base and is not the constructor
-		if (declaration->getScope() == &_base && declaration->getName() != _base.getName())
+		// Import if it was declared in the base, is not the constructor and is visible in derived classes
+		if (declaration->getScope() == &_base && declaration->getName() != _base.getName() &&
+				declaration->isVisibleInDerivedContracts())
 			m_currentScope->registerDeclaration(*declaration);
 	}
 }
@@ -308,7 +309,7 @@ void DeclarationRegistrationHelper::closeCurrentScope()
 
 void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaration, bool _opensScope)
 {
-	if (!m_scopes[m_currentScope].registerDeclaration(_declaration))
+	if (!m_scopes[m_currentScope].registerDeclaration(_declaration, !_declaration.isVisibleInContract()))
 		BOOST_THROW_EXCEPTION(DeclarationError() << errinfo_sourceLocation(_declaration.getLocation())
 												 << errinfo_comment("Identifier already declared."));
 	//@todo the exception should also contain the location of the first declaration
diff --git a/Parser.cpp b/Parser.cpp
index c96593f6..cf57ca50 100644
--- a/Parser.cpp
+++ b/Parser.cpp
@@ -190,6 +190,8 @@ Declaration::Visibility Parser::parseVisibilitySpecifier(Token::Value _token)
 		visibility = Declaration::Visibility::Protected;
 	else if (_token == Token::Private)
 		visibility = Declaration::Visibility::Private;
+	else if (_token == Token::External)
+		visibility = Declaration::Visibility::External;
 	else
 		solAssert(false, "Invalid visibility specifier.");
 	m_scanner->next();
@@ -306,7 +308,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(VarDeclParserOp
 	ASTPointer<ASTString> identifier;
 	Token::Value token = m_scanner->getCurrentToken();
 	Declaration::Visibility visibility(Declaration::Visibility::Default);
-	if (_options.isStateVariable && Token::isVisibilitySpecifier(token))
+	if (_options.isStateVariable && Token::isVariableVisibilitySpecifier(token))
 		visibility = parseVisibilitySpecifier(token);
 	if (_options.allowIndexed && token == Token::Indexed)
 	{
diff --git a/Token.h b/Token.h
index 3e599a6e..7bf8a7ef 100644
--- a/Token.h
+++ b/Token.h
@@ -150,6 +150,7 @@ namespace solidity
 	K(Do, "do", 0)                                                     \
 	K(Else, "else", 0)                                                 \
 	K(Event, "event", 0)                                               \
+	K(External, "external", 0)                                         \
 	K(Is, "is", 0)                                                     \
 	K(Indexed, "indexed", 0)                                           \
 	K(For, "for", 0)                                                   \
@@ -378,7 +379,8 @@ public:
 	static bool isUnaryOp(Value op) { return (Not <= op && op <= Delete) || op == Add || op == Sub; }
 	static bool isCountOp(Value op) { return op == Inc || op == Dec; }
 	static bool isShiftOp(Value op) { return (SHL <= op) && (op <= SHR); }
-	static bool isVisibilitySpecifier(Value op) { return op == Public || op == Private || op == Protected; }
+	static bool isVisibilitySpecifier(Value op) { return isVariableVisibilitySpecifier(op) || op == External; }
+	static bool isVariableVisibilitySpecifier(Value op) { return op == Public || op == Private || op == Protected; }
 	static bool isEtherSubdenomination(Value op) { return op == SubWei || op == SubSzabo || op == SubFinney || op == Token::SubEther; }
 
 	// Returns a string corresponding to the JS token string
diff --git a/Types.cpp b/Types.cpp
index b41834d8..d2f0e9bf 100644
--- a/Types.cpp
+++ b/Types.cpp
@@ -540,12 +540,19 @@ bool ByteArrayType::operator==(Type const& _other) const
 unsigned ByteArrayType::getSizeOnStack() const
 {
 	if (m_location == Location::CallData)
-		return 0;
+		// offset, length (stack top)
+		return 2;
 	else
+		// offset
 		return 1;
 }
 
-const MemberList ByteArrayType::s_byteArrayMemberList = MemberList({{"length", make_shared<IntegerType >(256)}});
+shared_ptr<ByteArrayType> ByteArrayType::copyForLocation(ByteArrayType::Location _location) const
+{
+	return make_shared<ByteArrayType>(_location);
+}
+
+const MemberList ByteArrayType::s_byteArrayMemberList = MemberList({{"length", make_shared<IntegerType>(256)}});
 
 bool ContractType::operator==(Type const& _other) const
 {
@@ -572,7 +579,8 @@ MemberList const& ContractType::getMembers() const
 		{
 			for (ContractDefinition const* base: m_contract.getLinearizedBaseContracts())
 				for (ASTPointer<FunctionDefinition> const& function: base->getDefinedFunctions())
-					if (!function->isConstructor() && !function->getName().empty())
+					if (!function->isConstructor() && !function->getName().empty() &&
+							function->isVisibleInDerivedContracts())
 						members.insert(make_pair(function->getName(), make_shared<FunctionType>(*function, true)));
 		}
 		else
@@ -957,10 +965,10 @@ MemberList const& TypeType::getMembers() const
 			ContractDefinition const& contract = dynamic_cast<ContractType const&>(*m_actualType).getContractDefinition();
 			vector<ContractDefinition const*> currentBases = m_currentContract->getLinearizedBaseContracts();
 			if (find(currentBases.begin(), currentBases.end(), &contract) != currentBases.end())
-				// We are accessing the type of a base contract, so add all public and private
+				// We are accessing the type of a base contract, so add all public and protected
 				// functions. Note that this does not add inherited functions on purpose.
 				for (ASTPointer<FunctionDefinition> const& f: contract.getDefinedFunctions())
-					if (!f->isConstructor() && !f->getName().empty())
+					if (!f->isConstructor() && !f->getName().empty() && f->isVisibleInDerivedContracts())
 						members[f->getName()] = make_shared<FunctionType>(*f);
 		}
 		else if (m_actualType->getCategory() == Category::Enum)
diff --git a/Types.h b/Types.h
index 3b4eee57..b66857f0 100644
--- a/Types.h
+++ b/Types.h
@@ -122,6 +122,8 @@ public:
 	/// is not a simple big-endian encoding or the type cannot be stored in calldata.
 	/// Note that irrespective of this size, each calldata element is padded to a multiple of 32 bytes.
 	virtual unsigned getCalldataEncodedSize() const { return 0; }
+	/// @returns true if the type is dynamically encoded in calldata
+	virtual bool isDynamicallySized() const { return false; }
 	/// @returns number of bytes required to hold this value in storage.
 	/// For dynamically "allocated" types, it returns the size of the statically allocated head,
 	virtual u256 getStorageSize() const { return 1; }
@@ -289,12 +291,17 @@ public:
 	virtual bool isImplicitlyConvertibleTo(Type const& _convertTo) const override;
 	virtual TypePointer unaryOperatorResult(Token::Value _operator) const override;
 	virtual bool operator==(const Type& _other) const override;
+	virtual bool isDynamicallySized() const { return true; }
 	virtual unsigned getSizeOnStack() const override;
 	virtual std::string toString() const override { return "bytes"; }
 	virtual MemberList const& getMembers() const override { return s_byteArrayMemberList; }
 
 	Location getLocation() const { return m_location; }
 
+	/// @returns a copy of this type with location changed to @a _location
+	/// @todo this might move as far up as Type later
+	std::shared_ptr<ByteArrayType> copyForLocation(Location _location) const;
+
 private:
 	Location m_location;
 	static const MemberList s_byteArrayMemberList;