diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index c5c429ce1..b452a49a3 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -626,6 +626,17 @@ void DeclarationRegistrationHelper::endVisit(ModifierDefinition&) closeCurrentScope(); } +bool DeclarationRegistrationHelper::visit(FunctionTypeName& _funTypeName) +{ + enterNewSubScope(_funTypeName); + return true; +} + +void DeclarationRegistrationHelper::endVisit(FunctionTypeName&) +{ + closeCurrentScope(); +} + bool DeclarationRegistrationHelper::visit(Block& _block) { _block.setScope(m_currentScope); diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index 8178ee17a..a72c21e3c 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/libsolidity/analysis/NameAndTypeResolver.h @@ -171,6 +171,8 @@ private: void endVisit(FunctionDefinition& _function) override; bool visit(ModifierDefinition& _modifier) override; void endVisit(ModifierDefinition& _modifier) override; + bool visit(FunctionTypeName& _funTypeName) override; + void endVisit(FunctionTypeName& _funTypeName) override; bool visit(Block& _block) override; void endVisit(Block& _block) override; bool visit(ForStatement& _forLoop) override; diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index fa0888dd2..f33de7b7d 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -30,7 +30,10 @@ #include #include +#include + #include +#include using namespace std; using namespace dev; @@ -155,7 +158,10 @@ void ReferencesResolver::endVisit(UserDefinedTypeName const& _typeName) else if (ContractDefinition const* contract = dynamic_cast(declaration)) _typeName.annotation().type = make_shared(*contract); else + { + _typeName.annotation().type = make_shared(); typeError(_typeName.location(), "Name has to refer to a struct, enum or contract."); + } } void ReferencesResolver::endVisit(FunctionTypeName const& _typeName) @@ -166,13 +172,13 @@ void ReferencesResolver::endVisit(FunctionTypeName const& _typeName) case VariableDeclaration::Visibility::External: break; default: - typeError(_typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\"."); + fatalTypeError(_typeName.location(), "Invalid visibility, can only be \"external\" or \"internal\"."); return; } if (_typeName.isPayable() && _typeName.visibility() != VariableDeclaration::Visibility::External) { - typeError(_typeName.location(), "Only external function types can be payable."); + fatalTypeError(_typeName.location(), "Only external function types can be payable."); return; } @@ -182,7 +188,7 @@ void ReferencesResolver::endVisit(FunctionTypeName const& _typeName) solAssert(t->annotation().type, "Type not set for parameter."); if (!t->annotation().type->canBeUsedExternally(false)) { - typeError(t->location(), "Internal type cannot be used for external function type."); + fatalTypeError(t->location(), "Internal type cannot be used for external function type."); return; } } @@ -300,6 +306,9 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable) if (_variable.annotation().type) return; + if (_variable.isConstant() && !_variable.isStateVariable()) + m_errorReporter.declarationError(_variable.location(), "The \"constant\" keyword can only be used for state variables."); + if (!_variable.typeName()) { // This can still happen in very unusual cases where a developer uses constructs, such as @@ -309,127 +318,92 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable) // after this step. return; } - - TypePointer type; - type = _variable.typeName()->annotation().type; using Location = VariableDeclaration::Location; Location varLoc = _variable.referenceLocation(); DataLocation typeLoc = DataLocation::Memory; - // References are forced to calldata for external function parameters (not return) - // and memory for parameters (also return) of publicly visible functions. - // They default to memory for function parameters and storage for local variables. - // As an exception, "storage" is allowed for library functions. - if (auto ref = dynamic_cast(type.get())) + + set allowedDataLocations = _variable.allowedDataLocations(); + if (!allowedDataLocations.count(varLoc)) { - bool isPointer = true; - if (_variable.isExternalCallableParameter()) + auto locationToString = [](VariableDeclaration::Location _location) -> string { - auto const& contract = dynamic_cast( - *dynamic_cast(*_variable.scope()).scope() - ); - if (contract.isLibrary()) + switch (_location) { - if (varLoc == Location::Memory) - fatalTypeError(_variable.location(), - "Location has to be calldata or storage for external " - "library functions (remove the \"memory\" keyword)." - ); + case Location::Memory: return "\"memory\""; + case Location::Storage: return "\"storage\""; + case Location::CallData: return "\"calldata\""; + case Location::Default: return "none"; } - else - { - // force location of external function parameters (not return) to calldata - if (varLoc != Location::CallData && varLoc != Location::Default) - fatalTypeError(_variable.location(), - "Location has to be calldata for external functions " - "(remove the \"memory\" or \"storage\" keyword)." - ); - } - if (varLoc == Location::Default || varLoc == Location::CallData) - typeLoc = DataLocation::CallData; - else - typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage; - } - else if (_variable.isCallableParameter() && dynamic_cast(*_variable.scope()).isPublic()) - { - auto const& contract = dynamic_cast( - *dynamic_cast(*_variable.scope()).scope() - ); - // force locations of public or external function (return) parameters to memory - if (varLoc != Location::Memory && varLoc != Location::Default && !contract.isLibrary()) - fatalTypeError(_variable.location(), - "Location has to be memory for publicly visible functions " - "(remove the \"storage\" or \"calldata\" keyword)." - ); - if (varLoc == Location::Default || !contract.isLibrary()) - typeLoc = DataLocation::Memory; - else - { - if (varLoc == Location::CallData) - fatalTypeError(_variable.location(), - "Location cannot be calldata for non-external functions " - "(remove the \"calldata\" keyword)." - ); - typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage; - } - } + return {}; + }; + + string errorString; + if (!_variable.hasReferenceOrMappingType()) + errorString = "Data location can only be specified for array, struct or mapping types"; else { - if (_variable.isConstant()) - { - if (varLoc != Location::Default && varLoc != Location::Memory) - fatalTypeError( - _variable.location(), - "Data location has to be \"memory\" (or unspecified) for constants." - ); - typeLoc = DataLocation::Memory; - } - else if (varLoc == Location::Default) - { - if (_variable.isCallableParameter()) - typeLoc = DataLocation::Memory; - else - { - typeLoc = DataLocation::Storage; - if (_variable.isLocalVariable()) - typeError( - _variable.location(), - "Data location must be specified as either \"memory\" or \"storage\"." - ); - } - } + errorString = "Data location must be " + + joinHumanReadable( + allowedDataLocations | boost::adaptors::transformed(locationToString), + ", ", + " or " + ); + if (_variable.isCallableParameter()) + errorString += + " for " + + string(_variable.isReturnParameter() ? "return " : "") + + "parameter in" + + string(_variable.isExternalCallableParameter() ? " external" : "") + + " function"; else - { - switch (varLoc) - { - case Location::Memory: - typeLoc = DataLocation::Memory; - break; - case Location::Storage: - typeLoc = DataLocation::Storage; - break; - case Location::CallData: - fatalTypeError(_variable.location(), - "Variable cannot be declared as \"calldata\" (remove the \"calldata\" keyword)." - ); - break; - default: - solAssert(false, "Unknown data location"); - } - } - isPointer = !_variable.isStateVariable(); + errorString += " for variable"; } + errorString += ", but " + locationToString(varLoc) + " was given."; + typeError(_variable.location(), errorString); + + solAssert(!allowedDataLocations.empty(), ""); + varLoc = *allowedDataLocations.begin(); + } + + // Find correct data location. + if (_variable.isEventParameter()) + { + solAssert(varLoc == Location::Default, ""); + typeLoc = DataLocation::Memory; + } + else if (_variable.isStateVariable()) + { + solAssert(varLoc == Location::Default, ""); + typeLoc = _variable.isConstant() ? DataLocation::Memory : DataLocation::Storage; + } + else if ( + dynamic_cast(_variable.scope()) || + dynamic_cast(_variable.scope()) + ) + // The actual location will later be changed depending on how the type is used. + typeLoc = DataLocation::Storage; + else + switch (varLoc) + { + case Location::Memory: + typeLoc = DataLocation::Memory; + break; + case Location::Storage: + typeLoc = DataLocation::Storage; + break; + case Location::CallData: + typeLoc = DataLocation::CallData; + break; + case Location::Default: + solAssert(!_variable.hasReferenceOrMappingType(), "Data location not properly set."); + } + + TypePointer type = _variable.typeName()->annotation().type; + if (auto ref = dynamic_cast(type.get())) + { + bool isPointer = !_variable.isStateVariable(); type = ref->copyForLocation(typeLoc, isPointer); } - else if (dynamic_cast(type.get())) - { - if (_variable.isLocalVariable() && varLoc != Location::Storage) - typeError( - _variable.location(), - "Data location for mappings must be specified as \"storage\"." - ); - } - else if (varLoc != Location::Default && !ref) - typeError(_variable.location(), "Data location can only be given for array or struct types."); _variable.annotation().type = type; } diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 601a5e7c2..8b609a315 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -630,8 +630,13 @@ bool TypeChecker::visit(FunctionDefinition const& _function) for (ASTPointer const& var: _function.parameters() + _function.returnParameters()) { if ( + type(*var)->category() == Type::Category::Mapping && + !type(*var)->dataStoredIn(DataLocation::Storage) + ) + m_errorReporter.typeError(var->location(), "Mapping types can only have a data location of \"storage\"."); + else if ( !type(*var)->canLiveOutsideStorage() && - !(_function.visibility() <= FunctionDefinition::Visibility::Internal) + _function.visibility() > FunctionDefinition::Visibility::Internal ) m_errorReporter.typeError(var->location(), "Type is required to live outside storage."); if (_function.visibility() >= FunctionDefinition::Visibility::Public && !(type(*var)->interfaceType(isLibraryFunction))) @@ -716,8 +721,6 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) expectType(*_variable.value(), *varType); if (_variable.isConstant()) { - if (!_variable.isStateVariable()) - m_errorReporter.typeError(_variable.location(), "Illegal use of \"constant\" specifier."); if (!_variable.type()->isValueType()) { bool allowed = false; diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 23797d52c..a376e55d2 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -418,6 +418,7 @@ bool VariableDeclaration::isLocalVariable() const { auto s = scope(); return + dynamic_cast(s) || dynamic_cast(s) || dynamic_cast(s) || dynamic_cast(s); @@ -425,14 +426,18 @@ bool VariableDeclaration::isLocalVariable() const bool VariableDeclaration::isCallableParameter() const { - auto const* callable = dynamic_cast(scope()); - if (!callable) - return false; - for (auto const& variable: callable->parameters()) - if (variable.get() == this) - return true; - if (callable->returnParameterList()) - for (auto const& variable: callable->returnParameterList()->parameters()) + if (isReturnParameter()) + return true; + + vector> const* parameters = nullptr; + + if (auto const* funTypeName = dynamic_cast(scope())) + parameters = &funTypeName->parameterTypes(); + else if (auto const* callable = dynamic_cast(scope())) + parameters = &callable->parameters(); + + if (parameters) + for (auto const& variable: *parameters) if (variable.get() == this) return true; return false; @@ -445,11 +450,16 @@ bool VariableDeclaration::isLocalOrReturn() const bool VariableDeclaration::isReturnParameter() const { - auto const* callable = dynamic_cast(scope()); - if (!callable) - return false; - if (callable->returnParameterList()) - for (auto const& variable: callable->returnParameterList()->parameters()) + vector> const* returnParameters = nullptr; + + if (auto const* funTypeName = dynamic_cast(scope())) + returnParameters = &funTypeName->returnParameterTypes(); + else if (auto const* callable = dynamic_cast(scope())) + if (callable->returnParameterList()) + returnParameters = &callable->returnParameterList()->parameters(); + + if (returnParameters) + for (auto const& variable: *returnParameters) if (variable.get() == this) return true; return false; @@ -457,15 +467,88 @@ bool VariableDeclaration::isReturnParameter() const bool VariableDeclaration::isExternalCallableParameter() const { - auto const* callable = dynamic_cast(scope()); - if (!callable || callable->visibility() != Declaration::Visibility::External) + if (!isCallableParameter()) return false; - for (auto const& variable: callable->parameters()) - if (variable.get() == this) - return true; + + if (auto const* callable = dynamic_cast(scope())) + if (callable->visibility() == Declaration::Visibility::External) + return !isReturnParameter(); + return false; } +bool VariableDeclaration::isInternalCallableParameter() const +{ + if (!isCallableParameter()) + return false; + + if (auto const* funTypeName = dynamic_cast(scope())) + return funTypeName->visibility() == Declaration::Visibility::Internal; + else if (auto const* callable = dynamic_cast(scope())) + return callable->visibility() <= Declaration::Visibility::Internal; + return false; +} + +bool VariableDeclaration::isLibraryFunctionParameter() const +{ + if (!isCallableParameter()) + return false; + if (auto const* funDef = dynamic_cast(scope())) + return dynamic_cast(*funDef->scope()).isLibrary(); + else + return false; +} + +bool VariableDeclaration::isEventParameter() const +{ + return dynamic_cast(scope()) != nullptr; +} + +bool VariableDeclaration::hasReferenceOrMappingType() const +{ + solAssert(typeName(), ""); + solAssert(typeName()->annotation().type, "Can only be called after reference resolution"); + TypePointer const& type = typeName()->annotation().type; + return type->category() == Type::Category::Mapping || dynamic_cast(type.get()); +} + +set VariableDeclaration::allowedDataLocations() const +{ + using Location = VariableDeclaration::Location; + + if (!hasReferenceOrMappingType() || isStateVariable() || isEventParameter()) + return set{ Location::Default }; + else if (isStateVariable() && isConstant()) + return set{ Location::Memory }; + else if (isExternalCallableParameter()) + { + set locations{ Location::CallData }; + if (isLibraryFunctionParameter()) + locations.insert(Location::Storage); + return locations; + } + else if (isCallableParameter()) + { + set locations{ Location::Memory }; + if (isInternalCallableParameter() || isLibraryFunctionParameter()) + locations.insert(Location::Storage); + return locations; + } + else if (isLocalVariable()) + { + solAssert(typeName(), ""); + solAssert(typeName()->annotation().type, "Can only be called after reference resolution"); + if (typeName()->annotation().type->category() == Type::Category::Mapping) + return set{ Location::Storage }; + else + // TODO: add Location::Calldata once implemented for local variables. + return set{ Location::Memory, Location::Storage }; + } + else + // Struct members etc. + return set{ Location::Default }; +} + TypePointer VariableDeclaration::type() const { return annotation().type; @@ -580,7 +663,7 @@ bool Literal::passesAddressChecksum() const return dev::passesAddressChecksum(value(), true); } -std::string Literal::getChecksummedAddress() const +string Literal::getChecksummedAddress() const { solAssert(isHexNumber(), "Expected hex number"); /// Pad literal to be a proper hex address. diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 69c6fa056..b953211dd 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -685,6 +685,8 @@ public: virtual bool isLValue() const override; virtual bool isPartOfExternalInterface() const override { return isPublic(); } + /// @returns true iff this variable is the parameter (or return parameter) of a function + /// (or function type name or event) or declared inside a function body. bool isLocalVariable() const; /// @returns true if this variable is a parameter or return parameter of a function. bool isCallableParameter() const; @@ -693,13 +695,27 @@ public: /// @returns true if this variable is a local variable or return parameter. bool isLocalOrReturn() const; /// @returns true if this variable is a parameter (not return parameter) of an external function. + /// This excludes parameters of external function type names. bool isExternalCallableParameter() const; + /// @returns true if this variable is a parameter or return parameter of an internal function + /// or a function type of internal visibility. + bool isInternalCallableParameter() const; + /// @returns true iff this variable is a parameter(or return parameter of a library function + bool isLibraryFunctionParameter() const; /// @returns true if the type of the variable does not need to be specified, i.e. it is declared /// in the body of a function or modifier. + /// @returns true if this variable is a parameter of an event. + bool isEventParameter() const; + /// @returns true if the type of the variable is a reference or mapping type, i.e. + /// array, struct or mapping. These types can take a data location (and often require it). + /// Can only be called after reference resolution. + bool hasReferenceOrMappingType() const; bool isStateVariable() const { return m_isStateVariable; } bool isIndexed() const { return m_isIndexed; } bool isConstant() const { return m_isConstant; } Location referenceLocation() const { return m_location; } + /// @returns a set of allowed storage locations for the variable. + std::set allowedDataLocations() const; virtual TypePointer type() const override;