"external" visibility specifier.

libsolidity/AST.h

@@ -133,7 +133,8 @@ 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 +143,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.

libsolidity/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;
 }
 

libsolidity/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;
 };
 
 }

libsolidity/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
+		// 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())
+		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

libsolidity/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)
 	{

libsolidity/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

libsolidity/Types.cpp

@@ -572,7 +572,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 +958,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)

test/SolidityNameAndTypeResolution.cpp

@@ -1083,6 +1083,56 @@ BOOST_AUTO_TEST_CASE(enum_duplicate_values)
 	BOOST_CHECK_THROW(parseTextAndResolveNames(text), DeclarationError);
 }
 
+BOOST_AUTO_TEST_CASE(private_visibility)
+{
+	char const* sourceCode = R"(
+		contract base {
+			function f() private {}
+		}
+		contract derived is base {
+			function g() { f(); }
+		}
+		)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), DeclarationError);
+}
+
+BOOST_AUTO_TEST_CASE(private_visibility_via_explicit_base_access)
+{
+	char const* sourceCode = R"(
+		contract base {
+			function f() private {}
+		}
+		contract derived is base {
+			function g() { base.f(); }
+		}
+		)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError);
+}
+
+BOOST_AUTO_TEST_CASE(external_visibility)
+{
+	char const* sourceCode = R"(
+		contract c {
+			function f() external {}
+			function g() { f(); }
+		}
+		)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), DeclarationError);
+}
+
+BOOST_AUTO_TEST_CASE(external_base_visibility)
+{
+	char const* sourceCode = R"(
+		contract base {
+			function f() external {}
+		}
+		contract derived is base {
+			function g() { base.f(); }
+		}
+		)";
+	BOOST_CHECK_THROW(parseTextAndResolveNames(sourceCode), TypeError);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }

test/SolidityParser.cpp

@@ -735,6 +735,24 @@ BOOST_AUTO_TEST_CASE(malformed_enum_declaration)
 	BOOST_CHECK_THROW(parseText(text), ParserError);
 }
 
+BOOST_AUTO_TEST_CASE(external_function)
+{
+	char const* text = R"(
+		contract c {
+			function x() external {}
+		})";
+	BOOST_CHECK_NO_THROW(parseTextExplainError(text));
+}
+
+BOOST_AUTO_TEST_CASE(external_variable)
+{
+	char const* text = R"(
+		contract c {
+			uint external x;
+		})";
+	BOOST_CHECK_THROW(parseText(text), ParserError);
+}
+
 BOOST_AUTO_TEST_SUITE_END()
 
 }