Merge pull request #7951 from ethereum/extractVisibility

Move Visibility out of Declaration.
This commit is contained in:
Erik K 2019-12-11 10:16:03 +01:00 committed by GitHub
commit 7247e72d8b
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 69 additions and 63 deletions

View File

@ -154,7 +154,7 @@ void OverrideChecker::checkIllegalOverrides(ContractDefinition const& _contract)
if (!hasEqualNameAndParameters(*stateVar, **it)) if (!hasEqualNameAndParameters(*stateVar, **it))
continue; continue;
if ((*it)->visibility() != Declaration::Visibility::External) if ((*it)->visibility() != Visibility::External)
overrideError(*stateVar, **it, "Public state variables can only override functions with external visibility."); overrideError(*stateVar, **it, "Public state variables can only override functions with external visibility.");
else else
checkOverride(*stateVar, **it); checkOverride(*stateVar, **it);
@ -265,8 +265,8 @@ void OverrideChecker::checkOverride(T const& _overriding, U const& _super)
// Visibility change from external to public is fine. // Visibility change from external to public is fine.
// Any other change is disallowed. // Any other change is disallowed.
if (!( if (!(
_super.visibility() == FunctionDefinition::Visibility::External && _super.visibility() == Visibility::External &&
_overriding.visibility() == FunctionDefinition::Visibility::Public _overriding.visibility() == Visibility::Public
)) ))
overrideError(_overriding, _super, "Overriding " + overridingName + " visibility differs."); overrideError(_overriding, _super, "Overriding " + overridingName + " visibility differs.");
} }

View File

@ -198,21 +198,21 @@ void ReferencesResolver::endVisit(FunctionTypeName const& _typeName)
{ {
switch (_typeName.visibility()) switch (_typeName.visibility())
{ {
case VariableDeclaration::Visibility::Internal: case Visibility::Internal:
case VariableDeclaration::Visibility::External: case Visibility::External:
break; break;
default: default:
fatalTypeError(_typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\"."); fatalTypeError(_typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\".");
return; return;
} }
if (_typeName.isPayable() && _typeName.visibility() != VariableDeclaration::Visibility::External) if (_typeName.isPayable() && _typeName.visibility() != Visibility::External)
{ {
fatalTypeError(_typeName.location(), "Only external function types can be payable."); fatalTypeError(_typeName.location(), "Only external function types can be payable.");
return; return;
} }
if (_typeName.visibility() == VariableDeclaration::Visibility::External) if (_typeName.visibility() == Visibility::External)
for (auto const& t: _typeName.parameterTypes() + _typeName.returnParameterTypes()) for (auto const& t: _typeName.parameterTypes() + _typeName.returnParameterTypes())
{ {
solAssert(t->annotation().type, "Type not set for parameter."); solAssert(t->annotation().type, "Type not set for parameter.");

View File

@ -332,7 +332,7 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
{ {
if (_function.annotation().contract->isInterface()) if (_function.annotation().contract->isInterface())
m_errorReporter.warning(_function.location(), "Interface functions are implicitly \"virtual\""); m_errorReporter.warning(_function.location(), "Interface functions are implicitly \"virtual\"");
if (_function.visibility() == Declaration::Visibility::Private) if (_function.visibility() == Visibility::Private)
m_errorReporter.typeError(_function.location(), "\"virtual\" and \"private\" cannot be used together."); m_errorReporter.typeError(_function.location(), "\"virtual\" and \"private\" cannot be used together.");
} }
@ -419,7 +419,7 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
if (_function.isImplemented()) if (_function.isImplemented())
m_errorReporter.typeError(_function.location(), "Functions in interfaces cannot have an implementation."); m_errorReporter.typeError(_function.location(), "Functions in interfaces cannot have an implementation.");
if (_function.visibility() != FunctionDefinition::Visibility::External) if (_function.visibility() != Visibility::External)
m_errorReporter.typeError(_function.location(), "Functions in interfaces must be declared external."); m_errorReporter.typeError(_function.location(), "Functions in interfaces must be declared external.");
if (_function.isConstructor()) if (_function.isConstructor())
@ -498,7 +498,7 @@ bool TypeChecker::visit(VariableDeclaration const& _variable)
if (!varType->canLiveOutsideStorage()) if (!varType->canLiveOutsideStorage())
m_errorReporter.typeError(_variable.location(), "Type " + varType->toString() + " is only valid in storage."); m_errorReporter.typeError(_variable.location(), "Type " + varType->toString() + " is only valid in storage.");
} }
else if (_variable.visibility() >= VariableDeclaration::Visibility::Public) else if (_variable.visibility() >= Visibility::Public)
{ {
FunctionType getter(_variable); FunctionType getter(_variable);
if (!_variable.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2)) if (!_variable.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2))
@ -604,7 +604,7 @@ void TypeChecker::visitManually(
bool TypeChecker::visit(EventDefinition const& _eventDef) bool TypeChecker::visit(EventDefinition const& _eventDef)
{ {
solAssert(_eventDef.visibility() > Declaration::Visibility::Internal, ""); solAssert(_eventDef.visibility() > Visibility::Internal, "");
unsigned numIndexed = 0; unsigned numIndexed = 0;
for (ASTPointer<VariableDeclaration> const& var: _eventDef.parameters()) for (ASTPointer<VariableDeclaration> const& var: _eventDef.parameters())
{ {
@ -1742,7 +1742,7 @@ void TypeChecker::typeCheckFallbackFunction(FunctionDefinition const& _function)
stateMutabilityToString(_function.stateMutability()) + stateMutabilityToString(_function.stateMutability()) +
"\"." "\"."
); );
if (_function.visibility() != FunctionDefinition::Visibility::External) if (_function.visibility() != Visibility::External)
m_errorReporter.typeError(_function.location(), "Fallback function must be defined as \"external\"."); m_errorReporter.typeError(_function.location(), "Fallback function must be defined as \"external\".");
if (!_function.returnParameters().empty()) if (!_function.returnParameters().empty())
{ {
@ -1769,7 +1769,7 @@ void TypeChecker::typeCheckReceiveFunction(FunctionDefinition const& _function)
stateMutabilityToString(_function.stateMutability()) + stateMutabilityToString(_function.stateMutability()) +
"\"." "\"."
); );
if (_function.visibility() != FunctionDefinition::Visibility::External) if (_function.visibility() != Visibility::External)
m_errorReporter.typeError(_function.location(), "Receive ether function must be defined as \"external\"."); m_errorReporter.typeError(_function.location(), "Receive ether function must be defined as \"external\".");
if (!_function.returnParameters().empty()) if (!_function.returnParameters().empty())
m_errorReporter.typeError(_function.returnParameterList()->location(), "Receive ether function cannot return values."); m_errorReporter.typeError(_function.returnParameterList()->location(), "Receive ether function cannot return values.");
@ -1790,7 +1790,7 @@ void TypeChecker::typeCheckConstructor(FunctionDefinition const& _function)
stateMutabilityToString(_function.stateMutability()) + stateMutabilityToString(_function.stateMutability()) +
"\"." "\"."
); );
if (_function.visibility() != FunctionDefinition::Visibility::Public && _function.visibility() != FunctionDefinition::Visibility::Internal) if (_function.visibility() != Visibility::Public && _function.visibility() != Visibility::Internal)
m_errorReporter.typeError(_function.location(), "Constructor must be public or internal."); m_errorReporter.typeError(_function.location(), "Constructor must be public or internal.");
} }

View File

@ -328,13 +328,13 @@ FunctionTypePointer FunctionDefinition::functionType(bool _internal) const
{ {
switch (visibility()) switch (visibility())
{ {
case Declaration::Visibility::Default: case Visibility::Default:
solAssert(false, "visibility() should not return Default"); solAssert(false, "visibility() should not return Default");
case Declaration::Visibility::Private: case Visibility::Private:
case Declaration::Visibility::Internal: case Visibility::Internal:
case Declaration::Visibility::Public: case Visibility::Public:
return TypeProvider::function(*this, _internal); return TypeProvider::function(*this, _internal);
case Declaration::Visibility::External: case Visibility::External:
return {}; return {};
} }
} }
@ -342,13 +342,13 @@ FunctionTypePointer FunctionDefinition::functionType(bool _internal) const
{ {
switch (visibility()) switch (visibility())
{ {
case Declaration::Visibility::Default: case Visibility::Default:
solAssert(false, "visibility() should not return Default"); solAssert(false, "visibility() should not return Default");
case Declaration::Visibility::Private: case Visibility::Private:
case Declaration::Visibility::Internal: case Visibility::Internal:
return {}; return {};
case Declaration::Visibility::Public: case Visibility::Public:
case Declaration::Visibility::External: case Visibility::External:
return TypeProvider::function(*this, _internal); return TypeProvider::function(*this, _internal);
} }
} }
@ -359,7 +359,7 @@ FunctionTypePointer FunctionDefinition::functionType(bool _internal) const
TypePointer FunctionDefinition::type() const TypePointer FunctionDefinition::type() const
{ {
solAssert(visibility() != Declaration::Visibility::External, ""); solAssert(visibility() != Visibility::External, "");
return TypeProvider::function(*this); return TypeProvider::function(*this);
} }
@ -518,7 +518,7 @@ bool VariableDeclaration::isExternalCallableParameter() const
return false; return false;
if (auto const* callable = dynamic_cast<CallableDeclaration const*>(scope())) if (auto const* callable = dynamic_cast<CallableDeclaration const*>(scope()))
if (callable->visibility() == Declaration::Visibility::External) if (callable->visibility() == Visibility::External)
return !isReturnParameter(); return !isReturnParameter();
return false; return false;
@ -530,9 +530,9 @@ bool VariableDeclaration::isInternalCallableParameter() const
return false; return false;
if (auto const* funTypeName = dynamic_cast<FunctionTypeName const*>(scope())) if (auto const* funTypeName = dynamic_cast<FunctionTypeName const*>(scope()))
return funTypeName->visibility() == Declaration::Visibility::Internal; return funTypeName->visibility() == Visibility::Internal;
else if (auto const* callable = dynamic_cast<CallableDeclaration const*>(scope())) else if (auto const* callable = dynamic_cast<CallableDeclaration const*>(scope()))
return callable->visibility() <= Declaration::Visibility::Internal; return callable->visibility() <= Visibility::Internal;
return false; return false;
} }
@ -607,13 +607,13 @@ FunctionTypePointer VariableDeclaration::functionType(bool _internal) const
return nullptr; return nullptr;
switch (visibility()) switch (visibility())
{ {
case Declaration::Visibility::Default: case Visibility::Default:
solAssert(false, "visibility() should not return Default"); solAssert(false, "visibility() should not return Default");
case Declaration::Visibility::Private: case Visibility::Private:
case Declaration::Visibility::Internal: case Visibility::Internal:
return nullptr; return nullptr;
case Declaration::Visibility::Public: case Visibility::Public:
case Declaration::Visibility::External: case Visibility::External:
return TypeProvider::function(*this); return TypeProvider::function(*this);
} }

View File

@ -182,20 +182,18 @@ protected:
class Declaration: public ASTNode, public Scopable class Declaration: public ASTNode, public Scopable
{ {
public: public:
/// Visibility ordered from restricted to unrestricted.
enum class Visibility { Default, Private, Internal, Public, External };
static std::string visibilityToString(Declaration::Visibility _visibility) static std::string visibilityToString(Visibility _visibility)
{ {
switch (_visibility) switch (_visibility)
{ {
case Declaration::Visibility::Public: case Visibility::Public:
return "public"; return "public";
case Declaration::Visibility::Internal: case Visibility::Internal:
return "internal"; return "internal";
case Declaration::Visibility::Private: case Visibility::Private:
return "private"; return "private";
case Declaration::Visibility::External: case Visibility::External:
return "external"; return "external";
default: default:
solAssert(false, "Invalid visibility specifier."); solAssert(false, "Invalid visibility specifier.");
@ -602,7 +600,7 @@ public:
CallableDeclaration( CallableDeclaration(
SourceLocation const& _location, SourceLocation const& _location,
ASTPointer<ASTString> const& _name, ASTPointer<ASTString> const& _name,
Declaration::Visibility _visibility, Visibility _visibility,
ASTPointer<ParameterList> const& _parameters, ASTPointer<ParameterList> const& _parameters,
bool _isVirtual = false, bool _isVirtual = false,
ASTPointer<OverrideSpecifier> const& _overrides = nullptr, ASTPointer<OverrideSpecifier> const& _overrides = nullptr,
@ -665,7 +663,7 @@ public:
FunctionDefinition( FunctionDefinition(
SourceLocation const& _location, SourceLocation const& _location,
ASTPointer<ASTString> const& _name, ASTPointer<ASTString> const& _name,
Declaration::Visibility _visibility, Visibility _visibility,
StateMutability _stateMutability, StateMutability _stateMutability,
Token _kind, Token _kind,
bool _isVirtual, bool _isVirtual,
@ -1028,7 +1026,7 @@ public:
SourceLocation const& _location, SourceLocation const& _location,
ASTPointer<ParameterList> const& _parameterTypes, ASTPointer<ParameterList> const& _parameterTypes,
ASTPointer<ParameterList> const& _returnTypes, ASTPointer<ParameterList> const& _returnTypes,
Declaration::Visibility _visibility, Visibility _visibility,
StateMutability _stateMutability StateMutability _stateMutability
): ):
TypeName(_location), m_parameterTypes(_parameterTypes), m_returnTypes(_returnTypes), TypeName(_location), m_parameterTypes(_parameterTypes), m_returnTypes(_returnTypes),
@ -1042,9 +1040,9 @@ public:
ASTPointer<ParameterList> const& parameterTypeList() const { return m_parameterTypes; } ASTPointer<ParameterList> const& parameterTypeList() const { return m_parameterTypes; }
ASTPointer<ParameterList> const& returnParameterTypeList() const { return m_returnTypes; } ASTPointer<ParameterList> const& returnParameterTypeList() const { return m_returnTypes; }
Declaration::Visibility visibility() const Visibility visibility() const
{ {
return m_visibility == Declaration::Visibility::Default ? Declaration::Visibility::Internal : m_visibility; return m_visibility == Visibility::Default ? Visibility::Internal : m_visibility;
} }
StateMutability stateMutability() const { return m_stateMutability; } StateMutability stateMutability() const { return m_stateMutability; }
bool isPayable() const { return m_stateMutability == StateMutability::Payable; } bool isPayable() const { return m_stateMutability == StateMutability::Payable; }
@ -1052,7 +1050,7 @@ public:
private: private:
ASTPointer<ParameterList> m_parameterTypes; ASTPointer<ParameterList> m_parameterTypes;
ASTPointer<ParameterList> m_returnTypes; ASTPointer<ParameterList> m_returnTypes;
Declaration::Visibility m_visibility; Visibility m_visibility;
StateMutability m_stateMutability; StateMutability m_stateMutability;
}; };

View File

@ -34,6 +34,9 @@ namespace solidity
// How a function can mutate the EVM state. // How a function can mutate the EVM state.
enum class StateMutability { Pure, View, NonPayable, Payable }; enum class StateMutability { Pure, View, NonPayable, Payable };
/// Visibility ordered from restricted to unrestricted.
enum class Visibility { Default, Private, Internal, Public, External };
inline std::string stateMutabilityToString(StateMutability const& _stateMutability) inline std::string stateMutabilityToString(StateMutability const& _stateMutability)
{ {
switch (_stateMutability) switch (_stateMutability)

View File

@ -26,7 +26,12 @@
#include <string> #include <string>
#include <vector> #include <vector>
// Forward-declare all AST node types // Forward-declare all AST node types and related enums.
namespace langutil
{
enum class Token : unsigned int;
}
namespace dev namespace dev
{ {

View File

@ -2601,7 +2601,7 @@ FunctionType::FunctionType(EventDefinition const& _event):
FunctionType::FunctionType(FunctionTypeName const& _typeName): FunctionType::FunctionType(FunctionTypeName const& _typeName):
m_parameterNames(_typeName.parameterTypes().size(), ""), m_parameterNames(_typeName.parameterTypes().size(), ""),
m_returnParameterNames(_typeName.returnParameterTypes().size(), ""), m_returnParameterNames(_typeName.returnParameterTypes().size(), ""),
m_kind(_typeName.visibility() == VariableDeclaration::Visibility::External ? Kind::External : Kind::Internal), m_kind(_typeName.visibility() == Visibility::External ? Kind::External : Kind::Internal),
m_stateMutability(_typeName.stateMutability()) m_stateMutability(_typeName.stateMutability())
{ {
if (_typeName.isPayable()) if (_typeName.isPayable())
@ -3011,8 +3011,8 @@ MemberList::MemberMap FunctionType::nativeMembers(ContractDefinition const*) con
{ {
auto const* functionDefinition = dynamic_cast<FunctionDefinition const*>(m_declaration); auto const* functionDefinition = dynamic_cast<FunctionDefinition const*>(m_declaration);
solAssert(functionDefinition, ""); solAssert(functionDefinition, "");
solAssert(functionDefinition->visibility() != Declaration::Visibility::Private, ""); solAssert(functionDefinition->visibility() != Visibility::Private, "");
if (functionDefinition->visibility() != Declaration::Visibility::Internal) if (functionDefinition->visibility() != Visibility::Internal)
{ {
auto const* contract = dynamic_cast<ContractDefinition const*>(m_declaration->scope()); auto const* contract = dynamic_cast<ContractDefinition const*>(m_declaration->scope());
solAssert(contract, ""); solAssert(contract, "");

View File

@ -377,23 +377,23 @@ ASTPointer<InheritanceSpecifier> Parser::parseInheritanceSpecifier()
return nodeFactory.createNode<InheritanceSpecifier>(name, std::move(arguments)); return nodeFactory.createNode<InheritanceSpecifier>(name, std::move(arguments));
} }
Declaration::Visibility Parser::parseVisibilitySpecifier() Visibility Parser::parseVisibilitySpecifier()
{ {
Declaration::Visibility visibility(Declaration::Visibility::Default); Visibility visibility(Visibility::Default);
Token token = m_scanner->currentToken(); Token token = m_scanner->currentToken();
switch (token) switch (token)
{ {
case Token::Public: case Token::Public:
visibility = Declaration::Visibility::Public; visibility = Visibility::Public;
break; break;
case Token::Internal: case Token::Internal:
visibility = Declaration::Visibility::Internal; visibility = Visibility::Internal;
break; break;
case Token::Private: case Token::Private:
visibility = Declaration::Visibility::Private; visibility = Visibility::Private;
break; break;
case Token::External: case Token::External:
visibility = Declaration::Visibility::External; visibility = Visibility::External;
break; break;
default: default:
solAssert(false, "Invalid visibility specifier."); solAssert(false, "Invalid visibility specifier.");
@ -476,11 +476,11 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _isStateVari
result.modifiers.push_back(parseModifierInvocation()); result.modifiers.push_back(parseModifierInvocation());
else if (TokenTraits::isVisibilitySpecifier(token)) else if (TokenTraits::isVisibilitySpecifier(token))
{ {
if (result.visibility != Declaration::Visibility::Default) if (result.visibility != Visibility::Default)
{ {
// There is the special case of a public state variable of function type. // There is the special case of a public state variable of function type.
// Detect this and return early. // Detect this and return early.
if (_isStateVariable && (result.visibility == Declaration::Visibility::External || result.visibility == Declaration::Visibility::Internal)) if (_isStateVariable && (result.visibility == Visibility::External || result.visibility == Visibility::Internal))
break; break;
parserError(string( parserError(string(
"Visibility already specified as \"" + "Visibility already specified as \"" +
@ -687,7 +687,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
bool isIndexed = false; bool isIndexed = false;
bool isDeclaredConst = false; bool isDeclaredConst = false;
ASTPointer<OverrideSpecifier> overrides = nullptr; ASTPointer<OverrideSpecifier> overrides = nullptr;
Declaration::Visibility visibility(Declaration::Visibility::Default); Visibility visibility(Visibility::Default);
VariableDeclaration::Location location = VariableDeclaration::Location::Unspecified; VariableDeclaration::Location location = VariableDeclaration::Location::Unspecified;
ASTPointer<ASTString> identifier; ASTPointer<ASTString> identifier;
@ -697,7 +697,7 @@ ASTPointer<VariableDeclaration> Parser::parseVariableDeclaration(
if (_options.isStateVariable && TokenTraits::isVariableVisibilitySpecifier(token)) if (_options.isStateVariable && TokenTraits::isVariableVisibilitySpecifier(token))
{ {
nodeFactory.markEndPosition(); nodeFactory.markEndPosition();
if (visibility != Declaration::Visibility::Default) if (visibility != Visibility::Default)
{ {
parserError(string( parserError(string(
"Visibility already specified as \"" + "Visibility already specified as \"" +
@ -1534,7 +1534,7 @@ ASTPointer<VariableDeclarationStatement> Parser::parseVariableDeclarationStateme
ASTPointer<TypeName>(), ASTPointer<TypeName>(),
name, name,
ASTPointer<Expression>(), ASTPointer<Expression>(),
VariableDeclaration::Visibility::Default Visibility::Default
); );
} }
variables.push_back(var); variables.push_back(var);

View File

@ -74,7 +74,7 @@ private:
ASTPointer<OverrideSpecifier> overrides; ASTPointer<OverrideSpecifier> overrides;
ASTPointer<ParameterList> parameters; ASTPointer<ParameterList> parameters;
ASTPointer<ParameterList> returnParameters; ASTPointer<ParameterList> returnParameters;
Declaration::Visibility visibility = Declaration::Visibility::Default; Visibility visibility = Visibility::Default;
StateMutability stateMutability = StateMutability::NonPayable; StateMutability stateMutability = StateMutability::NonPayable;
std::vector<ASTPointer<ModifierInvocation>> modifiers; std::vector<ASTPointer<ModifierInvocation>> modifiers;
}; };
@ -89,7 +89,7 @@ private:
std::pair<ContractDefinition::ContractKind, bool> parseContractKind(); std::pair<ContractDefinition::ContractKind, bool> parseContractKind();
ASTPointer<ContractDefinition> parseContractDefinition(); ASTPointer<ContractDefinition> parseContractDefinition();
ASTPointer<InheritanceSpecifier> parseInheritanceSpecifier(); ASTPointer<InheritanceSpecifier> parseInheritanceSpecifier();
Declaration::Visibility parseVisibilitySpecifier(); Visibility parseVisibilitySpecifier();
ASTPointer<OverrideSpecifier> parseOverrideSpecifier(); ASTPointer<OverrideSpecifier> parseOverrideSpecifier();
StateMutability parseStateMutability(); StateMutability parseStateMutability();
FunctionHeaderParserResult parseFunctionHeader(bool _isStateVariable); FunctionHeaderParserResult parseFunctionHeader(bool _isStateVariable);