From 4d060ef991c02184814ee2f97344faf646eca3ef Mon Sep 17 00:00:00 2001 From: Mathias Baumann Date: Thu, 14 Mar 2019 17:19:59 +0100 Subject: [PATCH] Merge interfaceType() canBeUsedExternally() And cache the result for expensive calls. --- libsolidity/analysis/ReferencesResolver.cpp | 2 +- libsolidity/analysis/TypeChecker.cpp | 2 +- libsolidity/ast/Types.cpp | 98 ++++++++++++--------- libsolidity/ast/Types.h | 9 +- 4 files changed, 62 insertions(+), 49 deletions(-) diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index cce1a3031..d624a0135 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -213,7 +213,7 @@ void ReferencesResolver::endVisit(FunctionTypeName const& _typeName) for (auto const& t: _typeName.parameterTypes() + _typeName.returnParameterTypes()) { solAssert(t->annotation().type, "Type not set for parameter."); - if (!t->annotation().type->canBeUsedExternally(false)) + if (!t->annotation().type->interfaceType(false)) { fatalTypeError(t->location(), "Internal type cannot be used for external function type."); return; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 4620d82b0..ce73164a1 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -599,7 +599,7 @@ void TypeChecker::endVisit(FunctionTypeName const& _funType) { FunctionType const& fun = dynamic_cast(*_funType.annotation().type); if (fun.kind() == FunctionType::Kind::External) - solAssert(fun.canBeUsedExternally(false), "External function type uses internal types."); + solAssert(fun.interfaceType(false), "External function type uses internal types."); } bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index c803b3b4b..9498d35ae 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -1871,33 +1871,36 @@ TypePointer ArrayType::decodingType() const TypePointer ArrayType::interfaceType(bool _inLibrary) const { - // Note: This has to fulfill canBeUsedExternally(_inLibrary) == !!interfaceType(_inLibrary) + if (_inLibrary && m_interfaceType_library.is_initialized()) + return *m_interfaceType_library; + + if (!_inLibrary && m_interfaceType.is_initialized()) + return *m_interfaceType; + + TypePointer result; + if (_inLibrary && location() == DataLocation::Storage) - return shared_from_this(); - - if (m_arrayKind != ArrayKind::Ordinary) - return this->copyForLocation(DataLocation::Memory, true); - TypePointer baseExt = m_baseType->interfaceType(_inLibrary); - if (!baseExt) - return TypePointer(); - - if (isDynamicallySized()) - return make_shared(DataLocation::Memory, baseExt); - else - return make_shared(DataLocation::Memory, baseExt, m_length); -} - -bool ArrayType::canBeUsedExternally(bool _inLibrary) const -{ - // Note: This has to fulfill canBeUsedExternally(_inLibrary) == !!interfaceType(_inLibrary) - if (_inLibrary && location() == DataLocation::Storage) - return true; + result = shared_from_this(); else if (m_arrayKind != ArrayKind::Ordinary) - return true; - else if (!m_baseType->canBeUsedExternally(_inLibrary)) - return false; + result = this->copyForLocation(DataLocation::Memory, true); else - return true; + { + TypePointer baseExt = m_baseType->interfaceType(_inLibrary); + + if (!baseExt) + result = TypePointer(); + else if (isDynamicallySized()) + result = make_shared(DataLocation::Memory, baseExt); + else + result = make_shared(DataLocation::Memory, baseExt, m_length); + } + + if (_inLibrary) + m_interfaceType_library = result; + else + m_interfaceType = result; + + return result; } u256 ArrayType::memorySize() const @@ -2134,22 +2137,18 @@ MemberList::MemberMap StructType::nativeMembers(ContractDefinition const*) const TypePointer StructType::interfaceType(bool _inLibrary) const { - if (!canBeUsedExternally(_inLibrary)) - return TypePointer(); + if (_inLibrary && m_interfaceType_library.is_initialized()) + return *m_interfaceType_library; - // Has to fulfill canBeUsedExternally(_inLibrary) == !!interfaceType(_inLibrary) - if (_inLibrary && location() == DataLocation::Storage) - return shared_from_this(); - else - return copyForLocation(DataLocation::Memory, true); -} + if (!_inLibrary && m_interfaceType.is_initialized()) + return *m_interfaceType; + + TypePointer result = TypePointer{}; -bool StructType::canBeUsedExternally(bool _inLibrary) const -{ if (_inLibrary && location() == DataLocation::Storage) - return true; + result = shared_from_this(); else if (recursive()) - return false; + result = TypePointer{}; else { // Check that all members have interface types. @@ -2159,17 +2158,32 @@ bool StructType::canBeUsedExternally(bool _inLibrary) const // Also return false if at least one struct member does not have a type. // This might happen, for example, if the type of the member does not exist, // which is reported as an error. + bool allOkay = true; for (auto const& var: m_struct.members()) { // If the struct member does not have a type return false. // A TypeError is expected in this case. if (!var->annotation().type) - return false; - if (!var->annotation().type->canBeUsedExternally(false)) - return false; + { + allOkay = false; + break; + } + if (!var->annotation().type->interfaceType(false)) + { + allOkay = false; + break; + } } + if (allOkay) + result = copyForLocation(DataLocation::Memory, true); } - return true; + + if (_inLibrary) + m_interfaceType_library = result; + else + m_interfaceType = result; + + return result; } TypePointer StructType::copyForLocation(DataLocation _location, bool _isPointer) const @@ -2560,7 +2574,7 @@ FunctionType::FunctionType(FunctionTypeName const& _typeName): solAssert(t->annotation().type, "Type not set for parameter."); if (m_kind == Kind::External) solAssert( - t->annotation().type->canBeUsedExternally(false), + t->annotation().type->interfaceType(false), "Internal type used as parameter for external function." ); m_parameterTypes.push_back(t->annotation().type); @@ -2570,7 +2584,7 @@ FunctionType::FunctionType(FunctionTypeName const& _typeName): solAssert(t->annotation().type, "Type not set for return parameter."); if (m_kind == Kind::External) solAssert( - t->annotation().type->canBeUsedExternally(false), + t->annotation().type->interfaceType(false), "Internal type used as return parameter for external function." ); m_returnParameterTypes.push_back(t->annotation().type); diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index f88232c9b..5d6de0532 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -305,9 +305,6 @@ public: /// @param _inLibrary if set, returns types as used in a library, e.g. struct and contract types /// are returned without modification. virtual TypePointer interfaceType(bool /*_inLibrary*/) const { return TypePointer(); } - /// @returns true iff this type can be passed on via calls (to libraries if _inLibrary is true), - /// should be have identical to !!interfaceType(_inLibrary) but might do optimizations. - virtual bool canBeUsedExternally(bool _inLibrary) const { return !!interfaceType(_inLibrary); } private: /// @returns a member list containing all members added to this type by `using for` directives. @@ -720,7 +717,6 @@ public: TypePointer encodingType() const override; TypePointer decodingType() const override; TypePointer interfaceType(bool _inLibrary) const override; - bool canBeUsedExternally(bool _inLibrary) const override; /// @returns true if this is valid to be stored in calldata bool validForCalldata() const; @@ -753,6 +749,8 @@ private: TypePointer m_baseType; bool m_hasDynamicLength = true; u256 m_length; + mutable boost::optional m_interfaceType; + mutable boost::optional m_interfaceType_library; }; /** @@ -845,7 +843,6 @@ public: return location() == DataLocation::Storage ? std::make_shared(256) : shared_from_this(); } TypePointer interfaceType(bool _inLibrary) const override; - bool canBeUsedExternally(bool _inLibrary) const override; TypePointer copyForLocation(DataLocation _location, bool _isPointer) const override; @@ -875,6 +872,8 @@ private: StructDefinition const& m_struct; /// Cache for the recursive() function. mutable boost::optional m_recursive; + mutable boost::optional m_interfaceType; + mutable boost::optional m_interfaceType_library; }; /**