diff --git a/libsolidity/analysis/DeclarationTypeChecker.cpp b/libsolidity/analysis/DeclarationTypeChecker.cpp index 07e8f6066..abd5a8a42 100644 --- a/libsolidity/analysis/DeclarationTypeChecker.cpp +++ b/libsolidity/analysis/DeclarationTypeChecker.cpp @@ -362,10 +362,7 @@ void DeclarationTypeChecker::endVisit(VariableDeclaration const& _variable) solAssert(varLoc == Location::Unspecified, ""); typeLoc = (_variable.isConstant() || _variable.immutable()) ? DataLocation::Memory : DataLocation::Storage; } - else if ( - dynamic_cast(_variable.scope()) || - dynamic_cast(_variable.scope()) - ) + else if (_variable.isStructMember() || _variable.isEnumMember()) // The actual location will later be changed depending on how the type is used. typeLoc = DataLocation::Storage; else diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 62a1861d1..b1d5d4f78 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -32,6 +32,60 @@ using namespace solidity; using namespace solidity::langutil; using namespace solidity::frontend; +namespace +{ + + +struct TypeComp +{ + bool operator()(Type const* lhs, Type const* rhs) const + { + solAssert(lhs && rhs, ""); + return lhs->richIdentifier() < rhs->richIdentifier(); + } +}; +using TypeSet = std::set; + +class OversizedTypeChecker +{ +public: + OversizedTypeChecker(Type const& _type) + { + visit(_type, true); + } + + vector oversizedTypes() + { + return util::convertContainer>(std::move(m_oversizedTypes)); + } + +private: + void visit(Type const& _type, bool _needsSizeCheck) + { + if (_needsSizeCheck && _type.storageSizeUpperBound() >= bigint(1) << 64) + m_oversizedTypes.insert(&_type); + + if (ArrayType const* array = dynamic_cast(&_type)) + visit(*array->baseType(), array->isDynamicallySized()); + else if (StructType const* structType = dynamic_cast(&_type)) + { + if (m_structsSeen.insert(&structType->structDefinition()).second) + { + for (auto const& member: structType->members(nullptr)) + visit(*member.type, false); + m_structsSeen.erase(&structType->structDefinition()); + } + } + else if (MappingType const* mappingType = dynamic_cast(&_type)) + visit(*mappingType->valueType(), true); + } + + set m_structsSeen; + TypeSet m_oversizedTypes; +}; + + +} /** * Helper class that determines whether a contract's constructor uses inline assembly. */ @@ -73,6 +127,7 @@ private: map m_usesAssembly; }; + StaticAnalyzer::StaticAnalyzer(ErrorReporter& _errorReporter): m_errorReporter(_errorReporter) { @@ -155,6 +210,19 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable) // This is not a no-op, the entry might pre-exist. m_localVarUseCount[make_pair(_variable.id(), &_variable)] += 0; } + + // This excludes struct members on purpose. + if (_variable.isStateVariable() || _variable.referenceLocation() == VariableDeclaration::Location::Storage) + for (Type const* subtype: OversizedTypeChecker{*_variable.annotation().type}.oversizedTypes()) + m_errorReporter.warning( + 7325_error, + _variable.typeName()->location(), + "Type " + + util::escapeAndQuoteString(subtype->canonicalName()) + + " covers a large part of storage and thus makes collisions likely." + " Either use mappings or dynamic arrays and allow their size to be increased only" + " in small quantities per transaction." + ); return true; } diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 762361080..f5a21c9d7 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -526,49 +526,19 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) m_errorReporter.typeError(6744_error, _variable.location(), "Internal or recursive type is not allowed for public state variables."); } - bool isStructMemberDeclaration = dynamic_cast(_variable.scope()) != nullptr; - if (isStructMemberDeclaration) - return false; - - if (auto referenceType = dynamic_cast(varType)) - { - auto result = referenceType->validForLocation(referenceType->location()); - if (result && _variable.isPublicCallableParameter()) - result = referenceType->validForLocation(DataLocation::CallData); - if (!result) + if (!_variable.isStructMember() && !_variable.isEnumMember()) + if (auto referenceType = dynamic_cast(varType)) { - solAssert(!result.message().empty(), "Expected detailed error message"); - m_errorReporter.typeError(1534_error, _variable.location(), result.message()); - return false; + auto result = referenceType->validForLocation(referenceType->location()); + if (result && _variable.isPublicCallableParameter()) + result = referenceType->validForLocation(DataLocation::CallData); + if (!result) + { + solAssert(!result.message().empty(), "Expected detailed error message"); + m_errorReporter.typeError(1534_error, _variable.location(), result.message()); + return false; + } } - } - - if (varType->dataStoredIn(DataLocation::Storage)) - { - auto collisionMessage = [&](string const& variableOrType, bool isVariable) -> string { - return - (isVariable ? "Variable " : "Type ") + - util::escapeAndQuoteString(variableOrType) + - " covers a large part of storage and thus makes collisions likely." - " Either use mappings or dynamic arrays and allow their size to be increased only" - " in small quantities per transaction."; - }; - - if (varType->storageSizeUpperBound() >= bigint(1) << 64) - { - if (_variable.isStateVariable()) - m_errorReporter.warning(3408_error, _variable.location(), collisionMessage(_variable.name(), true)); - else - m_errorReporter.warning( - 2332_error, - _variable.typeName() ? _variable.typeName()->location() : _variable.location(), - collisionMessage(varType->canonicalName(), false) - ); - } - vector oversizedSubtypes = frontend::oversizedSubtypes(*varType); - for (Type const* subtype: oversizedSubtypes) - m_errorReporter.warning(7325_error, _variable.typeName()->location(), collisionMessage(subtype->canonicalName(), false)); - } return false; } diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 34fefeb0a..fa8a5f2c5 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -940,6 +940,8 @@ public: /// Can only be called after reference resolution. bool hasReferenceOrMappingType() const; bool isStateVariable() const { return m_isStateVariable; } + bool isStructMember() const { solAssert(annotation().scope, ""); return dynamic_cast(annotation().scope); } + bool isEnumMember() const { solAssert(annotation().scope, ""); return dynamic_cast(annotation().scope); } bool isIndexed() const { return m_isIndexed; } Mutability mutability() const { return m_mutability; } bool isConstant() const { return m_mutability == Mutability::Constant; } diff --git a/libsolidity/ast/Types.cpp b/libsolidity/ast/Types.cpp index 825fc8d0c..ddc08555a 100644 --- a/libsolidity/ast/Types.cpp +++ b/libsolidity/ast/Types.cpp @@ -53,57 +53,6 @@ using namespace solidity::frontend; namespace { -struct TypeComp -{ - bool operator()(Type const* lhs, Type const* rhs) const - { - solAssert(lhs && rhs, ""); - return lhs->richIdentifier() < rhs->richIdentifier(); - } -}; -using TypeSet = std::set; - -void oversizedSubtypesInner( - Type const& _type, - bool _includeType, - set& _structsSeen, - TypeSet& _oversizedSubtypes -) -{ - switch (_type.category()) - { - case Type::Category::Array: - { - auto const& t = dynamic_cast(_type); - if (_includeType && t.storageSizeUpperBound() >= bigint(1) << 64) - _oversizedSubtypes.insert(&t); - oversizedSubtypesInner(*t.baseType(), t.isDynamicallySized(), _structsSeen, _oversizedSubtypes); - break; - } - case Type::Category::Struct: - { - auto const& t = dynamic_cast(_type); - if (_structsSeen.count(&t.structDefinition())) - return; - if (_includeType && t.storageSizeUpperBound() >= bigint(1) << 64) - _oversizedSubtypes.insert(&t); - _structsSeen.insert(&t.structDefinition()); - for (auto const& m: t.members(nullptr)) - oversizedSubtypesInner(*m.type, false, _structsSeen, _oversizedSubtypes); - _structsSeen.erase(&t.structDefinition()); - break; - } - case Type::Category::Mapping: - { - auto const* valueType = dynamic_cast(_type).valueType(); - oversizedSubtypesInner(*valueType, true, _structsSeen, _oversizedSubtypes); - break; - } - default: - break; - } -} - /// Check whether (_base ** _exp) fits into 4096 bits. bool fitsPrecisionExp(bigint const& _base, bigint const& _exp) { @@ -200,16 +149,6 @@ util::Result transformParametersToExternal(TypePointers const& _pa } -vector solidity::frontend::oversizedSubtypes(frontend::Type const& _type) -{ - set structsSeen; - TypeSet oversized; - oversizedSubtypesInner(_type, false, structsSeen, oversized); - vector res; - copy(oversized.cbegin(), oversized.cend(), back_inserter(res)); - return res; -} - void Type::clearCache() const { m_members.clear(); diff --git a/libsolidity/ast/Types.h b/libsolidity/ast/Types.h index 155fa7f14..707c32670 100644 --- a/libsolidity/ast/Types.h +++ b/libsolidity/ast/Types.h @@ -59,8 +59,6 @@ using BoolResult = util::Result; namespace solidity::frontend { -std::vector oversizedSubtypes(frontend::Type const& _type); - inline rational makeRational(bigint const& _numerator, bigint const& _denominator) { solAssert(_denominator != 0, "division by zero"); diff --git a/test/libsolidity/syntaxTests/iceRegressionTests/oversized_var.sol b/test/libsolidity/syntaxTests/iceRegressionTests/oversized_var.sol index 0e50ae138..62e24cb9a 100644 --- a/test/libsolidity/syntaxTests/iceRegressionTests/oversized_var.sol +++ b/test/libsolidity/syntaxTests/iceRegressionTests/oversized_var.sol @@ -10,6 +10,4 @@ contract b { } // ---- // Warning 2519: (105-110): This declaration shadows an existing declaration. -// Warning 3408: (66-69): Variable "d" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 2332: (105-110): Type "b.c" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. // SyntaxError 1719: (105-114): Use of the "var" keyword is disallowed. Use explicit declaration `struct b.c storage pointer d = ...ยด instead. diff --git a/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol b/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol index 3f7499ca2..57331a168 100644 --- a/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol +++ b/test/libsolidity/syntaxTests/largeTypes/large_storage_array_simple.sol @@ -2,4 +2,4 @@ contract C { uint[2**64] x; } // ---- -// Warning 3408: (17-30): Variable "x" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (17-28): Type "uint256[18446744073709551616]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol b/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol index 285b99deb..facb4b6cf 100644 --- a/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol +++ b/test/libsolidity/syntaxTests/largeTypes/large_storage_arrays_struct.sol @@ -3,4 +3,4 @@ contract C { S[2**20] x; } // ---- -// Warning 3408: (64-74): Variable "x" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (64-72): Type "C.S[1048576]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol b/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol index a29401d94..dc88c8355 100644 --- a/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol +++ b/test/libsolidity/syntaxTests/largeTypes/large_storage_structs.sol @@ -55,14 +55,14 @@ contract C { Q4 q4; } // ---- -// Warning 3408: (106-111): Variable "s0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (171-176): Variable "s1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (106-108): Type "C.S0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (171-173): Type "C.S1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. // Warning 7325: (341-343): Type "C.P[103]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. // Warning 7325: (341-343): Type "C.P[104]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (505-510): Variable "q0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. // Warning 7325: (505-507): Type "uint256[100000000000000000002]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. // Warning 7325: (505-507): Type "uint256[100000000000000000004]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (576-581): Variable "q1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (505-507): Type "C.Q0" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (576-578): Type "C.Q1" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. // Warning 7325: (647-649): Type "uint256[100000000000000000006]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (715-720): Variable "q3" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (715-717): Type "C.Q3" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. // Warning 7325: (783-785): Type "uint256[100000000000000000008]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/oversized_contract.sol b/test/libsolidity/syntaxTests/largeTypes/oversized_contract.sol index 44c7274e6..83eacfed0 100644 --- a/test/libsolidity/syntaxTests/largeTypes/oversized_contract.sol +++ b/test/libsolidity/syntaxTests/largeTypes/oversized_contract.sol @@ -6,5 +6,3 @@ contract C { } // ---- // TypeError 7676: (60-114): Contract too large for storage. -// Warning 3408: (77-91): Variable "a" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (97-111): Variable "b" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/oversized_contract_inheritance.sol b/test/libsolidity/syntaxTests/largeTypes/oversized_contract_inheritance.sol index 24073d4c0..72f3035fe 100644 --- a/test/libsolidity/syntaxTests/largeTypes/oversized_contract_inheritance.sol +++ b/test/libsolidity/syntaxTests/largeTypes/oversized_contract_inheritance.sol @@ -8,5 +8,3 @@ contract D is C { } // ---- // TypeError 7676: (95-134): Contract too large for storage. -// Warning 3408: (77-91): Variable "a" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. -// Warning 3408: (117-131): Variable "b" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/largeTypes/storage_parameter.sol b/test/libsolidity/syntaxTests/largeTypes/storage_parameter.sol index b32815028..12e905860 100644 --- a/test/libsolidity/syntaxTests/largeTypes/storage_parameter.sol +++ b/test/libsolidity/syntaxTests/largeTypes/storage_parameter.sol @@ -3,4 +3,4 @@ contract C { function f(S storage) internal {} } // ---- -// Warning 2332: (64-65): Type "C.S" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (64-65): Type "C.S" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. diff --git a/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol b/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol index 9b4f05c42..218f632ac 100644 --- a/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol +++ b/test/libsolidity/syntaxTests/types/rational_number_array_index_limit.sol @@ -2,4 +2,4 @@ contract c { uint[2**253] data; } // ---- -// Warning 3408: (17-34): Variable "data" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction. +// Warning 7325: (17-29): Type "uint256[14474011154664524427946373126085988481658748083205070504932198000989141204992]" covers a large part of storage and thus makes collisions likely. Either use mappings or dynamic arrays and allow their size to be increased only in small quantities per transaction.