From 0185ac5a0d4609732410d5962a2bd7ad867f90c3 Mon Sep 17 00:00:00 2001 From: Christian Date: Sat, 14 Feb 2015 00:43:02 +0100 Subject: [PATCH] "external" visibility specifier. --- libsolidity/AST.h | 7 ++-- libsolidity/DeclarationContainer.cpp | 13 ++++--- libsolidity/DeclarationContainer.h | 6 +++- libsolidity/NameAndTypeResolver.cpp | 9 ++--- libsolidity/Parser.cpp | 4 ++- libsolidity/Token.h | 4 ++- libsolidity/Types.cpp | 7 ++-- test/SolidityNameAndTypeResolution.cpp | 50 ++++++++++++++++++++++++++ test/SolidityParser.cpp | 18 ++++++++++ 9 files changed, 102 insertions(+), 16 deletions(-) diff --git a/libsolidity/AST.h b/libsolidity/AST.h index 51d8031a3..c40e24d1d 100644 --- a/libsolidity/AST.h +++ b/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 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. diff --git a/libsolidity/DeclarationContainer.cpp b/libsolidity/DeclarationContainer.cpp index 2e810a4cf..2594d4281 100644 --- a/libsolidity/DeclarationContainer.cpp +++ b/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; } diff --git a/libsolidity/DeclarationContainer.h b/libsolidity/DeclarationContainer.h index 1216fcef2..f70881f5b 100644 --- a/libsolidity/DeclarationContainer.h +++ b/libsolidity/DeclarationContainer.h @@ -23,6 +23,7 @@ #pragma once #include +#include #include #include @@ -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 const& getDeclarations() const { return m_declarations; } @@ -53,6 +56,7 @@ private: Declaration const* m_enclosingDeclaration; DeclarationContainer const* m_enclosingContainer; std::map m_declarations; + std::set m_invisibleDeclarations; }; } diff --git a/libsolidity/NameAndTypeResolver.cpp b/libsolidity/NameAndTypeResolver.cpp index dbe5693a8..ea70b65b4 100644 --- a/libsolidity/NameAndTypeResolver.cpp +++ b/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 - 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/libsolidity/Parser.cpp b/libsolidity/Parser.cpp index c96593f64..cf57ca50e 100644 --- a/libsolidity/Parser.cpp +++ b/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 Parser::parseVariableDeclaration(VarDeclParserOp ASTPointer 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/libsolidity/Token.h b/libsolidity/Token.h index 3e599a6ee..7bf8a7ef0 100644 --- a/libsolidity/Token.h +++ b/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 diff --git a/libsolidity/Types.cpp b/libsolidity/Types.cpp index 5d753645c..99515a3ac 100644 --- a/libsolidity/Types.cpp +++ b/libsolidity/Types.cpp @@ -572,7 +572,8 @@ MemberList const& ContractType::getMembers() const { for (ContractDefinition const* base: m_contract.getLinearizedBaseContracts()) for (ASTPointer 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(*function, true))); } else @@ -957,10 +958,10 @@ MemberList const& TypeType::getMembers() const ContractDefinition const& contract = dynamic_cast(*m_actualType).getContractDefinition(); vector 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 const& f: contract.getDefinedFunctions()) - if (!f->isConstructor() && !f->getName().empty()) + if (!f->isConstructor() && !f->getName().empty() && f->isVisibleInDerivedContracts()) members[f->getName()] = make_shared(*f); } else if (m_actualType->getCategory() == Category::Enum) diff --git a/test/SolidityNameAndTypeResolution.cpp b/test/SolidityNameAndTypeResolution.cpp index d6e4ed516..f30de96ce 100644 --- a/test/SolidityNameAndTypeResolution.cpp +++ b/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() } diff --git a/test/SolidityParser.cpp b/test/SolidityParser.cpp index af82f612a..5f9064e0c 100644 --- a/test/SolidityParser.cpp +++ b/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() }