diff options
Diffstat (limited to 'libsolidity/analysis')
23 files changed, 1473 insertions, 1029 deletions
diff --git a/libsolidity/analysis/ConstantEvaluator.cpp b/libsolidity/analysis/ConstantEvaluator.cpp index 8659bbfd..f9b00927 100644 --- a/libsolidity/analysis/ConstantEvaluator.cpp +++ b/libsolidity/analysis/ConstantEvaluator.cpp @@ -46,7 +46,7 @@ void ConstantEvaluator::endVisit(BinaryOperation const& _operation) m_errorReporter.fatalTypeError( _operation.location(), "Operator " + - string(Token::toString(_operation.getOperator())) + + string(TokenTraits::toString(_operation.getOperator())) + " not compatible with types " + left->toString() + " and " + @@ -54,7 +54,7 @@ void ConstantEvaluator::endVisit(BinaryOperation const& _operation) ); setType( _operation, - Token::isCompareOp(_operation.getOperator()) ? + TokenTraits::isCompareOp(_operation.getOperator()) ? make_shared<BoolType>() : commonType ); diff --git a/libsolidity/analysis/ControlFlowAnalyzer.cpp b/libsolidity/analysis/ControlFlowAnalyzer.cpp index 6edf7986..8a608552 100644 --- a/libsolidity/analysis/ControlFlowAnalyzer.cpp +++ b/libsolidity/analysis/ControlFlowAnalyzer.cpp @@ -28,8 +28,11 @@ bool ControlFlowAnalyzer::analyze(ASTNode const& _astRoot) bool ControlFlowAnalyzer::visit(FunctionDefinition const& _function) { - auto const& functionFlow = m_cfg.functionFlow(_function); - checkUnassignedStorageReturnValues(_function, functionFlow.entry, functionFlow.exit); + if (_function.isImplemented()) + { + auto const& functionFlow = m_cfg.functionFlow(_function); + checkUnassignedStorageReturnValues(_function, functionFlow.entry, functionFlow.exit); + } return false; } @@ -75,7 +78,10 @@ void ControlFlowAnalyzer::checkUnassignedStorageReturnValues( { auto& unassignedAtFunctionEntry = unassigned[_functionEntry]; for (auto const& returnParameter: _function.returnParameterList()->parameters()) - if (returnParameter->type()->dataStoredIn(DataLocation::Storage)) + if ( + returnParameter->type()->dataStoredIn(DataLocation::Storage) || + returnParameter->type()->category() == Type::Category::Mapping + ) unassignedAtFunctionEntry.insert(returnParameter.get()); } @@ -144,12 +150,12 @@ void ControlFlowAnalyzer::checkUnassignedStorageReturnValues( ssl.append("Problematic end of function:", _function.location()); } - m_errorReporter.warning( + m_errorReporter.typeError( returnVal->location(), - "This variable is of storage pointer type and might be returned without assignment. " - "This can cause storage corruption. Assign the variable (potentially from itself) " - "to remove this warning.", - ssl + ssl, + "This variable is of storage pointer type and might be returned without assignment and " + "could be used uninitialized. Assign the variable (potentially from itself) " + "to fix this error." ); } } diff --git a/libsolidity/analysis/ControlFlowBuilder.cpp b/libsolidity/analysis/ControlFlowBuilder.cpp index 35d7687c..5bd39da3 100644 --- a/libsolidity/analysis/ControlFlowBuilder.cpp +++ b/libsolidity/analysis/ControlFlowBuilder.cpp @@ -159,15 +159,14 @@ bool ControlFlowBuilder::visit(WhileStatement const& _whileStatement) { auto afterWhile = newLabel(); auto whileBody = createLabelHere(); + auto condition = newLabel(); { - // Note that "continue" in this case currently indeed jumps to whileBody - // and not to the condition. This is inconsistent with JavaScript and C and - // therefore a bug. This will be fixed in the future (planned for 0.5.0) - // and the Control Flow Graph will have to be adjusted accordingly. - BreakContinueScope scope(*this, afterWhile, whileBody); + BreakContinueScope scope(*this, afterWhile, condition); appendControlFlow(_whileStatement.body()); } + + placeAndConnectLabel(condition); appendControlFlow(_whileStatement.condition()); connect(m_currentNode, whileBody); diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp index 786272e4..cf12a49d 100644 --- a/libsolidity/analysis/DeclarationContainer.cpp +++ b/libsolidity/analysis/DeclarationContainer.cpp @@ -49,16 +49,10 @@ Declaration const* DeclarationContainer::conflictingDeclaration( dynamic_cast<MagicVariableDeclaration const*>(&_declaration) ) { - // check that all other declarations with the same name are functions or a public state variable or events. - // And then check that the signatures are different. + // check that all other declarations are of the same kind (in which + // case the type checker will ensure that the signatures are different) for (Declaration const* declaration: declarations) { - if (auto variableDeclaration = dynamic_cast<VariableDeclaration const*>(declaration)) - { - if (variableDeclaration->isStateVariable() && !variableDeclaration->isConstant() && variableDeclaration->isPublic()) - continue; - return declaration; - } if ( dynamic_cast<FunctionDefinition const*>(&_declaration) && !dynamic_cast<FunctionDefinition const*>(declaration) @@ -96,6 +90,11 @@ void DeclarationContainer::activateVariable(ASTString const& _name) m_invisibleDeclarations.erase(_name); } +bool DeclarationContainer::isInvisible(ASTString const& _name) const +{ + return m_invisibleDeclarations.count(_name); +} + bool DeclarationContainer::registerDeclaration( Declaration const& _declaration, ASTString const* _name, @@ -138,20 +137,23 @@ vector<Declaration const*> DeclarationContainer::resolveName(ASTString const& _n vector<ASTString> DeclarationContainer::similarNames(ASTString const& _name) const { - static size_t const MAXIMUM_EDIT_DISTANCE = 2; - vector<ASTString> similar; + // because the function below has quadratic runtime - it will not magically improve once a better algorithm is discovered ;) + // since 80 is the suggested line length limit, we use 80^2 as length threshold + static size_t const MAXIMUM_LENGTH_THRESHOLD = 80 * 80; + vector<ASTString> similar; + size_t maximumEditDistance = _name.size() > 3 ? 2 : _name.size() / 2; for (auto const& declaration: m_declarations) { string const& declarationName = declaration.first; - if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE)) + if (stringWithinDistance(_name, declarationName, maximumEditDistance, MAXIMUM_LENGTH_THRESHOLD)) similar.push_back(declarationName); } for (auto const& declaration: m_invisibleDeclarations) { string const& declarationName = declaration.first; - if (stringWithinDistance(_name, declarationName, MAXIMUM_EDIT_DISTANCE)) + if (stringWithinDistance(_name, declarationName, maximumEditDistance, MAXIMUM_LENGTH_THRESHOLD)) similar.push_back(declarationName); } diff --git a/libsolidity/analysis/DeclarationContainer.h b/libsolidity/analysis/DeclarationContainer.h index e4b3320a..9d7a17a3 100644 --- a/libsolidity/analysis/DeclarationContainer.h +++ b/libsolidity/analysis/DeclarationContainer.h @@ -58,10 +58,13 @@ public: /// @returns whether declaration is valid, and if not also returns previous declaration. Declaration const* conflictingDeclaration(Declaration const& _declaration, ASTString const* _name = nullptr) const; - /// Activates a previously inactive (invisible) variable. To be used in C99 scpoing for + /// Activates a previously inactive (invisible) variable. To be used in C99 scoping for /// VariableDeclarationStatements. void activateVariable(ASTString const& _name); + /// @returns true if declaration is currently invisible. + bool isInvisible(ASTString const& _name) const; + /// @returns existing declaration names similar to @a _name. /// Searches this and all parent containers. std::vector<ASTString> similarNames(ASTString const& _name) const; diff --git a/libsolidity/analysis/DocStringAnalyser.cpp b/libsolidity/analysis/DocStringAnalyser.cpp index b3fb5258..c1b97def 100644 --- a/libsolidity/analysis/DocStringAnalyser.cpp +++ b/libsolidity/analysis/DocStringAnalyser.cpp @@ -48,7 +48,10 @@ bool DocStringAnalyser::visit(ContractDefinition const& _contract) bool DocStringAnalyser::visit(FunctionDefinition const& _function) { - handleCallable(_function, _function, _function.annotation()); + if (_function.isConstructor()) + handleConstructor(_function, _function, _function.annotation()); + else + handleCallable(_function, _function, _function.annotation()); return true; } @@ -66,15 +69,11 @@ bool DocStringAnalyser::visit(EventDefinition const& _event) return true; } -void DocStringAnalyser::handleCallable( +void DocStringAnalyser::checkParameters( CallableDeclaration const& _callable, - Documented const& _node, DocumentedAnnotation& _annotation ) { - static const set<string> validTags = set<string>{"author", "dev", "notice", "return", "param"}; - parseDocStrings(_node, _annotation, validTags, "functions"); - set<string> validParams; for (auto const& p: _callable.parameters()) validParams.insert(p->name()); @@ -89,6 +88,29 @@ void DocStringAnalyser::handleCallable( i->second.paramName + "\" not found in the parameter list of the function." ); + +} + +void DocStringAnalyser::handleConstructor( + CallableDeclaration const& _callable, + Documented const& _node, + DocumentedAnnotation& _annotation +) +{ + static const set<string> validTags = set<string>{"author", "dev", "notice", "param"}; + parseDocStrings(_node, _annotation, validTags, "constructor"); + checkParameters(_callable, _annotation); +} + +void DocStringAnalyser::handleCallable( + CallableDeclaration const& _callable, + Documented const& _node, + DocumentedAnnotation& _annotation +) +{ + static const set<string> validTags = set<string>{"author", "dev", "notice", "return", "param"}; + parseDocStrings(_node, _annotation, validTags, "functions"); + checkParameters(_callable, _annotation); } void DocStringAnalyser::parseDocStrings( diff --git a/libsolidity/analysis/DocStringAnalyser.h b/libsolidity/analysis/DocStringAnalyser.h index 158b4060..5d339428 100644 --- a/libsolidity/analysis/DocStringAnalyser.h +++ b/libsolidity/analysis/DocStringAnalyser.h @@ -48,6 +48,17 @@ private: virtual bool visit(ModifierDefinition const& _modifier) override; virtual bool visit(EventDefinition const& _event) override; + void checkParameters( + CallableDeclaration const& _callable, + DocumentedAnnotation& _annotation + ); + + void handleConstructor( + CallableDeclaration const& _callable, + Documented const& _node, + DocumentedAnnotation& _annotation + ); + void handleCallable( CallableDeclaration const& _callable, Documented const& _node, diff --git a/libsolidity/analysis/GlobalContext.cpp b/libsolidity/analysis/GlobalContext.cpp index 756bb540..cba2655c 100644 --- a/libsolidity/analysis/GlobalContext.cpp +++ b/libsolidity/analysis/GlobalContext.cpp @@ -42,7 +42,7 @@ m_magicVariables(vector<shared_ptr<MagicVariableDeclaration const>>{ make_shared<MagicVariableDeclaration>("blockhash", make_shared<FunctionType>(strings{"uint256"}, strings{"bytes32"}, FunctionType::Kind::BlockHash, false, StateMutability::View)), make_shared<MagicVariableDeclaration>("ecrecover", make_shared<FunctionType>(strings{"bytes32", "uint8", "bytes32", "bytes32"}, strings{"address"}, FunctionType::Kind::ECRecover, false, StateMutability::Pure)), make_shared<MagicVariableDeclaration>("gasleft", make_shared<FunctionType>(strings(), strings{"uint256"}, FunctionType::Kind::GasLeft, false, StateMutability::View)), - make_shared<MagicVariableDeclaration>("keccak256", make_shared<FunctionType>(strings(), strings{"bytes32"}, FunctionType::Kind::SHA3, true, StateMutability::Pure)), + make_shared<MagicVariableDeclaration>("keccak256", make_shared<FunctionType>(strings{"bytes memory"}, strings{"bytes32"}, FunctionType::Kind::KECCAK256, false, StateMutability::Pure)), make_shared<MagicVariableDeclaration>("log0", make_shared<FunctionType>(strings{"bytes32"}, strings{}, FunctionType::Kind::Log0)), make_shared<MagicVariableDeclaration>("log1", make_shared<FunctionType>(strings{"bytes32", "bytes32"}, strings{}, FunctionType::Kind::Log1)), make_shared<MagicVariableDeclaration>("log2", make_shared<FunctionType>(strings{"bytes32", "bytes32", "bytes32"}, strings{}, FunctionType::Kind::Log2)), @@ -55,11 +55,11 @@ m_magicVariables(vector<shared_ptr<MagicVariableDeclaration const>>{ make_shared<MagicVariableDeclaration>("require", make_shared<FunctionType>(strings{"bool", "string memory"}, strings{}, FunctionType::Kind::Require, false, StateMutability::Pure)), make_shared<MagicVariableDeclaration>("revert", make_shared<FunctionType>(strings(), strings(), FunctionType::Kind::Revert, false, StateMutability::Pure)), make_shared<MagicVariableDeclaration>("revert", make_shared<FunctionType>(strings{"string memory"}, strings(), FunctionType::Kind::Revert, false, StateMutability::Pure)), - make_shared<MagicVariableDeclaration>("ripemd160", make_shared<FunctionType>(strings(), strings{"bytes20"}, FunctionType::Kind::RIPEMD160, true, StateMutability::Pure)), - make_shared<MagicVariableDeclaration>("selfdestruct", make_shared<FunctionType>(strings{"address"}, strings{}, FunctionType::Kind::Selfdestruct)), - make_shared<MagicVariableDeclaration>("sha256", make_shared<FunctionType>(strings(), strings{"bytes32"}, FunctionType::Kind::SHA256, true, StateMutability::Pure)), - make_shared<MagicVariableDeclaration>("sha3", make_shared<FunctionType>(strings(), strings{"bytes32"}, FunctionType::Kind::SHA3, true, StateMutability::Pure)), - make_shared<MagicVariableDeclaration>("suicide", make_shared<FunctionType>(strings{"address"}, strings{}, FunctionType::Kind::Selfdestruct)), + make_shared<MagicVariableDeclaration>("ripemd160", make_shared<FunctionType>(strings{"bytes memory"}, strings{"bytes20"}, FunctionType::Kind::RIPEMD160, false, StateMutability::Pure)), + make_shared<MagicVariableDeclaration>("selfdestruct", make_shared<FunctionType>(strings{"address payable"}, strings{}, FunctionType::Kind::Selfdestruct)), + make_shared<MagicVariableDeclaration>("sha256", make_shared<FunctionType>(strings{"bytes memory"}, strings{"bytes32"}, FunctionType::Kind::SHA256, false, StateMutability::Pure)), + make_shared<MagicVariableDeclaration>("sha3", make_shared<FunctionType>(strings{"bytes memory"}, strings{"bytes32"}, FunctionType::Kind::KECCAK256, false, StateMutability::Pure)), + make_shared<MagicVariableDeclaration>("suicide", make_shared<FunctionType>(strings{"address payable"}, strings{}, FunctionType::Kind::Selfdestruct)), make_shared<MagicVariableDeclaration>("tx", make_shared<MagicType>(MagicType::Kind::Transaction)) }) { diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 0a356f04..b452a49a 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -54,11 +54,10 @@ NameAndTypeResolver::NameAndTypeResolver( bool NameAndTypeResolver::registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope) { - bool useC99Scoping = _sourceUnit.annotation().experimentalFeatures.count(ExperimentalFeature::V050); // The helper registers all declarations in m_scopes as a side-effect of its construction. try { - DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, useC99Scoping, m_errorReporter, _currentScope); + DeclarationRegistrationHelper registrar(m_scopes, _sourceUnit, m_errorReporter, _currentScope); } catch (FatalError const&) { @@ -157,7 +156,13 @@ bool NameAndTypeResolver::updateDeclaration(Declaration const& _declaration) void NameAndTypeResolver::activateVariable(string const& _name) { solAssert(m_currentScope, ""); - m_currentScope->activateVariable(_name); + // Scoped local variables are invisible before activation. + // When a local variable is activated, its name is removed + // from a scope's invisible variables. + // This is used to avoid activation of variables of same name + // in the same scope (an error is returned). + if (m_currentScope->isInvisible(_name)) + m_currentScope->activateVariable(_name); } vector<Declaration const*> NameAndTypeResolver::resolveName(ASTString const& _name, ASTNode const* _scope) const @@ -226,7 +231,7 @@ vector<Declaration const*> NameAndTypeResolver::cleanedDeclarations( shared_ptr<FunctionType const> newFunctionType { d->functionType(false) }; if (!newFunctionType) newFunctionType = d->functionType(true); - return newFunctionType && functionType->hasEqualArgumentTypes(*newFunctionType); + return newFunctionType && functionType->hasEqualParameterTypes(*newFunctionType); } )) uniqueFunctions.push_back(declaration); @@ -290,10 +295,7 @@ bool NameAndTypeResolver::resolveNamesAndTypesInternal(ASTNode& _node, bool _res { setScope(contract); if (!resolveNamesAndTypes(*node, false)) - { success = false; - break; - } } if (!success) @@ -449,11 +451,9 @@ string NameAndTypeResolver::similarNameSuggestions(ASTString const& _name) const DeclarationRegistrationHelper::DeclarationRegistrationHelper( map<ASTNode const*, shared_ptr<DeclarationContainer>>& _scopes, ASTNode& _astRoot, - bool _useC99Scoping, ErrorReporter& _errorReporter, ASTNode const* _currentScope ): - m_useC99Scoping(_useC99Scoping), m_scopes(_scopes), m_currentScope(_currentScope), m_errorReporter(_errorReporter) @@ -626,32 +626,39 @@ 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); - if (m_useC99Scoping) - enterNewSubScope(_block); + enterNewSubScope(_block); return true; } void DeclarationRegistrationHelper::endVisit(Block&) { - if (m_useC99Scoping) - closeCurrentScope(); + closeCurrentScope(); } bool DeclarationRegistrationHelper::visit(ForStatement& _for) { _for.setScope(m_currentScope); - if (m_useC99Scoping) - enterNewSubScope(_for); + enterNewSubScope(_for); return true; } void DeclarationRegistrationHelper::endVisit(ForStatement&) { - if (m_useC99Scoping) - closeCurrentScope(); + closeCurrentScope(); } void DeclarationRegistrationHelper::endVisit(VariableDeclarationStatement& _variableDeclarationStatement) @@ -711,14 +718,9 @@ void DeclarationRegistrationHelper::registerDeclaration(Declaration& _declaratio dynamic_cast<EventDefinition const*>(m_currentScope) ) warnAboutShadowing = false; - // Do not warn about the constructor shadowing the contract. - if (auto fun = dynamic_cast<FunctionDefinition const*>(&_declaration)) - if (fun->isConstructor()) - warnAboutShadowing = false; - // Register declaration as inactive if we are in block scope and C99 mode. + // Register declaration as inactive if we are in block scope. bool inactive = - m_useC99Scoping && (dynamic_cast<Block const*>(m_currentScope) || dynamic_cast<ForStatement const*>(m_currentScope)); registerDeclaration(*m_scopes[m_currentScope], _declaration, nullptr, nullptr, warnAboutShadowing, inactive, m_errorReporter); diff --git a/libsolidity/analysis/NameAndTypeResolver.h b/libsolidity/analysis/NameAndTypeResolver.h index 3d10fbd8..a72c21e3 100644 --- a/libsolidity/analysis/NameAndTypeResolver.h +++ b/libsolidity/analysis/NameAndTypeResolver.h @@ -69,7 +69,7 @@ public: /// that create their own scope. /// @returns false in case of error. bool updateDeclaration(Declaration const& _declaration); - /// Activates a previously inactive (invisible) variable. To be used in C99 scpoing for + /// Activates a previously inactive (invisible) variable. To be used in C99 scoping for /// VariableDeclarationStatements. void activateVariable(std::string const& _name); @@ -142,7 +142,6 @@ public: DeclarationRegistrationHelper( std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& _scopes, ASTNode& _astRoot, - bool _useC99Scoping, ErrorReporter& _errorReporter, ASTNode const* _currentScope = nullptr ); @@ -172,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; @@ -190,7 +191,6 @@ private: /// @returns the canonical name of the current scope. std::string currentCanonicalName() const; - bool m_useC99Scoping = false; std::map<ASTNode const*, std::shared_ptr<DeclarationContainer>>& m_scopes; ASTNode const* m_currentScope = nullptr; VariableScope* m_currentFunction = nullptr; diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp index 19d0b708..240d7973 100644 --- a/libsolidity/analysis/PostTypeChecker.cpp +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -91,8 +91,11 @@ bool PostTypeChecker::visit(Identifier const& _identifier) VariableDeclaration const* PostTypeChecker::findCycle(VariableDeclaration const& _startingFrom) { - auto visitor = [&](VariableDeclaration const& _variable, CycleDetector<VariableDeclaration>& _cycleDetector) + auto visitor = [&](VariableDeclaration const& _variable, CycleDetector<VariableDeclaration>& _cycleDetector, size_t _depth) { + if (_depth >= 256) + m_errorReporter.fatalDeclarationError(_variable.location(), "Variable definition exhausting cyclic dependency validator."); + // Iterating through the dependencies needs to be deterministic and thus cannot // depend on the memory layout. // Because of that, we sort by AST node id. diff --git a/libsolidity/analysis/ReferencesResolver.cpp b/libsolidity/analysis/ReferencesResolver.cpp index f91eaf6e..2adc8e77 100644 --- a/libsolidity/analysis/ReferencesResolver.cpp +++ b/libsolidity/analysis/ReferencesResolver.cpp @@ -30,7 +30,10 @@ #include <libsolidity/inlineasm/AsmData.h> #include <libsolidity/interface/ErrorReporter.h> +#include <libdevcore/StringUtils.h> + #include <boost/algorithm/string.hpp> +#include <boost/range/adaptor/transformed.hpp> using namespace std; using namespace dev; @@ -47,10 +50,7 @@ bool ReferencesResolver::visit(Block const& _block) { if (!m_resolveInsideCode) return false; - m_experimental050Mode = _block.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - // C99-scoped variables - if (m_experimental050Mode) - m_resolver.setScope(&_block); + m_resolver.setScope(&_block); return true; } @@ -59,19 +59,14 @@ void ReferencesResolver::endVisit(Block const& _block) if (!m_resolveInsideCode) return; - // C99-scoped variables - if (m_experimental050Mode) - m_resolver.setScope(_block.scope()); + m_resolver.setScope(_block.scope()); } bool ReferencesResolver::visit(ForStatement const& _for) { if (!m_resolveInsideCode) return false; - m_experimental050Mode = _for.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - // C99-scoped variables - if (m_experimental050Mode) - m_resolver.setScope(&_for); + m_resolver.setScope(&_for); return true; } @@ -79,18 +74,16 @@ void ReferencesResolver::endVisit(ForStatement const& _for) { if (!m_resolveInsideCode) return; - if (m_experimental050Mode) - m_resolver.setScope(_for.scope()); + m_resolver.setScope(_for.scope()); } void ReferencesResolver::endVisit(VariableDeclarationStatement const& _varDeclStatement) { if (!m_resolveInsideCode) return; - if (m_experimental050Mode) - for (auto const& var: _varDeclStatement.declarations()) - if (var) - m_resolver.activateVariable(var->name()); + for (auto const& var: _varDeclStatement.declarations()) + if (var) + m_resolver.activateVariable(var->name()); } bool ReferencesResolver::visit(Identifier const& _identifier) @@ -99,9 +92,14 @@ bool ReferencesResolver::visit(Identifier const& _identifier) if (declarations.empty()) { string suggestions = m_resolver.similarNameSuggestions(_identifier.name()); - string errorMessage = - "Undeclared identifier." + - (suggestions.empty()? "": " Did you mean " + std::move(suggestions) + "?"); + string errorMessage = "Undeclared identifier."; + if (!suggestions.empty()) + { + if ("\"" + _identifier.name() + "\"" == suggestions) + errorMessage += " " + std::move(suggestions) + " is not (or not yet) visible at this point."; + else + errorMessage += " Did you mean " + std::move(suggestions) + "?"; + } declarationError(_identifier.location(), errorMessage); } else if (declarations.size() == 1) @@ -114,7 +112,28 @@ bool ReferencesResolver::visit(Identifier const& _identifier) bool ReferencesResolver::visit(ElementaryTypeName const& _typeName) { - _typeName.annotation().type = Type::fromElementaryTypeName(_typeName.typeName()); + if (!_typeName.annotation().type) + { + _typeName.annotation().type = Type::fromElementaryTypeName(_typeName.typeName()); + if (_typeName.stateMutability().is_initialized()) + { + // for non-address types this was already caught by the parser + solAssert(_typeName.annotation().type->category() == Type::Category::Address, ""); + switch(*_typeName.stateMutability()) + { + case StateMutability::Payable: + case StateMutability::NonPayable: + _typeName.annotation().type = make_shared<AddressType>(*_typeName.stateMutability()); + break; + default: + m_errorReporter.typeError( + _typeName.location(), + "Address types can only be payable or non-payable." + ); + break; + } + } + } return true; } @@ -147,7 +166,7 @@ void ReferencesResolver::endVisit(UserDefinedTypeName const& _typeName) Declaration const* declaration = m_resolver.pathFromCurrentScope(_typeName.namePath()); if (!declaration) { - declarationError(_typeName.location(), "Identifier not found or not unique."); + fatalDeclarationError(_typeName.location(), "Identifier not found or not unique."); return; } @@ -160,7 +179,10 @@ void ReferencesResolver::endVisit(UserDefinedTypeName const& _typeName) else if (ContractDefinition const* contract = dynamic_cast<ContractDefinition const*>(declaration)) _typeName.annotation().type = make_shared<ContractType>(*contract); else + { + _typeName.annotation().type = make_shared<TupleType>(); typeError(_typeName.location(), "Name has to refer to a struct, enum or contract."); + } } void ReferencesResolver::endVisit(FunctionTypeName const& _typeName) @@ -171,13 +193,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; } @@ -187,7 +209,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; } } @@ -224,6 +246,8 @@ void ReferencesResolver::endVisit(ArrayTypeName const& _typeName) RationalNumberType const* lengthType = dynamic_cast<RationalNumberType const*>(lengthTypeGeneric.get()); if (!lengthType || !lengthType->mobileType()) fatalTypeError(length->location(), "Invalid array length, expected integer literal or constant expression."); + else if (lengthType->isZero()) + fatalTypeError(length->location(), "Array with zero length specified."); else if (lengthType->isFractional()) fatalTypeError(length->location(), "Array with fractional length specified."); else if (lengthType->isNegative()) @@ -245,26 +269,34 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly) // external references. ErrorList errors; ErrorReporter errorsIgnored(errors); - julia::ExternalIdentifierAccess::Resolver resolver = - [&](assembly::Identifier const& _identifier, julia::IdentifierContext, bool _crossesFunctionBoundary) { - auto declarations = m_resolver.nameFromCurrentScope(_identifier.name); - bool isSlot = boost::algorithm::ends_with(_identifier.name, "_slot"); - bool isOffset = boost::algorithm::ends_with(_identifier.name, "_offset"); + yul::ExternalIdentifierAccess::Resolver resolver = + [&](assembly::Identifier const& _identifier, yul::IdentifierContext, bool _crossesFunctionBoundary) { + auto declarations = m_resolver.nameFromCurrentScope(_identifier.name.str()); + bool isSlot = boost::algorithm::ends_with(_identifier.name.str(), "_slot"); + bool isOffset = boost::algorithm::ends_with(_identifier.name.str(), "_offset"); if (isSlot || isOffset) { // special mode to access storage variables if (!declarations.empty()) // the special identifier exists itself, we should not allow that. return size_t(-1); - string realName = _identifier.name.substr(0, _identifier.name.size() - ( + string realName = _identifier.name.str().substr(0, _identifier.name.str().size() - ( isSlot ? string("_slot").size() : string("_offset").size() )); + if (realName.empty()) + { + declarationError(_identifier.location, "In variable names _slot and _offset can only be used as a suffix."); + return size_t(-1); + } declarations = m_resolver.nameFromCurrentScope(realName); } if (declarations.size() != 1) + { + declarationError(_identifier.location, "Multiple matching identifiers. Resolving overloaded identifiers is not supported."); return size_t(-1); + } if (auto var = dynamic_cast<VariableDeclaration const*>(declarations.front())) if (var->isLocalVariable() && _crossesFunctionBoundary) { @@ -280,7 +312,7 @@ bool ReferencesResolver::visit(InlineAssembly const& _inlineAssembly) // Will be re-generated later with correct information // We use the latest EVM version because we will re-run it anyway. assembly::AsmAnalysisInfo analysisInfo; - boost::optional<Error::Type> errorTypeForLoose = m_experimental050Mode ? Error::Type::SyntaxError : Error::Type::Warning; + boost::optional<Error::Type> errorTypeForLoose = Error::Type::SyntaxError; assembly::AsmAnalyzer(analysisInfo, errorsIgnored, EVMVersion(), errorTypeForLoose, assembly::AsmFlavour::Loose, resolver).analyze(_inlineAssembly.operations()); return false; } @@ -297,112 +329,106 @@ void ReferencesResolver::endVisit(VariableDeclaration const& _variable) if (_variable.annotation().type) return; - TypePointer type; - if (_variable.typeName()) + 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 + // `var a;`, however, such code will have generated errors already. + // However, we cannot blindingly solAssert() for that here, as the TypeChecker (which is + // invoking ReferencesResolver) is generating it, so the error is most likely(!) generated + // after this step. + return; + } + using Location = VariableDeclaration::Location; + Location varLoc = _variable.referenceLocation(); + DataLocation typeLoc = DataLocation::Memory; + + set<Location> allowedDataLocations = _variable.allowedDataLocations(); + if (!allowedDataLocations.count(varLoc)) { - 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<ReferenceType const*>(type.get())) + auto locationToString = [](VariableDeclaration::Location _location) -> string { - bool isPointer = true; - if (_variable.isExternalCallableParameter()) - { - auto const& contract = dynamic_cast<ContractDefinition const&>( - *dynamic_cast<Declaration const&>(*_variable.scope()).scope() - ); - if (contract.isLibrary()) - { - if (varLoc == Location::Memory) - fatalTypeError(_variable.location(), - "Location has to be calldata or storage for external " - "library functions (remove the \"memory\" keyword)." - ); - } - else - { - // force location of external function parameters (not return) to calldata - if (varLoc != Location::Default) - fatalTypeError(_variable.location(), - "Location has to be calldata for external functions " - "(remove the \"memory\" or \"storage\" keyword)." - ); - } - if (varLoc == Location::Default) - typeLoc = DataLocation::CallData; - else - typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage; - } - else if (_variable.isCallableParameter() && dynamic_cast<Declaration const&>(*_variable.scope()).isPublic()) + switch (_location) { - auto const& contract = dynamic_cast<ContractDefinition const&>( - *dynamic_cast<Declaration const&>(*_variable.scope()).scope() - ); - // force locations of public or external function (return) parameters to memory - if (varLoc == Location::Storage && !contract.isLibrary()) - fatalTypeError(_variable.location(), - "Location has to be memory for publicly visible functions " - "(remove the \"storage\" keyword)." - ); - if (varLoc == Location::Default || !contract.isLibrary()) - typeLoc = DataLocation::Memory; - else - typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage; + case Location::Memory: return "\"memory\""; + case Location::Storage: return "\"storage\""; + case Location::CallData: return "\"calldata\""; + case Location::Unspecified: return "none"; } + return {}; + }; + + string errorString; + if (!_variable.hasReferenceOrMappingType()) + errorString = "Data location can only be specified for array, struct or mapping types"; + else + { + 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 - { - if (_variable.isConstant()) - { - if (varLoc != Location::Default && varLoc != Location::Memory) - fatalTypeError( - _variable.location(), - "Storage 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()) - { - if (_variable.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) - typeError( - _variable.location(), - "Storage location must be specified as either \"memory\" or \"storage\"." - ); - else - m_errorReporter.warning( - _variable.location(), - "Variable is declared as a storage pointer. " - "Use an explicit \"storage\" keyword to silence this warning." - ); - } - } - } - else - typeLoc = varLoc == Location::Memory ? DataLocation::Memory : DataLocation::Storage; - isPointer = !_variable.isStateVariable(); - } + errorString += " for variable"; + } + errorString += ", but " + locationToString(varLoc) + " was given."; + typeError(_variable.location(), errorString); - type = ref->copyForLocation(typeLoc, isPointer); + solAssert(!allowedDataLocations.empty(), ""); + varLoc = *allowedDataLocations.begin(); + } + + // Find correct data location. + if (_variable.isEventParameter()) + { + solAssert(varLoc == Location::Unspecified, ""); + typeLoc = DataLocation::Memory; + } + else if (_variable.isStateVariable()) + { + solAssert(varLoc == Location::Unspecified, ""); + typeLoc = _variable.isConstant() ? DataLocation::Memory : DataLocation::Storage; + } + else if ( + dynamic_cast<StructDefinition const*>(_variable.scope()) || + dynamic_cast<EnumDefinition const*>(_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::Unspecified: + solAssert(!_variable.hasReferenceOrMappingType(), "Data location not properly set."); } - else if (varLoc != Location::Default && !ref) - typeError(_variable.location(), "Storage location can only be given for array or struct types."); - _variable.annotation().type = type; + TypePointer type = _variable.typeName()->annotation().type; + if (auto ref = dynamic_cast<ReferenceType const*>(type.get())) + { + bool isPointer = !_variable.isStateVariable(); + type = ref->copyForLocation(typeLoc, isPointer); } - else if (!_variable.canHaveAutoType()) - typeError(_variable.location(), "Explicit type needed."); - // otherwise we have a "var"-declaration whose type is resolved by the first assignment + + _variable.annotation().type = type; } void ReferencesResolver::typeError(SourceLocation const& _location, string const& _description) diff --git a/libsolidity/analysis/ReferencesResolver.h b/libsolidity/analysis/ReferencesResolver.h index 4e8f54b5..24ec4643 100644 --- a/libsolidity/analysis/ReferencesResolver.h +++ b/libsolidity/analysis/ReferencesResolver.h @@ -94,7 +94,6 @@ private: std::vector<ParameterList const*> m_returnParameters; bool const m_resolveInsideCode; bool m_errorOccurred = false; - bool m_experimental050Mode = false; }; } diff --git a/libsolidity/analysis/SemVerHandler.cpp b/libsolidity/analysis/SemVerHandler.cpp index 42186396..64fa17b3 100644 --- a/libsolidity/analysis/SemVerHandler.cpp +++ b/libsolidity/analysis/SemVerHandler.cpp @@ -106,18 +106,22 @@ bool SemVerMatchExpression::MatchComponent::matches(SemVerVersion const& _versio } if (cmp == 0 && !_version.prerelease.empty() && didCompare) cmp = -1; - if (prefix == Token::Assign) + + switch (prefix) + { + case Token::Assign: return cmp == 0; - else if (prefix == Token::LessThan) + case Token::LessThan: return cmp < 0; - else if (prefix == Token::LessThanOrEqual) + case Token::LessThanOrEqual: return cmp <= 0; - else if (prefix == Token::GreaterThan) + case Token::GreaterThan: return cmp > 0; - else if (prefix == Token::GreaterThanOrEqual) + case Token::GreaterThanOrEqual: return cmp >= 0; - else + default: solAssert(false, "Invalid SemVer expression"); + } return false; } } @@ -195,22 +199,23 @@ void SemVerMatchExpressionParser::parseMatchExpression() SemVerMatchExpression::MatchComponent SemVerMatchExpressionParser::parseMatchComponent() { SemVerMatchExpression::MatchComponent component; - Token::Value token = currentToken(); - if ( - token == Token::BitXor || - token == Token::BitNot || - token == Token::LessThan || - token == Token::LessThanOrEqual|| - token == Token::GreaterThan || - token == Token::GreaterThanOrEqual || - token == Token::Assign - ) + Token token = currentToken(); + + switch (token) { + case Token::BitXor: + case Token::BitNot: + case Token::LessThan: + case Token::LessThanOrEqual: + case Token::GreaterThan: + case Token::GreaterThanOrEqual: + case Token::Assign: component.prefix = token; nextToken(); - } - else + break; + default: component.prefix = Token::Assign; + } component.levelsPresent = 0; while (component.levelsPresent < 3) @@ -275,7 +280,7 @@ char SemVerMatchExpressionParser::nextChar() return currentChar(); } -Token::Value SemVerMatchExpressionParser::currentToken() const +Token SemVerMatchExpressionParser::currentToken() const { if (m_pos < m_tokens.size()) return m_tokens[m_pos]; diff --git a/libsolidity/analysis/SemVerHandler.h b/libsolidity/analysis/SemVerHandler.h index 76b70c5b..03a557c5 100644 --- a/libsolidity/analysis/SemVerHandler.h +++ b/libsolidity/analysis/SemVerHandler.h @@ -61,7 +61,7 @@ struct SemVerMatchExpression struct MatchComponent { /// Prefix from < > <= >= ~ ^ - Token::Value prefix = Token::Illegal; + Token prefix = Token::Illegal; /// Version, where unsigned(-1) in major, minor or patch denotes '*', 'x' or 'X' SemVerVersion version; /// Whether we have 1, 1.2 or 1.2.4 @@ -81,7 +81,7 @@ struct SemVerMatchExpression class SemVerMatchExpressionParser { public: - SemVerMatchExpressionParser(std::vector<Token::Value> const& _tokens, std::vector<std::string> const& _literals): + SemVerMatchExpressionParser(std::vector<Token> const& _tokens, std::vector<std::string> const& _literals): m_tokens(_tokens), m_literals(_literals) {} SemVerMatchExpression parse(); @@ -95,10 +95,10 @@ private: char currentChar() const; char nextChar(); - Token::Value currentToken() const; + Token currentToken() const; void nextToken(); - std::vector<Token::Value> m_tokens; + std::vector<Token> m_tokens; std::vector<std::string> m_literals; unsigned m_pos = 0; diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 00a581d0..487a5cca 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -51,22 +51,11 @@ void StaticAnalyzer::endVisit(ContractDefinition const&) bool StaticAnalyzer::visit(FunctionDefinition const& _function) { - const bool isInterface = m_currentContract->contractKind() == ContractDefinition::ContractKind::Interface; - - if (_function.noVisibilitySpecified()) - m_errorReporter.warning( - _function.location(), - "No visibility specified. Defaulting to \"" + - Declaration::visibilityToString(_function.visibility()) + - "\". " + - (isInterface ? "In interfaces it defaults to external." : "") - ); if (_function.isImplemented()) m_currentFunction = &_function; else solAssert(!m_currentFunction, ""); solAssert(m_localVarUseCount.empty(), ""); - m_nonPayablePublic = _function.isPublic() && !_function.isPayable(); m_constructor = _function.isConstructor(); return true; } @@ -74,7 +63,6 @@ bool StaticAnalyzer::visit(FunctionDefinition const& _function) void StaticAnalyzer::endVisit(FunctionDefinition const&) { m_currentFunction = nullptr; - m_nonPayablePublic = false; m_constructor = false; for (auto const& var: m_localVarUseCount) if (var.second == 0) @@ -150,61 +138,27 @@ bool StaticAnalyzer::visit(ExpressionStatement const& _statement) bool StaticAnalyzer::visit(MemberAccess const& _memberAccess) { - bool const v050 = m_currentContract->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - if (MagicType const* type = dynamic_cast<MagicType const*>(_memberAccess.expression().annotation().type.get())) { if (type->kind() == MagicType::Kind::Message && _memberAccess.memberName() == "gas") - { - if (v050) - m_errorReporter.typeError( - _memberAccess.location(), - "\"msg.gas\" has been deprecated in favor of \"gasleft()\"" - ); - else - m_errorReporter.warning( - _memberAccess.location(), - "\"msg.gas\" has been deprecated in favor of \"gasleft()\"" - ); - } - if (type->kind() == MagicType::Kind::Block && _memberAccess.memberName() == "blockhash") - { - if (v050) - m_errorReporter.typeError( - _memberAccess.location(), - "\"block.blockhash()\" has been deprecated in favor of \"blockhash()\"" - ); - else - m_errorReporter.warning( - _memberAccess.location(), - "\"block.blockhash()\" has been deprecated in favor of \"blockhash()\"" - ); - } + m_errorReporter.typeError( + _memberAccess.location(), + "\"msg.gas\" has been deprecated in favor of \"gasleft()\"" + ); + else if (type->kind() == MagicType::Kind::Block && _memberAccess.memberName() == "blockhash") + m_errorReporter.typeError( + _memberAccess.location(), + "\"block.blockhash()\" has been deprecated in favor of \"blockhash()\"" + ); } - if (m_nonPayablePublic && !m_library) - if (MagicType const* type = dynamic_cast<MagicType const*>(_memberAccess.expression().annotation().type.get())) - if (type->kind() == MagicType::Kind::Message && _memberAccess.memberName() == "value") - m_errorReporter.warning( - _memberAccess.location(), - "\"msg.value\" used in non-payable function. Do you want to add the \"payable\" modifier to this function?" - ); - if (_memberAccess.memberName() == "callcode") if (auto const* type = dynamic_cast<FunctionType const*>(_memberAccess.annotation().type.get())) if (type->kind() == FunctionType::Kind::BareCallCode) - { - if (v050) - m_errorReporter.typeError( - _memberAccess.location(), - "\"callcode\" has been deprecated in favour of \"delegatecall\"." - ); - else - m_errorReporter.warning( - _memberAccess.location(), - "\"callcode\" has been deprecated in favour of \"delegatecall\"." - ); - } + m_errorReporter.typeError( + _memberAccess.location(), + "\"callcode\" has been deprecated in favour of \"delegatecall\"." + ); if (m_constructor) { diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index 2a62e391..7f5c743a 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -75,9 +75,6 @@ private: /// Flag that indicates whether the current contract definition is a library. bool m_library = false; - /// Flag that indicates whether a public function does not contain the "payable" modifier. - bool m_nonPayablePublic = false; - /// Number of uses of each (named) local variable in a function, counter is initialized with zero. /// Pairs of AST ids and pointers are used as keys to ensure a deterministic order /// when traversing. diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index 396058f4..3f9f8373 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -22,6 +22,10 @@ #include <libsolidity/analysis/SemVerHandler.h> #include <libsolidity/interface/ErrorReporter.h> #include <libsolidity/interface/Version.h> +#include <boost/algorithm/cxx11/all_of.hpp> + +#include <boost/algorithm/string.hpp> +#include <string> using namespace std; using namespace dev; @@ -49,7 +53,7 @@ void SyntaxChecker::endVisit(SourceUnit const& _sourceUnit) SemVerVersion recommendedVersion{string(VersionString)}; if (!recommendedVersion.isPrerelease()) errorString += - "Consider adding \"pragma solidity ^" + + " Consider adding \"pragma solidity ^" + to_string(recommendedVersion.major()) + string(".") + to_string(recommendedVersion.minor()) + @@ -72,7 +76,7 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma) { solAssert(m_sourceUnit, ""); vector<string> literals(_pragma.literals().begin() + 1, _pragma.literals().end()); - if (literals.size() == 0) + if (literals.empty()) m_errorReporter.syntaxError( _pragma.location(), "Experimental feature name is missing." @@ -102,7 +106,7 @@ bool SyntaxChecker::visit(PragmaDirective const& _pragma) } else if (_pragma.literals()[0] == "solidity") { - vector<Token::Value> tokens(_pragma.tokens().begin() + 1, _pragma.tokens().end()); + vector<Token> tokens(_pragma.tokens().begin() + 1, _pragma.tokens().end()); vector<string> literals(_pragma.literals().begin() + 1, _pragma.literals().end()); SemVerMatchExpressionParser parser(tokens, literals); auto matchExpression = parser.parse(); @@ -134,9 +138,25 @@ void SyntaxChecker::endVisit(ModifierDefinition const& _modifier) m_placeholderFound = false; } -bool SyntaxChecker::visit(WhileStatement const&) +void SyntaxChecker::checkSingleStatementVariableDeclaration(ASTNode const& _statement) +{ + auto varDecl = dynamic_cast<VariableDeclarationStatement const*>(&_statement); + if (varDecl) + m_errorReporter.syntaxError(_statement.location(), "Variable declarations can only be used inside blocks."); +} + +bool SyntaxChecker::visit(IfStatement const& _ifStatement) +{ + checkSingleStatementVariableDeclaration(_ifStatement.trueStatement()); + if (Statement const* _statement = _ifStatement.falseStatement()) + checkSingleStatementVariableDeclaration(*_statement); + return true; +} + +bool SyntaxChecker::visit(WhileStatement const& _whileStatement) { m_inLoopDepth++; + checkSingleStatementVariableDeclaration(_whileStatement.body()); return true; } @@ -145,9 +165,10 @@ void SyntaxChecker::endVisit(WhileStatement const&) m_inLoopDepth--; } -bool SyntaxChecker::visit(ForStatement const&) +bool SyntaxChecker::visit(ForStatement const& _forStatement) { m_inLoopDepth++; + checkSingleStatementVariableDeclaration(_forStatement.body()); return true; } @@ -174,33 +195,58 @@ bool SyntaxChecker::visit(Break const& _breakStatement) bool SyntaxChecker::visit(Throw const& _throwStatement) { - bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050); - - if (v050) - m_errorReporter.syntaxError( - _throwStatement.location(), - "\"throw\" is deprecated in favour of \"revert()\", \"require()\" and \"assert()\"." - ); - else - m_errorReporter.warning( - _throwStatement.location(), - "\"throw\" is deprecated in favour of \"revert()\", \"require()\" and \"assert()\"." - ); + m_errorReporter.syntaxError( + _throwStatement.location(), + "\"throw\" is deprecated in favour of \"revert()\", \"require()\" and \"assert()\"." + ); return true; } -bool SyntaxChecker::visit(UnaryOperation const& _operation) +bool SyntaxChecker::visit(Literal const& _literal) { - bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050); + if (_literal.token() != Token::Number) + return true; - if (_operation.getOperator() == Token::Add) + ASTString const& value = _literal.value(); + solAssert(!value.empty(), ""); + + // Generic checks no matter what base this number literal is of: + if (value.back() == '_') { - if (v050) - m_errorReporter.syntaxError(_operation.location(), "Use of unary + is deprecated."); - else - m_errorReporter.warning(_operation.location(), "Use of unary + is deprecated."); + m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No trailing underscores allowed."); + return true; + } + + if (value.find("__") != ASTString::npos) + { + m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. Only one consecutive underscores between digits allowed."); + return true; + } + + if (!_literal.isHexNumber()) // decimal literal + { + if (value.find("._") != ASTString::npos) + m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscores in front of the fraction part allowed."); + + if (value.find("_.") != ASTString::npos) + m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscores in front of the fraction part allowed."); + + if (value.find("_e") != ASTString::npos) + m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscore at the end of the mantissa allowed."); + + if (value.find("e_") != ASTString::npos) + m_errorReporter.syntaxError(_literal.location(), "Invalid use of underscores in number literal. No underscore in front of exponent allowed."); } + + return true; +} + +bool SyntaxChecker::visit(UnaryOperation const& _operation) +{ + if (_operation.getOperator() == Token::Add) + m_errorReporter.syntaxError(_operation.location(), "Use of unary + is disallowed."); + return true; } @@ -210,40 +256,34 @@ bool SyntaxChecker::visit(PlaceholderStatement const&) return true; } -bool SyntaxChecker::visit(FunctionDefinition const& _function) +bool SyntaxChecker::visit(ContractDefinition const& _contract) { - bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050); - - if (v050 && _function.noVisibilitySpecified()) - m_errorReporter.syntaxError(_function.location(), "No visibility specified."); + m_isInterface = _contract.contractKind() == ContractDefinition::ContractKind::Interface; - if (_function.isOldStyleConstructor()) - { - if (v050) - m_errorReporter.syntaxError( - _function.location(), + ASTString const& contractName = _contract.name(); + for (FunctionDefinition const* function: _contract.definedFunctions()) + if (function->name() == contractName) + m_errorReporter.syntaxError(function->location(), "Functions are not allowed to have the same name as the contract. " "If you intend this to be a constructor, use \"constructor(...) { ... }\" to define it." ); - else - m_errorReporter.warning( - _function.location(), - "Defining constructors as functions with the same name as the contract is deprecated. " - "Use \"constructor(...) { ... }\" instead." - ); - } - if (!_function.isImplemented() && !_function.modifiers().empty()) + return true; +} + +bool SyntaxChecker::visit(FunctionDefinition const& _function) +{ + if (_function.noVisibilitySpecified()) { - if (v050) - m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers."); - else - m_errorReporter.warning(_function.location(), "Modifiers of functions without implementation are ignored." ); - } - if (_function.name() == "constructor") - m_errorReporter.warning(_function.location(), - "This function is named \"constructor\" but is not the constructor of the contract. " - "If you intend this to be a constructor, use \"constructor(...) { ... }\" without the \"function\" keyword to define it." + string suggestedVisibility = _function.isFallback() || m_isInterface ? "external" : "public"; + m_errorReporter.syntaxError( + _function.location(), + "No visibility specified. Did you intend to add \"" + suggestedVisibility + "\"?" ); + } + + if (!_function.isImplemented() && !_function.modifiers().empty()) + m_errorReporter.syntaxError(_function.location(), "Functions without implementation cannot have modifiers."); + return true; } @@ -255,35 +295,27 @@ bool SyntaxChecker::visit(FunctionTypeName const& _node) for (auto const& decl: _node.returnParameterTypeList()->parameters()) if (!decl->name().empty()) - m_errorReporter.warning(decl->location(), "Naming function type return parameters is deprecated."); + m_errorReporter.syntaxError(decl->location(), "Return parameters in function types may not be named."); return true; } -bool SyntaxChecker::visit(VariableDeclaration const& _declaration) +bool SyntaxChecker::visit(VariableDeclarationStatement const& _statement) { - bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050); + // Report if none of the variable components in the tuple have a name (only possible via deprecated "var") + if (boost::algorithm::all_of_equal(_statement.declarations(), nullptr)) + m_errorReporter.syntaxError( + _statement.location(), + "The use of the \"var\" keyword is disallowed. The declaration part of the statement can be removed, since it is empty." + ); - if (!_declaration.typeName()) - { - if (v050) - m_errorReporter.syntaxError(_declaration.location(), "Use of the \"var\" keyword is deprecated."); - else - m_errorReporter.warning(_declaration.location(), "Use of the \"var\" keyword is deprecated."); - } return true; } bool SyntaxChecker::visit(StructDefinition const& _struct) { - bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050); - if (_struct.members().empty()) - { - if (v050) - m_errorReporter.syntaxError(_struct.location(), "Defining empty structs is disallowed."); - else - m_errorReporter.warning(_struct.location(), "Defining empty structs is deprecated."); - } + m_errorReporter.syntaxError(_struct.location(), "Defining empty structs is disallowed."); + return true; } diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index 1579df57..f5716bf9 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -52,6 +52,12 @@ private: virtual bool visit(ModifierDefinition const& _modifier) override; virtual void endVisit(ModifierDefinition const& _modifier) override; + /// Reports an error if _statement is a VariableDeclarationStatement. + /// Used by if/while/for to check for single statement variable declarations + /// without a block. + void checkSingleStatementVariableDeclaration(ASTNode const& _statement); + + virtual bool visit(IfStatement const& _ifStatement) override; virtual bool visit(WhileStatement const& _whileStatement) override; virtual void endVisit(WhileStatement const& _whileStatement) override; virtual bool visit(ForStatement const& _forStatement) override; @@ -66,12 +72,14 @@ private: virtual bool visit(PlaceholderStatement const& _placeholderStatement) override; + virtual bool visit(ContractDefinition const& _contract) override; virtual bool visit(FunctionDefinition const& _function) override; virtual bool visit(FunctionTypeName const& _node) override; - virtual bool visit(VariableDeclaration const& _declaration) override; + virtual bool visit(VariableDeclarationStatement const& _statement) override; virtual bool visit(StructDefinition const& _struct) override; + virtual bool visit(Literal const& _literal) override; ErrorReporter& m_errorReporter; @@ -82,6 +90,7 @@ private: bool m_versionPragmaFound = false; int m_inLoopDepth = 0; + bool m_isInterface = false; SourceUnit const* m_sourceUnit = nullptr; }; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 2062458e..c5e6488b 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -22,13 +22,16 @@ #include <libsolidity/analysis/TypeChecker.h> #include <memory> +#include <boost/algorithm/cxx11/all_of.hpp> #include <boost/algorithm/string/predicate.hpp> +#include <boost/algorithm/string/join.hpp> #include <boost/range/adaptor/reversed.hpp> #include <libsolidity/ast/AST.h> #include <libsolidity/inlineasm/AsmAnalysis.h> #include <libsolidity/inlineasm/AsmAnalysisInfo.h> #include <libsolidity/inlineasm/AsmData.h> #include <libsolidity/interface/ErrorReporter.h> +#include <libdevcore/Algorithms.h> using namespace std; using namespace dev; @@ -41,15 +44,13 @@ bool typeSupportedByOldABIEncoder(Type const& _type) { if (_type.dataStoredIn(DataLocation::Storage)) return true; - else if (_type.category() == Type::Category::Struct) + if (_type.category() == Type::Category::Struct) return false; - else if (_type.category() == Type::Category::Array) + if (_type.category() == Type::Category::Array) { auto const& arrayType = dynamic_cast<ArrayType const&>(_type); auto base = arrayType.baseType(); - if (!typeSupportedByOldABIEncoder(*base)) - return false; - else if (base->category() == Type::Category::Array && base->isDynamicallySized()) + if (!typeSupportedByOldABIEncoder(*base) || (base->category() == Type::Category::Array && base->isDynamicallySized())) return false; } return true; @@ -125,10 +126,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract) m_errorReporter.typeError(function->parameterList().location(), "Fallback function cannot take parameters."); if (!function->returnParameters().empty()) m_errorReporter.typeError(function->returnParameterList()->location(), "Fallback function cannot return values."); - if ( - _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) && - function->visibility() != FunctionDefinition::Visibility::External - ) + if (function->visibility() != FunctionDefinition::Visibility::External) m_errorReporter.typeError(function->location(), "Fallback function must be defined as \"external\"."); } @@ -216,7 +214,7 @@ void TypeChecker::findDuplicateDefinitions(map<string, vector<T>> const& _defini SecondarySourceLocation ssl; for (size_t j = i + 1; j < overloads.size(); ++j) - if (FunctionType(*overloads[i]).hasEqualArgumentTypes(FunctionType(*overloads[j]))) + if (FunctionType(*overloads[i]).hasEqualParameterTypes(FunctionType(*overloads[j]))) { ssl.append("Other declaration is here:", overloads[j]->location()); reported.insert(j); @@ -254,7 +252,7 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont FunctionTypePointer funType = make_shared<FunctionType>(*function); auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag) { - return funType->hasEqualArgumentTypes(*_funAndFlag.first); + return funType->hasEqualParameterTypes(*_funAndFlag.first); }); if (it == overloads.end()) overloads.push_back(make_pair(funType, function->isImplemented())); @@ -281,8 +279,6 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) { - bool const v050 = _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts; // Determine the arguments that are used for the base constructors. @@ -290,27 +286,19 @@ void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const { if (FunctionDefinition const* constructor = contract->constructor()) for (auto const& modifier: constructor->modifiers()) - { - auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(*modifier->name())); - if (modifier->arguments()) + if (auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(*modifier->name()))) { - if (baseContract && baseContract->constructor()) - annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); - } - else - { - if (v050) - m_errorReporter.declarationError( - modifier->location(), - "Modifier-style base constructor call without arguments." - ); + if (modifier->arguments()) + { + if (baseContract->constructor()) + annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); + } else - m_errorReporter.warning( + m_errorReporter.declarationError( modifier->location(), "Modifier-style base constructor call without arguments." ); } - } for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts()) { @@ -337,8 +325,6 @@ void TypeChecker::annotateBaseConstructorArguments( ASTNode const* _argumentNode ) { - bool const v050 = _currentContract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - solAssert(_baseConstructor, ""); solAssert(_argumentNode, ""); @@ -351,7 +337,7 @@ void TypeChecker::annotateBaseConstructorArguments( SourceLocation const* mainLocation = nullptr; SecondarySourceLocation ssl; - + if ( _currentContract.location().contains(previousNode->location()) || _currentContract.location().contains(_argumentNode->location()) @@ -367,18 +353,11 @@ void TypeChecker::annotateBaseConstructorArguments( ssl.append("Second constructor call is here: ", previousNode->location()); } - if (v050) - m_errorReporter.declarationError( - *mainLocation, - ssl, - "Base constructor arguments given twice." - ); - else - m_errorReporter.warning( - *mainLocation, - "Base constructor arguments given twice.", - ssl - ); + m_errorReporter.declarationError( + *mainLocation, + ssl, + "Base constructor arguments given twice." + ); } } @@ -425,7 +404,7 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& function, Func FunctionType functionType(function); FunctionType superType(super); - if (!functionType.hasEqualArgumentTypes(superType)) + if (!functionType.hasEqualParameterTypes(superType)) return; if (!function.annotation().superFunction) @@ -462,7 +441,7 @@ void TypeChecker::overrideError(FunctionDefinition const& function, FunctionDefi { m_errorReporter.typeError( function.location(), - SecondarySourceLocation().append("Overriden function is here:", super.location()), + SecondarySourceLocation().append("Overridden function is here:", super.location()), message ); } @@ -496,7 +475,7 @@ void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _co for (auto const& it: externalDeclarations) for (size_t i = 0; i < it.second.size(); ++i) for (size_t j = i + 1; j < it.second.size(); ++j) - if (!it.second[i].second->hasEqualArgumentTypes(*it.second[j].second)) + if (!it.second[i].second->hasEqualParameterTypes(*it.second[j].second)) m_errorReporter.typeError( it.second[j].first->location(), "Function overload clash during conversion to external types for arguments." @@ -519,7 +498,12 @@ void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment) TupleType const& lhs = dynamic_cast<TupleType const&>(*type(_assignment.leftHandSide())); TupleType const& rhs = dynamic_cast<TupleType const&>(*type(_assignment.rightHandSide())); - bool fillRight = !lhs.components().empty() && (!lhs.components().back() || lhs.components().front()); + if (lhs.components().size() != rhs.components().size()) + { + solAssert(m_errorReporter.hasErrors(), ""); + return; + } + size_t storageToStorageCopies = 0; size_t toStorageCopies = 0; for (size_t i = 0; i < lhs.components().size(); ++i) @@ -527,10 +511,8 @@ void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment) ReferenceType const* ref = dynamic_cast<ReferenceType const*>(lhs.components()[i].get()); if (!ref || !ref->dataStoredIn(DataLocation::Storage) || ref->isPointer()) continue; - size_t rhsPos = fillRight ? i : rhs.components().size() - (lhs.components().size() - i); - solAssert(rhsPos < rhs.components().size(), ""); toStorageCopies++; - if (rhs.components()[rhsPos]->dataStoredIn(DataLocation::Storage)) + if (rhs.components()[i]->dataStoredIn(DataLocation::Storage)) storageToStorageCopies++; } if (storageToStorageCopies >= 1 && toStorageCopies >= 2) @@ -543,6 +525,76 @@ void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment) ); } +TypePointers TypeChecker::typeCheckABIDecodeAndRetrieveReturnType(FunctionCall const& _functionCall, bool _abiEncoderV2) +{ + vector<ASTPointer<Expression const>> arguments = _functionCall.arguments(); + if (arguments.size() != 2) + m_errorReporter.typeError( + _functionCall.location(), + "This function takes two arguments, but " + + toString(arguments.size()) + + " were provided." + ); + if (arguments.size() >= 1 && !type(*arguments.front())->isImplicitlyConvertibleTo(ArrayType::bytesMemory())) + m_errorReporter.typeError( + arguments.front()->location(), + "Invalid type for argument in function call. " + "Invalid implicit conversion from " + + type(*arguments.front())->toString() + + " to bytes memory requested." + ); + + if (arguments.size() < 2) + return {}; + + // The following is a rather syntactic restriction, but we check it here anyway: + // The second argument has to be a tuple expression containing type names. + TupleExpression const* tupleExpression = dynamic_cast<TupleExpression const*>(arguments[1].get()); + if (!tupleExpression) + { + m_errorReporter.typeError( + arguments[1]->location(), + "The second argument to \"abi.decode\" has to be a tuple of types." + ); + return {}; + } + + TypePointers components; + for (auto const& typeArgument: tupleExpression->components()) + { + solAssert(typeArgument, ""); + if (TypeType const* argTypeType = dynamic_cast<TypeType const*>(type(*typeArgument).get())) + { + TypePointer actualType = argTypeType->actualType(); + solAssert(actualType, ""); + // We force memory because the parser currently cannot handle + // data locations. Furthermore, storage can be a little dangerous and + // calldata is not really implemented anyway. + actualType = ReferenceType::copyForLocationIfReference(DataLocation::Memory, actualType); + // We force address payable for address types. + if (actualType->category() == Type::Category::Address) + actualType = make_shared<AddressType>(StateMutability::Payable); + solAssert( + !actualType->dataStoredIn(DataLocation::CallData) && + !actualType->dataStoredIn(DataLocation::Storage), + "" + ); + if (!actualType->fullEncodingType(false, _abiEncoderV2, false)) + m_errorReporter.typeError( + typeArgument->location(), + "Decoding type " + actualType->toString(false) + " not supported." + ); + components.push_back(actualType); + } + else + { + m_errorReporter.typeError(typeArgument->location(), "Argument has to be a type name."); + components.push_back(make_shared<TupleType>()); + } + } + return components; +} + void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance) { auto base = dynamic_cast<ContractDefinition const*>(&dereference(_inheritance.name())); @@ -562,33 +614,18 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance) if (arguments) { - bool v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - if (parameterTypes.size() != arguments->size()) { - if (arguments->size() == 0 && !v050) - m_errorReporter.warning( - _inheritance.location(), - "Wrong argument count for constructor call: " + - toString(arguments->size()) + - " arguments given but expected " + - toString(parameterTypes.size()) + - "." - ); - else - { - m_errorReporter.typeError( - _inheritance.location(), - "Wrong argument count for constructor call: " + - toString(arguments->size()) + - " arguments given but expected " + - toString(parameterTypes.size()) + - "." - ); - return; - } + m_errorReporter.typeError( + _inheritance.location(), + "Wrong argument count for constructor call: " + + toString(arguments->size()) + + " arguments given but expected " + + toString(parameterTypes.size()) + + ". Remove parentheses if you do not want to provide arguments here." + ); } - for (size_t i = 0; i < arguments->size(); ++i) + for (size_t i = 0; i < std::min(arguments->size(), parameterTypes.size()); ++i) if (!type(*(*arguments)[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) m_errorReporter.typeError( (*arguments)[i]->location(), @@ -613,41 +650,44 @@ void TypeChecker::endVisit(UsingForDirective const& _usingFor) bool TypeChecker::visit(StructDefinition const& _struct) { - if (m_scope->contractKind() == ContractDefinition::ContractKind::Interface) - m_errorReporter.typeError(_struct.location(), "Structs cannot be defined in interfaces."); - for (ASTPointer<VariableDeclaration> const& member: _struct.members()) if (!type(*member)->canBeStored()) m_errorReporter.typeError(member->location(), "Type cannot be used in struct."); // Check recursion, fatal error if detected. - using StructPointer = StructDefinition const*; - using StructPointersSet = set<StructPointer>; - function<void(StructPointer,StructPointersSet const&)> check = [&](StructPointer _struct, StructPointersSet const& _parents) - { - if (_parents.count(_struct)) - m_errorReporter.fatalTypeError(_struct->location(), "Recursive struct definition."); - StructPointersSet parents = _parents; - parents.insert(_struct); - for (ASTPointer<VariableDeclaration> const& member: _struct->members()) - if (type(*member)->category() == Type::Category::Struct) + auto visitor = [&](StructDefinition const& _struct, CycleDetector<StructDefinition>& _cycleDetector, size_t _depth) + { + if (_depth >= 256) + m_errorReporter.fatalDeclarationError(_struct.location(), "Struct definition exhausting cyclic dependency validator."); + + for (ASTPointer<VariableDeclaration> const& member: _struct.members()) + { + Type const* memberType = type(*member).get(); + while (auto arrayType = dynamic_cast<ArrayType const*>(memberType)) { - auto const& typeName = dynamic_cast<UserDefinedTypeName const&>(*member->typeName()); - check(&dynamic_cast<StructDefinition const&>(*typeName.annotation().referencedDeclaration), parents); + if (arrayType->isDynamicallySized()) + break; + memberType = arrayType->baseType().get(); } + if (auto structType = dynamic_cast<StructType const*>(memberType)) + if (_cycleDetector.run(structType->structDefinition())) + return; + } }; - check(&_struct, StructPointersSet{}); + if (CycleDetector<StructDefinition>(visitor).run(_struct) != nullptr) + m_errorReporter.fatalTypeError(_struct.location(), "Recursive struct definition."); + bool insideStruct = true; + swap(insideStruct, m_insideStruct); ASTNode::listAccept(_struct.members(), *this); + m_insideStruct = insideStruct; return false; } bool TypeChecker::visit(FunctionDefinition const& _function) { - bool isLibraryFunction = - dynamic_cast<ContractDefinition const*>(_function.scope()) && - dynamic_cast<ContractDefinition const*>(_function.scope())->isLibrary(); + bool isLibraryFunction = _function.inContractKind() == ContractDefinition::ContractKind::Library; if (_function.isPayable()) { if (isLibraryFunction) @@ -657,7 +697,15 @@ bool TypeChecker::visit(FunctionDefinition const& _function) } for (ASTPointer<VariableDeclaration> const& var: _function.parameters() + _function.returnParameters()) { - if (!type(*var)->canLiveOutsideStorage()) + 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 + ) m_errorReporter.typeError(var->location(), "Type is required to live outside storage."); if (_function.visibility() >= FunctionDefinition::Visibility::Public && !(type(*var)->interfaceType(isLibraryFunction))) m_errorReporter.fatalTypeError(var->location(), "Internal or recursive type is not allowed for public or external functions."); @@ -696,18 +744,10 @@ bool TypeChecker::visit(FunctionDefinition const& _function) { if (_function.isImplemented()) m_errorReporter.typeError(_function.location(), "Functions in interfaces cannot have an implementation."); - if (_function.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) - { - if (_function.visibility() != FunctionDefinition::Visibility::External) - m_errorReporter.typeError(_function.location(), "Functions in interfaces must be declared external."); - } - else - { - if (_function.visibility() < FunctionDefinition::Visibility::Public) - m_errorReporter.typeError(_function.location(), "Functions in interfaces cannot be internal or private."); - else if (_function.visibility() != FunctionDefinition::Visibility::External) - m_errorReporter.warning(_function.location(), "Functions in interfaces should be declared external."); - } + + if (_function.visibility() != FunctionDefinition::Visibility::External) + m_errorReporter.typeError(_function.location(), "Functions in interfaces must be declared external."); + if (_function.isConstructor()) m_errorReporter.typeError(_function.location(), "Constructor cannot be defined in interfaces."); } @@ -726,10 +766,12 @@ bool TypeChecker::visit(FunctionDefinition const& _function) bool TypeChecker::visit(VariableDeclaration const& _variable) { // Forbid any variable declarations inside interfaces unless they are part of - // a function's input/output parameters. + // * a function's input/output parameters, + // * or inside of a struct definition. if ( m_scope->contractKind() == ContractDefinition::ContractKind::Interface && !_variable.isCallableParameter() + && !m_insideStruct ) m_errorReporter.typeError(_variable.location(), "Variables cannot be declared in interfaces."); @@ -742,12 +784,11 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) // TypeChecker at the VariableDeclarationStatement level. TypePointer varType = _variable.annotation().type; solAssert(!!varType, "Failed to infer variable type."); + if (_variable.value()) 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; @@ -760,19 +801,10 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) if (!_variable.value()) m_errorReporter.typeError(_variable.location(), "Uninitialized \"constant\" variable."); else if (!_variable.value()->annotation().isPure) - { - if (_variable.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) - m_errorReporter.typeError( - _variable.value()->location(), - "Initial value for constant variable has to be compile-time constant." - ); - else - m_errorReporter.warning( - _variable.value()->location(), - "Initial value for constant variable has to be compile-time constant. " - "This will fail to compile with the next breaking version change." - ); - } + m_errorReporter.typeError( + _variable.value()->location(), + "Initial value for constant variable has to be compile-time constant." + ); } if (!_variable.isStateVariable()) { @@ -786,7 +818,9 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) ) m_errorReporter.typeError(_variable.location(), "Internal or recursive type is not allowed for public state variables."); - if (varType->category() == Type::Category::Array) + switch (varType->category()) + { + case Type::Category::Array: if (auto arrayType = dynamic_cast<ArrayType const*>(varType.get())) if ( ((arrayType->location() == DataLocation::Memory) || @@ -794,17 +828,22 @@ bool TypeChecker::visit(VariableDeclaration const& _variable) !arrayType->validForCalldata() ) m_errorReporter.typeError(_variable.location(), "Array is too large to be encoded."); + break; + case Type::Category::Mapping: + if (auto mappingType = dynamic_cast<MappingType const*>(varType.get())) + if ( + mappingType->keyType()->isDynamicallySized() && + _variable.visibility() == Declaration::Visibility::Public + ) + m_errorReporter.typeError(_variable.location(), "Dynamically-sized keys for public mappings are not supported."); + break; + default: + break; + } return false; } -bool TypeChecker::visit(EnumDefinition const& _enum) -{ - if (m_scope->contractKind() == ContractDefinition::ContractKind::Interface) - m_errorReporter.typeError(_enum.location(), "Enumerable cannot be declared in interfaces."); - return false; -} - void TypeChecker::visitManually( ModifierInvocation const& _modifier, vector<ContractDefinition const*> const& _bases @@ -913,9 +952,9 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) { // External references have already been resolved in a prior stage and stored in the annotation. // We run the resolve step again regardless. - julia::ExternalIdentifierAccess::Resolver identifierAccess = [&]( + yul::ExternalIdentifierAccess::Resolver identifierAccess = [&]( assembly::Identifier const& _identifier, - julia::IdentifierContext _context, + yul::IdentifierContext _context, bool ) { @@ -924,6 +963,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) return size_t(-1); Declaration const* declaration = ref->second.declaration; solAssert(!!declaration, ""); + bool requiresStorage = ref->second.isSlot || ref->second.isOffset; if (auto var = dynamic_cast<VariableDeclaration const*>(declaration)) { if (var->isConstant()) @@ -931,14 +971,14 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) m_errorReporter.typeError(_identifier.location, "Constant variables not supported by inline assembly."); return size_t(-1); } - else if (ref->second.isSlot || ref->second.isOffset) + else if (requiresStorage) { if (!var->isStateVariable() && !var->type()->dataStoredIn(DataLocation::Storage)) { m_errorReporter.typeError(_identifier.location, "The suffixes _offset and _slot can only be used on storage variables."); return size_t(-1); } - else if (_context != julia::IdentifierContext::RValue) + else if (_context != yul::IdentifierContext::RValue) { m_errorReporter.typeError(_identifier.location, "Storage variables cannot be assigned to."); return size_t(-1); @@ -951,7 +991,7 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) } else if (var->type()->dataStoredIn(DataLocation::Storage)) { - m_errorReporter.typeError(_identifier.location, "You have to use the _slot or _offset prefix to access storage reference variables."); + m_errorReporter.typeError(_identifier.location, "You have to use the _slot or _offset suffix to access storage reference variables."); return size_t(-1); } else if (var->type()->sizeOnStack() != 1) @@ -963,13 +1003,18 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) return size_t(-1); } } - else if (_context == julia::IdentifierContext::LValue) + else if (requiresStorage) + { + m_errorReporter.typeError(_identifier.location, "The suffixes _offset and _slot can only be used on storage variables."); + return size_t(-1); + } + else if (_context == yul::IdentifierContext::LValue) { m_errorReporter.typeError(_identifier.location, "Only local variables can be assigned to in inline assembly."); return size_t(-1); } - if (_context == julia::IdentifierContext::RValue) + if (_context == yul::IdentifierContext::RValue) { solAssert(!!declaration->type(), "Type of declaration required but not yet determined."); if (dynamic_cast<FunctionDefinition const*>(declaration)) @@ -994,15 +1039,11 @@ bool TypeChecker::visit(InlineAssembly const& _inlineAssembly) }; solAssert(!_inlineAssembly.annotation().analysisInfo, ""); _inlineAssembly.annotation().analysisInfo = make_shared<assembly::AsmAnalysisInfo>(); - boost::optional<Error::Type> errorTypeForLoose = - m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) ? - Error::Type::SyntaxError : - Error::Type::Warning; assembly::AsmAnalyzer analyzer( *_inlineAssembly.annotation().analysisInfo, m_errorReporter, m_evmVersion, - errorTypeForLoose, + Error::Type::SyntaxError, assembly::AsmFlavour::Loose, identifierAccess ); @@ -1041,9 +1082,13 @@ bool TypeChecker::visit(ForStatement const& _forStatement) void TypeChecker::endVisit(Return const& _return) { + ParameterList const* params = _return.annotation().functionReturnParameters; if (!_return.expression()) + { + if (params && !params->parameters().empty()) + m_errorReporter.typeError(_return.location(), "Return arguments required."); return; - ParameterList const* params = _return.annotation().functionReturnParameters; + } if (!params) { m_errorReporter.typeError(_return.location(), "Return arguments not allowed."); @@ -1094,29 +1139,87 @@ void TypeChecker::endVisit(EmitStatement const& _emit) m_insideEmitStatement = false; } +namespace +{ +/** + * @returns a suggested left-hand-side of a multi-variable declaration contairing + * the variable declarations given in @a _decls. + */ +string createTupleDecl(vector<ASTPointer<VariableDeclaration>> const& _decls) +{ + vector<string> components; + for (ASTPointer<VariableDeclaration> const& decl: _decls) + if (decl) + { + solAssert(decl->annotation().type, ""); + components.emplace_back(decl->annotation().type->toString(false) + " " + decl->name()); + } + else + components.emplace_back(); + + if (_decls.size() == 1) + return components.front(); + else + return "(" + boost::algorithm::join(components, ", ") + ")"; +} + +bool typeCanBeExpressed(vector<ASTPointer<VariableDeclaration>> const& decls) +{ + for (ASTPointer<VariableDeclaration> const& decl: decls) + { + // skip empty tuples (they can be expressed of course) + if (!decl) + continue; + + if (!decl->annotation().type) + return false; + + if (auto functionType = dynamic_cast<FunctionType const*>(decl->annotation().type.get())) + if ( + functionType->kind() != FunctionType::Kind::Internal && + functionType->kind() != FunctionType::Kind::External + ) + return false; + } + + return true; +} +} + bool TypeChecker::visit(VariableDeclarationStatement const& _statement) { - bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); if (!_statement.initialValue()) { // No initial value is only permitted for single variables with specified type. if (_statement.declarations().size() != 1 || !_statement.declarations().front()) - m_errorReporter.fatalTypeError(_statement.location(), "Assignment necessary for type detection."); + { + if (boost::algorithm::all_of_equal(_statement.declarations(), nullptr)) + { + // The syntax checker has already generated an error for this case (empty LHS tuple). + solAssert(m_errorReporter.hasErrors(), ""); + + // It is okay to return here, as there are no named components on the + // left-hand-side that could cause any damage later. + return false; + } + else + // Bailing out *fatal* here, as those (untyped) vars may be used later, and diagnostics wouldn't be helpful then. + m_errorReporter.fatalTypeError(_statement.location(), "Use of the \"var\" keyword is disallowed."); + } + VariableDeclaration const& varDecl = *_statement.declarations().front(); if (!varDecl.annotation().type) - m_errorReporter.fatalTypeError(_statement.location(), "Assignment necessary for type detection."); + m_errorReporter.fatalTypeError(_statement.location(), "Use of the \"var\" keyword is disallowed."); + if (auto ref = dynamic_cast<ReferenceType const*>(type(varDecl).get())) { if (ref->dataStoredIn(DataLocation::Storage)) { string errorText{"Uninitialized storage pointer."}; - if (varDecl.referenceLocation() == VariableDeclaration::Location::Default) + if (varDecl.referenceLocation() == VariableDeclaration::Location::Unspecified) errorText += " Did you mean '<type> memory " + varDecl.name() + "'?"; solAssert(m_scope, ""); - if (v050) - m_errorReporter.declarationError(varDecl.location(), errorText); - else - m_errorReporter.warning(varDecl.location(), errorText); + m_errorReporter.declarationError(varDecl.location(), errorText); } } else if (dynamic_cast<MappingType const*>(type(varDecl).get())) @@ -1138,85 +1241,34 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement) else valueTypes = TypePointers{type(*_statement.initialValue())}; - // Determine which component is assigned to which variable. - // If numbers do not match, fill up if variables begin or end empty (not both). - vector<VariableDeclaration const*>& assignments = _statement.annotation().assignments; - assignments.resize(valueTypes.size(), nullptr); vector<ASTPointer<VariableDeclaration>> const& variables = _statement.declarations(); if (variables.empty()) - { - if (!valueTypes.empty()) - m_errorReporter.fatalTypeError( - _statement.location(), - "Too many components (" + - toString(valueTypes.size()) + - ") in value for variable assignment (0) needed" - ); - } + // We already have an error for this in the SyntaxChecker. + solAssert(m_errorReporter.hasErrors(), ""); else if (valueTypes.size() != variables.size()) - { - if (v050) - m_errorReporter.fatalTypeError( - _statement.location(), - "Different number of components on the left hand side (" + - toString(variables.size()) + - ") than on the right hand side (" + - toString(valueTypes.size()) + - ")." - ); - else if (!variables.front() && !variables.back()) - m_errorReporter.fatalTypeError( - _statement.location(), - "Wildcard both at beginning and end of variable declaration list is only allowed " - "if the number of components is equal." - ); - else - m_errorReporter.warning( - _statement.location(), - "Different number of components on the left hand side (" + - toString(variables.size()) + - ") than on the right hand side (" + - toString(valueTypes.size()) + - ")." - ); - } - size_t minNumValues = variables.size(); - if (!variables.empty() && (!variables.back() || !variables.front())) - --minNumValues; - if (valueTypes.size() < minNumValues) - m_errorReporter.fatalTypeError( - _statement.location(), - "Not enough components (" + - toString(valueTypes.size()) + - ") in value to assign all variables (" + - toString(minNumValues) + ")." - ); - if (valueTypes.size() > variables.size() && variables.front() && variables.back()) - m_errorReporter.fatalTypeError( + m_errorReporter.typeError( _statement.location(), - "Too many components (" + + "Different number of components on the left hand side (" + + toString(variables.size()) + + ") than on the right hand side (" + toString(valueTypes.size()) + - ") in value for variable assignment (" + - toString(minNumValues) + - " needed)." + ")." ); - bool fillRight = !variables.empty() && (!variables.back() || variables.front()); - for (size_t i = 0; i < min(variables.size(), valueTypes.size()); ++i) - if (fillRight) - assignments[i] = variables[i].get(); - else - assignments[assignments.size() - i - 1] = variables[variables.size() - i - 1].get(); - for (size_t i = 0; i < assignments.size(); ++i) + bool autoTypeDeductionNeeded = false; + + for (size_t i = 0; i < min(variables.size(), valueTypes.size()); ++i) { - if (!assignments[i]) + if (!variables[i]) continue; - VariableDeclaration const& var = *assignments[i]; + VariableDeclaration const& var = *variables[i]; solAssert(!var.value(), "Value has to be tied to statement."); TypePointer const& valueComponentType = valueTypes[i]; solAssert(!!valueComponentType, ""); if (!var.annotation().type) { + autoTypeDeductionNeeded = true; + // Infer type from value. solAssert(!var.typeName(), ""); var.annotation().type = valueComponentType->mobileType(); @@ -1260,14 +1312,6 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement) } else solAssert(dynamic_cast<FixedPointType const*>(var.annotation().type.get()), "Unknown type."); - - m_errorReporter.warning( - _statement.location(), - "The type of this variable was inferred as " + - typeName + - extension + - ". This is probably not desired. Use an explicit type to silence this warning." - ); } var.accept(*this); @@ -1304,6 +1348,23 @@ bool TypeChecker::visit(VariableDeclarationStatement const& _statement) } } } + + if (autoTypeDeductionNeeded) + { + if (!typeCanBeExpressed(variables)) + m_errorReporter.syntaxError( + _statement.location(), + "Use of the \"var\" keyword is disallowed. " + "Type cannot be expressed in syntax." + ); + else + m_errorReporter.syntaxError( + _statement.location(), + "Use of the \"var\" keyword is disallowed. " + "Use explicit declaration `" + createTupleDecl(variables) + " = ...´ instead." + ); + } + return false; } @@ -1321,7 +1382,8 @@ void TypeChecker::endVisit(ExpressionStatement const& _statement) if ( kind == FunctionType::Kind::BareCall || kind == FunctionType::Kind::BareCallCode || - kind == FunctionType::Kind::BareDelegateCall + kind == FunctionType::Kind::BareDelegateCall || + kind == FunctionType::Kind::BareStaticCall ) m_errorReporter.warning(_statement.location(), "Return value of low-level calls not used."); else if (kind == FunctionType::Kind::Send) @@ -1375,12 +1437,45 @@ bool TypeChecker::visit(Conditional const& _conditional) return false; } +void TypeChecker::checkExpressionAssignment(Type const& _type, Expression const& _expression) +{ + if (auto const* tupleExpression = dynamic_cast<TupleExpression const*>(&_expression)) + { + auto const* tupleType = dynamic_cast<TupleType const*>(&_type); + auto const& types = tupleType ? tupleType->components() : vector<TypePointer> { _type.shared_from_this() }; + + solAssert( + tupleExpression->components().size() == types.size() || m_errorReporter.hasErrors(), + "Array sizes don't match or no errors generated." + ); + + for (size_t i = 0; i < min(tupleExpression->components().size(), types.size()); i++) + if (types[i]) + { + solAssert(!!tupleExpression->components()[i], ""); + checkExpressionAssignment(*types[i], *tupleExpression->components()[i]); + } + } + else if (_type.category() == Type::Category::Mapping) + { + bool isLocalOrReturn = false; + if (auto const* identifier = dynamic_cast<Identifier const*>(&_expression)) + if (auto const *variableDeclaration = dynamic_cast<VariableDeclaration const*>(identifier->annotation().referencedDeclaration)) + if (variableDeclaration->isLocalOrReturn()) + isLocalOrReturn = true; + if (!isLocalOrReturn) + m_errorReporter.typeError(_expression.location(), "Mappings cannot be assigned to."); + } +} + bool TypeChecker::visit(Assignment const& _assignment) { - bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); requireLValue(_assignment.leftHandSide()); TypePointer t = type(_assignment.leftHandSide()); _assignment.annotation().type = t; + + checkExpressionAssignment(*t, _assignment.leftHandSide()); + if (TupleType const* tupleType = dynamic_cast<TupleType const*>(t.get())) { if (_assignment.assignmentOperator() != Token::Assign) @@ -1394,30 +1489,8 @@ bool TypeChecker::visit(Assignment const& _assignment) expectType(_assignment.rightHandSide(), *tupleType); // expectType does not cause fatal errors, so we have to check again here. - if (TupleType const* rhsType = dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get())) - { + if (dynamic_cast<TupleType const*>(type(_assignment.rightHandSide()).get())) checkDoubleStorageAssignment(_assignment); - // @todo For 0.5.0, this code shoud move to TupleType::isImplicitlyConvertibleTo, - // but we cannot do it right now. - if (rhsType->components().size() != tupleType->components().size()) - { - string message = - "Different number of components on the left hand side (" + - toString(tupleType->components().size()) + - ") than on the right hand side (" + - toString(rhsType->components().size()) + - ")."; - if (v050) - m_errorReporter.typeError(_assignment.location(), message); - else - m_errorReporter.warning(_assignment.location(), message); - } - } - } - else if (t->category() == Type::Category::Mapping) - { - m_errorReporter.typeError(_assignment.location(), "Mappings cannot be assigned to."); - _assignment.rightHandSide().accept(*this); } else if (_assignment.assignmentOperator() == Token::Assign) expectType(_assignment.rightHandSide(), *t); @@ -1426,14 +1499,14 @@ bool TypeChecker::visit(Assignment const& _assignment) // compound assignment _assignment.rightHandSide().accept(*this); TypePointer resultType = t->binaryOperatorResult( - Token::AssignmentToBinaryOp(_assignment.assignmentOperator()), + TokenTraits::AssignmentToBinaryOp(_assignment.assignmentOperator()), type(_assignment.rightHandSide()) ); if (!resultType || *resultType != *t) m_errorReporter.typeError( _assignment.location(), "Operator " + - string(Token::toString(_assignment.assignmentOperator())) + + string(TokenTraits::toString(_assignment.assignmentOperator())) + " not compatible with types " + t->toString() + " and " + @@ -1469,14 +1542,12 @@ bool TypeChecker::visit(TupleExpression const& _tuple) } else { - bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); bool isPure = true; TypePointer inlineArrayType; for (size_t i = 0; i < components.size(); ++i) { - // Outside of an lvalue-context, the only situation where a component can be empty is (x,). - if (!components[i] && !(i == 1 && components.size() == 2)) + if (!components[i]) m_errorReporter.fatalTypeError(_tuple.location(), "Tuple component cannot be empty."); else if (components[i]) { @@ -1488,10 +1559,7 @@ bool TypeChecker::visit(TupleExpression const& _tuple) { if (_tuple.isInlineArray()) m_errorReporter.fatalTypeError(components[i]->location(), "Array component cannot be empty."); - if (v050) - m_errorReporter.fatalTypeError(components[i]->location(), "Tuple component cannot be empty."); - else - m_errorReporter.warning(components[i]->location(), "Tuple component cannot be empty."); + m_errorReporter.typeError(components[i]->location(), "Tuple component cannot be empty."); } // Note: code generation will visit each of the expression even if they are not assigned from. @@ -1529,11 +1597,7 @@ bool TypeChecker::visit(TupleExpression const& _tuple) if (components.size() == 1) _tuple.annotation().type = type(*components[0]); else - { - if (components.size() == 2 && !components[1]) - types.pop_back(); _tuple.annotation().type = make_shared<TupleType>(types); - } } } @@ -1543,8 +1607,8 @@ bool TypeChecker::visit(TupleExpression const& _tuple) bool TypeChecker::visit(UnaryOperation const& _operation) { // Inc, Dec, Add, Sub, Not, BitNot, Delete - Token::Value op = _operation.getOperator(); - bool const modifying = (op == Token::Value::Inc || op == Token::Value::Dec || op == Token::Value::Delete); + Token op = _operation.getOperator(); + bool const modifying = (op == Token::Inc || op == Token::Dec || op == Token::Delete); if (modifying) requireLValue(_operation.subExpression()); else @@ -1556,7 +1620,7 @@ bool TypeChecker::visit(UnaryOperation const& _operation) m_errorReporter.typeError( _operation.location(), "Unary operator " + - string(Token::toString(op)) + + string(TokenTraits::toString(op)) + " cannot be applied to type " + subExprType->toString() ); @@ -1577,7 +1641,7 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) m_errorReporter.typeError( _operation.location(), "Operator " + - string(Token::toString(_operation.getOperator())) + + string(TokenTraits::toString(_operation.getOperator())) + " not compatible with types " + leftType->toString() + " and " + @@ -1587,7 +1651,7 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) } _operation.annotation().commonType = commonType; _operation.annotation().type = - Token::isCompareOp(_operation.getOperator()) ? + TokenTraits::isCompareOp(_operation.getOperator()) ? make_shared<BoolType>() : commonType; _operation.annotation().isPure = @@ -1617,61 +1681,91 @@ void TypeChecker::endVisit(BinaryOperation const& _operation) } } -bool TypeChecker::visit(FunctionCall const& _functionCall) +TypePointer TypeChecker::typeCheckTypeConversionAndRetrieveReturnType( + FunctionCall const& _functionCall +) { - bool isPositionalCall = _functionCall.names().empty(); - vector<ASTPointer<Expression const>> arguments = _functionCall.arguments(); - vector<ASTPointer<ASTString>> const& argumentNames = _functionCall.names(); - - bool isPure = true; - - // We need to check arguments' type first as they will be needed for overload resolution. - shared_ptr<TypePointers> argumentTypes; - if (isPositionalCall) - argumentTypes = make_shared<TypePointers>(); - for (ASTPointer<Expression const> const& argument: arguments) - { - argument->accept(*this); - if (!argument->annotation().isPure) - isPure = false; - // only store them for positional calls - if (isPositionalCall) - argumentTypes->push_back(type(*argument)); - } - if (isPositionalCall) - _functionCall.expression().annotation().argumentTypes = move(argumentTypes); + solAssert(_functionCall.annotation().kind == FunctionCallKind::TypeConversion, ""); + TypePointer const& expressionType = type(_functionCall.expression()); - _functionCall.expression().accept(*this); - TypePointer expressionType = type(_functionCall.expression()); + vector<ASTPointer<Expression const>> const& arguments = _functionCall.arguments(); + bool const isPositionalCall = _functionCall.names().empty(); - if (auto const* typeType = dynamic_cast<TypeType const*>(expressionType.get())) - { - if (typeType->actualType()->category() == Type::Category::Struct) - _functionCall.annotation().kind = FunctionCallKind::StructConstructorCall; - else - _functionCall.annotation().kind = FunctionCallKind::TypeConversion; - - } + TypePointer resultType = dynamic_cast<TypeType const&>(*expressionType).actualType(); + if (arguments.size() != 1) + m_errorReporter.typeError( + _functionCall.location(), + "Exactly one argument expected for explicit type conversion." + ); + else if (!isPositionalCall) + m_errorReporter.typeError( + _functionCall.location(), + "Type conversion cannot allow named arguments." + ); else - _functionCall.annotation().kind = FunctionCallKind::FunctionCall; - solAssert(_functionCall.annotation().kind != FunctionCallKind::Unset, ""); - - if (_functionCall.annotation().kind == FunctionCallKind::TypeConversion) { - TypeType const& t = dynamic_cast<TypeType const&>(*expressionType); - TypePointer resultType = t.actualType(); - if (arguments.size() != 1) - m_errorReporter.typeError(_functionCall.location(), "Exactly one argument expected for explicit type conversion."); - else if (!isPositionalCall) - m_errorReporter.typeError(_functionCall.location(), "Type conversion cannot allow named arguments."); + TypePointer const& argType = type(*arguments.front()); + // Resulting data location is memory unless we are converting from a reference + // type with a different data location. + // (data location cannot yet be specified for type conversions) + DataLocation dataLoc = DataLocation::Memory; + if (auto argRefType = dynamic_cast<ReferenceType const*>(argType.get())) + dataLoc = argRefType->location(); + if (auto type = dynamic_cast<ReferenceType const*>(resultType.get())) + resultType = type->copyForLocation(dataLoc, type->isPointer()); + if (argType->isExplicitlyConvertibleTo(*resultType)) + { + if (auto argArrayType = dynamic_cast<ArrayType const*>(argType.get())) + { + auto resultArrayType = dynamic_cast<ArrayType const*>(resultType.get()); + solAssert(!!resultArrayType, ""); + solAssert( + argArrayType->location() != DataLocation::Storage || + ( + ( + resultArrayType->isPointer() || + (argArrayType->isByteArray() && resultArrayType->isByteArray()) + ) && + resultArrayType->location() == DataLocation::Storage + ), + "Invalid explicit conversion to storage type." + ); + } + } else { - TypePointer const& argType = type(*arguments.front()); - if (auto argRefType = dynamic_cast<ReferenceType const*>(argType.get())) - // do not change the data location when converting - // (data location cannot yet be specified for type conversions) - resultType = ReferenceType::copyForLocationIfReference(argRefType->location(), resultType); - if (!argType->isExplicitlyConvertibleTo(*resultType)) + if ( + resultType->category() == Type::Category::Contract && + argType->category() == Type::Category::Address + ) + { + solAssert(dynamic_cast<ContractType const*>(resultType.get())->isPayable(), ""); + solAssert( + dynamic_cast<AddressType const*>(argType.get())->stateMutability() < + StateMutability::Payable, + "" + ); + SecondarySourceLocation ssl; + if ( + auto const* identifier = dynamic_cast<Identifier const*>(arguments.front().get()) + ) + if ( + auto const* variableDeclaration = dynamic_cast<VariableDeclaration const*>( + identifier->annotation().referencedDeclaration + ) + ) + ssl.append( + "Did you mean to declare this variable as \"address payable\"?", + variableDeclaration->location() + ); + m_errorReporter.typeError( + _functionCall.location(), ssl, + "Explicit type conversion not allowed from non-payable \"address\" to \"" + + resultType->toString() + + "\", which has a payable fallback function." + ); + } + else m_errorReporter.typeError( _functionCall.location(), "Explicit type conversion not allowed from \"" + @@ -1681,267 +1775,475 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) "\"." ); } - _functionCall.annotation().type = resultType; - _functionCall.annotation().isPure = isPure; - - return false; + if (resultType->category() == Type::Category::Address) + { + bool const payable = argType->isExplicitlyConvertibleTo(AddressType::addressPayable()); + resultType = make_shared<AddressType>( + payable ? StateMutability::Payable : StateMutability::NonPayable + ); + } } + return resultType; +} +void TypeChecker::typeCheckFunctionCall( + FunctionCall const& _functionCall, + FunctionTypePointer _functionType +) +{ // Actual function call or struct constructor call. - FunctionTypePointer functionType; + solAssert(!!_functionType, ""); + solAssert(_functionType->kind() != FunctionType::Kind::ABIDecode, ""); + + // Check for unsupported use of bare static call + if ( + _functionType->kind() == FunctionType::Kind::BareStaticCall && + !m_evmVersion.hasStaticCall() + ) + m_errorReporter.typeError( + _functionCall.location(), + "\"staticcall\" is not supported by the VM version." + ); - /// For error message: Struct members that were removed during conversion to memory. - set<string> membersRemovedForStructConstructor; - if (_functionCall.annotation().kind == FunctionCallKind::StructConstructorCall) + // Check for deprecated function names + if (_functionType->kind() == FunctionType::Kind::KECCAK256) { - TypeType const& t = dynamic_cast<TypeType const&>(*expressionType); - auto const& structType = dynamic_cast<StructType const&>(*t.actualType()); - functionType = structType.constructorType(); - membersRemovedForStructConstructor = structType.membersMissingInMemory(); - _functionCall.annotation().isPure = isPure; + if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression())) + if (functionName->name() == "sha3") + m_errorReporter.typeError( + _functionCall.location(), + "\"sha3\" has been deprecated in favour of \"keccak256\"" + ); } - else if ((functionType = dynamic_pointer_cast<FunctionType const>(expressionType))) - _functionCall.annotation().isPure = - isPure && - _functionCall.expression().annotation().isPure && - functionType->isPure(); - - bool allowDynamicTypes = m_evmVersion.supportsReturndata(); - if (!functionType) + else if (_functionType->kind() == FunctionType::Kind::Selfdestruct) { - m_errorReporter.typeError(_functionCall.location(), "Type is not callable"); - _functionCall.annotation().type = make_shared<TupleType>(); - return false; + if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression())) + if (functionName->name() == "suicide") + m_errorReporter.typeError( + _functionCall.location(), + "\"suicide\" has been deprecated in favour of \"selfdestruct\"" + ); } - auto returnTypes = - allowDynamicTypes ? - functionType->returnParameterTypes() : - functionType->returnParameterTypesWithoutDynamicTypes(); - if (returnTypes.size() == 1) - _functionCall.annotation().type = returnTypes.front(); - else - _functionCall.annotation().type = make_shared<TupleType>(returnTypes); + // Check for event outside of emit statement + if (!m_insideEmitStatement && _functionType->kind() == FunctionType::Kind::Event) + m_errorReporter.typeError( + _functionCall.location(), + "Event invocations have to be prefixed by \"emit\"." + ); + + // Perform standard function call type checking + typeCheckFunctionGeneralChecks(_functionCall, _functionType); +} - bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); +void TypeChecker::typeCheckABIEncodeFunctions( + FunctionCall const& _functionCall, + FunctionTypePointer _functionType +) +{ + solAssert(!!_functionType, ""); + solAssert( + _functionType->kind() == FunctionType::Kind::ABIEncode || + _functionType->kind() == FunctionType::Kind::ABIEncodePacked || + _functionType->kind() == FunctionType::Kind::ABIEncodeWithSelector || + _functionType->kind() == FunctionType::Kind::ABIEncodeWithSignature, + "ABI function has unexpected FunctionType::Kind." + ); + solAssert(_functionType->takesArbitraryParameters(), "ABI functions should be variadic."); + + bool const isPacked = _functionType->kind() == FunctionType::Kind::ABIEncodePacked; + solAssert(_functionType->padArguments() != isPacked, "ABI function with unexpected padding"); - if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression())) + bool const abiEncoderV2 = m_scope->sourceUnit().annotation().experimentalFeatures.count( + ExperimentalFeature::ABIEncoderV2 + ); + + // Check for named arguments + if (!_functionCall.names().empty()) { - string msg; - if (functionName->name() == "sha3" && functionType->kind() == FunctionType::Kind::SHA3) - msg = "\"sha3\" has been deprecated in favour of \"keccak256\""; - else if (functionName->name() == "suicide" && functionType->kind() == FunctionType::Kind::Selfdestruct) - msg = "\"suicide\" has been deprecated in favour of \"selfdestruct\""; - if (!msg.empty()) + m_errorReporter.typeError( + _functionCall.location(), + "Named arguments cannot be used for functions that take arbitrary parameters." + ); + return; + } + + // Perform standard function call type checking + typeCheckFunctionGeneralChecks(_functionCall, _functionType); + + // Check additional arguments for variadic functions + vector<ASTPointer<Expression const>> const& arguments = _functionCall.arguments(); + for (size_t i = 0; i < arguments.size(); ++i) + { + auto const& argType = type(*arguments[i]); + + if (argType->category() == Type::Category::RationalNumber) { - if (v050) - m_errorReporter.typeError(_functionCall.location(), msg); - else - m_errorReporter.warning(_functionCall.location(), msg); + if (!argType->mobileType()) + { + m_errorReporter.typeError( + arguments[i]->location(), + "Invalid rational number (too large or division by zero)." + ); + continue; + } + else if (isPacked) + { + m_errorReporter.typeError( + arguments[i]->location(), + "Cannot perform packed encoding for a literal." + " Please convert it to an explicit type first." + ); + continue; + } } + + if (!argType->fullEncodingType(false, abiEncoderV2, !_functionType->padArguments())) + m_errorReporter.typeError( + arguments[i]->location(), + "This type cannot be encoded." + ); } - if (!m_insideEmitStatement && functionType->kind() == FunctionType::Kind::Event) +} + +void TypeChecker::typeCheckFunctionGeneralChecks( + FunctionCall const& _functionCall, + FunctionTypePointer _functionType +) +{ + // Actual function call or struct constructor call. + + solAssert(!!_functionType, ""); + solAssert(_functionType->kind() != FunctionType::Kind::ABIDecode, ""); + + bool const isPositionalCall = _functionCall.names().empty(); + bool const isVariadic = _functionType->takesArbitraryParameters(); + + solAssert( + !isVariadic || _functionCall.annotation().kind == FunctionCallKind::FunctionCall, + "Struct constructor calls cannot be variadic." + ); + + TypePointers const& parameterTypes = _functionType->parameterTypes(); + vector<ASTPointer<Expression const>> const& arguments = _functionCall.arguments(); + vector<ASTPointer<ASTString>> const& argumentNames = _functionCall.names(); + + // Check number of passed in arguments + if ( + arguments.size() < parameterTypes.size() || + (!isVariadic && arguments.size() > parameterTypes.size()) + ) { - if (m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050)) - m_errorReporter.typeError(_functionCall.location(), "Event invocations have to be prefixed by \"emit\"."); + bool const isStructConstructorCall = + _functionCall.annotation().kind == FunctionCallKind::StructConstructorCall; + + string msg; + + if (isVariadic) + msg += + "Need at least " + + toString(parameterTypes.size()) + + " arguments for " + + string(isStructConstructorCall ? "struct constructor" : "function call") + + ", but provided only " + + toString(arguments.size()) + + "."; else - m_errorReporter.warning(_functionCall.location(), "Invoking events without \"emit\" prefix is deprecated."); + msg += + "Wrong argument count for " + + string(isStructConstructorCall ? "struct constructor" : "function call") + + ": " + + toString(arguments.size()) + + " arguments given but " + + string(isVariadic ? "need at least " : "expected ") + + toString(parameterTypes.size()) + + "."; + + // Extend error message in case we try to construct a struct with mapping member. + if (isStructConstructorCall) + { + /// For error message: Struct members that were removed during conversion to memory. + TypePointer const expressionType = type(_functionCall.expression()); + TypeType const& t = dynamic_cast<TypeType const&>(*expressionType); + auto const& structType = dynamic_cast<StructType const&>(*t.actualType()); + set<string> membersRemovedForStructConstructor = structType.membersMissingInMemory(); + + if (!membersRemovedForStructConstructor.empty()) + { + msg += " Members that have to be skipped in memory:"; + for (auto const& member: membersRemovedForStructConstructor) + msg += " " + member; + } + } + else if ( + _functionType->kind() == FunctionType::Kind::BareCall || + _functionType->kind() == FunctionType::Kind::BareCallCode || + _functionType->kind() == FunctionType::Kind::BareDelegateCall || + _functionType->kind() == FunctionType::Kind::BareStaticCall + ) + { + if (arguments.empty()) + msg += + " This function requires a single bytes argument." + " Use \"\" as argument to provide empty calldata."; + else + msg += + " This function requires a single bytes argument." + " If all your arguments are value types, you can use" + " abi.encode(...) to properly generate it."; + } + else if ( + _functionType->kind() == FunctionType::Kind::KECCAK256 || + _functionType->kind() == FunctionType::Kind::SHA256 || + _functionType->kind() == FunctionType::Kind::RIPEMD160 + ) + msg += + " This function requires a single bytes argument." + " Use abi.encodePacked(...) to obtain the pre-0.5.0" + " behaviour or abi.encode(...) to use ABI encoding."; + m_errorReporter.typeError(_functionCall.location(), msg); + return; } - TypePointers parameterTypes = functionType->parameterTypes(); + // Parameter to argument map + std::vector<Expression const*> paramArgMap(parameterTypes.size()); - if (!functionType->padArguments()) + // Map parameters to arguments - trivially for positional calls, less so for named calls + if (isPositionalCall) + for (size_t i = 0; i < paramArgMap.size(); ++i) + paramArgMap[i] = arguments[i].get(); + else { - for (size_t i = 0; i < arguments.size(); ++i) + auto const& parameterNames = _functionType->parameterNames(); + + // Check for expected number of named arguments + if (parameterNames.size() != argumentNames.size()) { - auto const& argType = type(*arguments[i]); - if (auto literal = dynamic_cast<RationalNumberType const*>(argType.get())) - { - /* If no mobile type is available an error will be raised elsewhere. */ - if (literal->mobileType()) - { - if (v050) + m_errorReporter.typeError( + _functionCall.location(), + parameterNames.size() > argumentNames.size() ? + "Some argument names are missing." : + "Too many arguments." + ); + return; + } + + // Check for duplicate argument names + { + bool duplication = false; + for (size_t i = 0; i < argumentNames.size(); i++) + for (size_t j = i + 1; j < argumentNames.size(); j++) + if (*argumentNames[i] == *argumentNames[j]) + { + duplication = true; m_errorReporter.typeError( arguments[i]->location(), - "Cannot perform packed encoding for a literal. Please convert it to an explicit type first." - ); - else - m_errorReporter.warning( - arguments[i]->location(), - "The type of \"" + - argType->toString() + - "\" was inferred as " + - literal->mobileType()->toString() + - ". This is probably not desired. Use an explicit type to silence this warning." + "Duplicate named argument \"" + *argumentNames[i] + "\"." ); + } + if (duplication) + return; + } + + // map parameter names to argument names + { + bool not_all_mapped = false; + + for (size_t i = 0; i < paramArgMap.size(); i++) + { + size_t j; + for (j = 0; j < argumentNames.size(); j++) + if (parameterNames[i] == *argumentNames[j]) + break; + + if (j < argumentNames.size()) + paramArgMap[i] = arguments[j].get(); + else + { + paramArgMap[i] = nullptr; + not_all_mapped = true; + m_errorReporter.typeError( + _functionCall.location(), + "Named argument \"" + + *argumentNames[i] + + "\" does not match function declaration." + ); } } + + if (not_all_mapped) + return; } } - if (functionType->takesSinglePackedBytesParameter()) + // Check for compatible types between arguments and parameters + for (size_t i = 0; i < paramArgMap.size(); ++i) { - if ( - (arguments.size() > 1) || - (arguments.size() == 1 && !type(*arguments.front())->isImplicitlyConvertibleTo(ArrayType(DataLocation::Memory))) - ) + solAssert(!!paramArgMap[i], "unmapped parameter"); + if (!type(*paramArgMap[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) { string msg = - "This function only accepts a single \"bytes\" argument. Please use " - "\"abi.encodePacked(...)\" or a similar function to encode the data."; - if (v050) - m_errorReporter.typeError(_functionCall.location(), msg); - else - m_errorReporter.warning(_functionCall.location(), msg); + "Invalid type for argument in function call. " + "Invalid implicit conversion from " + + type(*paramArgMap[i])->toString() + + " to " + + parameterTypes[i]->toString() + + " requested."; + if ( + _functionType->kind() == FunctionType::Kind::BareCall || + _functionType->kind() == FunctionType::Kind::BareCallCode || + _functionType->kind() == FunctionType::Kind::BareDelegateCall || + _functionType->kind() == FunctionType::Kind::BareStaticCall + ) + msg += + " This function requires a single bytes argument." + " If all your arguments are value types, you can" + " use abi.encode(...) to properly generate it."; + else if ( + _functionType->kind() == FunctionType::Kind::KECCAK256 || + _functionType->kind() == FunctionType::Kind::SHA256 || + _functionType->kind() == FunctionType::Kind::RIPEMD160 + ) + msg += + " This function requires a single bytes argument." + " Use abi.encodePacked(...) to obtain the pre-0.5.0" + " behaviour or abi.encode(...) to use ABI encoding."; + m_errorReporter.typeError(paramArgMap[i]->location(), msg); } + } +} - if (arguments.size() == 1 && !type(*arguments.front())->isImplicitlyConvertibleTo(ArrayType(DataLocation::Memory))) - { - string msg = - "The provided argument of type " + - type(*arguments.front())->toString() + - " is not implicitly convertible to expected type bytes memory."; - if (v050) - m_errorReporter.typeError(_functionCall.location(), msg); - else - m_errorReporter.warning(_functionCall.location(), msg); - } +bool TypeChecker::visit(FunctionCall const& _functionCall) +{ + vector<ASTPointer<Expression const>> const& arguments = _functionCall.arguments(); + bool argumentsArePure = true; + + // We need to check arguments' type first as they will be needed for overload resolution. + for (ASTPointer<Expression const> const& argument: arguments) + { + argument->accept(*this); + if (!argument->annotation().isPure) + argumentsArePure = false; } - if (functionType->takesArbitraryParameters() && arguments.size() < parameterTypes.size()) + // For positional calls only, store argument types + if (_functionCall.names().empty()) { - solAssert(_functionCall.annotation().kind == FunctionCallKind::FunctionCall, ""); - m_errorReporter.typeError( - _functionCall.location(), - "Need at least " + - toString(parameterTypes.size()) + - " arguments for function call, but provided only " + - toString(arguments.size()) + - "." - ); + shared_ptr<TypePointers> argumentTypes = make_shared<TypePointers>(); + for (ASTPointer<Expression const> const& argument: arguments) + argumentTypes->push_back(type(*argument)); + _functionCall.expression().annotation().argumentTypes = move(argumentTypes); } - else if (!functionType->takesArbitraryParameters() && parameterTypes.size() != arguments.size()) + + _functionCall.expression().accept(*this); + + TypePointer const& expressionType = type(_functionCall.expression()); + + // Determine function call kind and function type for this FunctionCall node + FunctionCallAnnotation& funcCallAnno = _functionCall.annotation(); + FunctionTypePointer functionType; + + // Determine and assign function call kind, purity and function type for this FunctionCall node + switch (expressionType->category()) { - bool isStructConstructorCall = _functionCall.annotation().kind == FunctionCallKind::StructConstructorCall; + case Type::Category::Function: + functionType = dynamic_pointer_cast<FunctionType const>(expressionType); + funcCallAnno.kind = FunctionCallKind::FunctionCall; - string msg = - "Wrong argument count for " + - string(isStructConstructorCall ? "struct constructor" : "function call") + - ": " + - toString(arguments.size()) + - " arguments given but expected " + - toString(parameterTypes.size()) + - "."; - // Extend error message in case we try to construct a struct with mapping member. - if (_functionCall.annotation().kind == FunctionCallKind::StructConstructorCall && !membersRemovedForStructConstructor.empty()) + // Purity for function calls also depends upon the callee and its FunctionType + funcCallAnno.isPure = + argumentsArePure && + _functionCall.expression().annotation().isPure && + functionType && + functionType->isPure(); + + break; + + case Type::Category::TypeType: + { + // Determine type for type conversion or struct construction expressions + TypePointer const& actualType = dynamic_cast<TypeType const&>(*expressionType).actualType(); + solAssert(!!actualType, ""); + + if (actualType->category() == Type::Category::Struct) { - msg += " Members that have to be skipped in memory:"; - for (auto const& member: membersRemovedForStructConstructor) - msg += " " + member; + functionType = dynamic_cast<StructType const&>(*actualType).constructorType(); + funcCallAnno.kind = FunctionCallKind::StructConstructorCall; + funcCallAnno.isPure = argumentsArePure; } - m_errorReporter.typeError(_functionCall.location(), msg); + else + { + funcCallAnno.kind = FunctionCallKind::TypeConversion; + funcCallAnno.isPure = argumentsArePure; + } + + break; + } + + default: + m_errorReporter.typeError(_functionCall.location(), "Type is not callable"); + funcCallAnno.kind = FunctionCallKind::Unset; + funcCallAnno.isPure = argumentsArePure; + break; } - else if (isPositionalCall) + + // Determine return types + switch (funcCallAnno.kind) + { + case FunctionCallKind::TypeConversion: + funcCallAnno.type = typeCheckTypeConversionAndRetrieveReturnType(_functionCall); + break; + + case FunctionCallKind::StructConstructorCall: // fall-through + case FunctionCallKind::FunctionCall: { - bool const abiEncodeV2 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2); + TypePointers returnTypes; - for (size_t i = 0; i < arguments.size(); ++i) + switch (functionType->kind()) { - auto const& argType = type(*arguments[i]); - if (functionType->takesArbitraryParameters() && i >= parameterTypes.size()) - { - bool errored = false; - if (auto t = dynamic_cast<RationalNumberType const*>(argType.get())) - if (!t->mobileType()) - { - m_errorReporter.typeError(arguments[i]->location(), "Invalid rational number (too large or division by zero)."); - errored = true; - } - if (!errored) - { - TypePointer encodingType; - if ( - argType->mobileType() && - argType->mobileType()->interfaceType(false) && - argType->mobileType()->interfaceType(false)->encodingType() - ) - encodingType = argType->mobileType()->interfaceType(false)->encodingType(); - // Structs are fine as long as ABIV2 is activated and we do not do packed encoding. - if (!encodingType || ( - dynamic_cast<StructType const*>(encodingType.get()) && - !(abiEncodeV2 && functionType->padArguments()) - )) - m_errorReporter.typeError(arguments[i]->location(), "This type cannot be encoded."); - } - } - else if (!type(*arguments[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) - m_errorReporter.typeError( - arguments[i]->location(), - "Invalid type for argument in function call. " - "Invalid implicit conversion from " + - type(*arguments[i])->toString() + - " to " + - parameterTypes[i]->toString() + - " requested." + case FunctionType::Kind::ABIDecode: + { + bool const abiEncoderV2 = + m_scope->sourceUnit().annotation().experimentalFeatures.count( + ExperimentalFeature::ABIEncoderV2 ); + returnTypes = typeCheckABIDecodeAndRetrieveReturnType(_functionCall, abiEncoderV2); + break; } - } - else - { - // call by named arguments - auto const& parameterNames = functionType->parameterNames(); - if (functionType->takesArbitraryParameters()) - m_errorReporter.typeError( - _functionCall.location(), - "Named arguments cannnot be used for functions that take arbitrary parameters." - ); - else if (parameterNames.size() > argumentNames.size()) - m_errorReporter.typeError(_functionCall.location(), "Some argument names are missing."); - else if (parameterNames.size() < argumentNames.size()) - m_errorReporter.typeError(_functionCall.location(), "Too many arguments."); - else + case FunctionType::Kind::ABIEncode: + case FunctionType::Kind::ABIEncodePacked: + case FunctionType::Kind::ABIEncodeWithSelector: + case FunctionType::Kind::ABIEncodeWithSignature: { - // check duplicate names - bool duplication = false; - for (size_t i = 0; i < argumentNames.size(); i++) - for (size_t j = i + 1; j < argumentNames.size(); j++) - if (*argumentNames[i] == *argumentNames[j]) - { - duplication = true; - m_errorReporter.typeError(arguments[i]->location(), "Duplicate named argument."); - } - - // check actual types - if (!duplication) - for (size_t i = 0; i < argumentNames.size(); i++) - { - bool found = false; - for (size_t j = 0; j < parameterNames.size(); j++) - if (parameterNames[j] == *argumentNames[i]) - { - found = true; - // check type convertible - if (!type(*arguments[i])->isImplicitlyConvertibleTo(*parameterTypes[j])) - m_errorReporter.typeError( - arguments[i]->location(), - "Invalid type for argument in function call. " - "Invalid implicit conversion from " + - type(*arguments[i])->toString() + - " to " + - parameterTypes[i]->toString() + - " requested." - ); - break; - } - - if (!found) - m_errorReporter.typeError( - _functionCall.location(), - "Named argument does not match function declaration." - ); - } + typeCheckABIEncodeFunctions(_functionCall, functionType); + returnTypes = functionType->returnParameterTypes(); + break; + } + default: + { + typeCheckFunctionCall(_functionCall, functionType); + returnTypes = m_evmVersion.supportsReturndata() ? + functionType->returnParameterTypes() : + functionType->returnParameterTypesWithoutDynamicTypes(); + break; + } } + + funcCallAnno.type = returnTypes.size() == 1 ? + move(returnTypes.front()) : + make_shared<TupleType>(move(returnTypes)); + + break; + } + + case FunctionCallKind::Unset: // fall-through + default: + // for non-callables, ensure error reported and annotate node to void function + solAssert(m_errorReporter.hasErrors(), ""); + funcCallAnno.kind = FunctionCallKind::FunctionCall; + funcCallAnno.type = make_shared<TupleType>(); + break; } return false; @@ -1959,7 +2261,7 @@ void TypeChecker::endVisit(NewExpression const& _newExpression) if (!contract) m_errorReporter.fatalTypeError(_newExpression.location(), "Identifier is not a contract."); if (contract->contractKind() == ContractDefinition::ContractKind::Interface) - m_errorReporter.fatalTypeError(_newExpression.location(), "Cannot instantiate an interface."); + m_errorReporter.fatalTypeError(_newExpression.location(), "Cannot instantiate an interface."); if (!contract->annotation().unimplementedFunctions.empty()) { SecondarySourceLocation ssl; @@ -2040,7 +2342,10 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) else ++it; } - if (possibleMembers.size() == 0) + + auto& annotation = _memberAccess.annotation(); + + if (possibleMembers.empty()) { if (initialMemberCount == 0) { @@ -2057,11 +2362,38 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) " outside of storage." ); } + string errorMsg = "Member \"" + memberName + "\" not found or not visible " + "after argument-dependent lookup in " + exprType->toString() + "."; + if (memberName == "value") + { + errorMsg.pop_back(); + errorMsg += " - did you forget the \"payable\" modifier?"; + } + else if (exprType->category() == Type::Category::Function) + { + if (auto const& funType = dynamic_pointer_cast<FunctionType const>(exprType)) + { + auto const& t = funType->returnParameterTypes(); + if (t.size() == 1) + if ( + t.front()->category() == Type::Category::Contract || + t.front()->category() == Type::Category::Struct + ) + errorMsg += " Did you intend to call the function?"; + } + } + if (exprType->category() == Type::Category::Contract) + for (auto const& addressMember: AddressType::addressPayable().nativeMembers(nullptr)) + if (addressMember.name == memberName) + { + Identifier const* var = dynamic_cast<Identifier const*>(&_memberAccess.expression()); + string varName = var ? var->name() : "..."; + errorMsg += " Use \"address(" + varName + ")." + memberName + "\" to access this address member."; + break; + } m_errorReporter.fatalTypeError( _memberAccess.location(), - "Member \"" + memberName + "\" not found or not visible " - "after argument-dependent lookup in " + exprType->toString() + - (memberName == "value" ? " - did you forget the \"payable\" modifier?" : "") + errorMsg ); } else if (possibleMembers.size() > 1) @@ -2069,10 +2401,9 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) _memberAccess.location(), "Member \"" + memberName + "\" not unique " "after argument-dependent lookup in " + exprType->toString() + - (memberName == "value" ? " - did you forget the \"payable\" modifier?" : "") + (memberName == "value" ? " - did you forget the \"payable\" modifier?" : ".") ); - auto& annotation = _memberAccess.annotation(); annotation.referencedDeclaration = possibleMembers.front().declaration; annotation.type = possibleMembers.front().type; @@ -2081,7 +2412,7 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) m_errorReporter.typeError( _memberAccess.location(), "Function \"" + memberName + "\" cannot be called on an object of type " + - exprType->toString() + " (expected " + funType->selfType()->toString() + ")" + exprType->toString() + " (expected " + funType->selfType()->toString() + ")." ); if (exprType->category() == Type::Category::Struct) @@ -2105,20 +2436,6 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) if (exprType->category() == Type::Category::Contract) { - // Warn about using address members on contracts - bool v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - for (auto const& addressMember: IntegerType(160, IntegerType::Modifier::Address).nativeMembers(nullptr)) - if (addressMember.name == memberName && *annotation.type == *addressMember.type) - { - solAssert(!v050, "Address member still present on contract in v0.5.0."); - m_errorReporter.warning( - _memberAccess.location(), - "Using contract member \"" + memberName +"\" inherited from the address type is deprecated." + - " Convert the contract to \"address\" type to access the member," - " for example use \"address(contract)." + memberName + "\" instead." - ); - } - // Warn about using send or transfer with a non-payable fallback function. if (auto callType = dynamic_cast<FunctionType const*>(type(_memberAccess).get())) { @@ -2172,12 +2489,13 @@ bool TypeChecker::visit(IndexAccess const& _access) else { expectType(*index, IntegerType(256)); - if (auto numberType = dynamic_cast<RationalNumberType const*>(type(*index).get())) - { - if (!numberType->isFractional()) // error is reported above + if (!m_errorReporter.hasErrors()) + if (auto numberType = dynamic_cast<RationalNumberType const*>(type(*index).get())) + { + solAssert(!numberType->isFractional(), ""); if (!actualType.isDynamicallySized() && actualType.length() <= numberType->literalValue(nullptr)) m_errorReporter.typeError(_access.location(), "Out of bounds array access."); - } + } } resultType = actualType.baseType(); isLValue = actualType.location() != DataLocation::CallData; @@ -2220,7 +2538,8 @@ bool TypeChecker::visit(IndexAccess const& _access) m_errorReporter.typeError(_access.location(), "Index expression cannot be omitted."); else { - expectType(*index, IntegerType(256)); + if (!expectType(*index, IntegerType(256))) + m_errorReporter.fatalTypeError(_access.location(), "Index expression cannot be represented as an unsigned integer."); if (auto integerType = dynamic_cast<RationalNumberType const*>(type(*index).get())) if (bytesType.numBytes() <= integerType->literalValue(nullptr)) m_errorReporter.typeError(_access.location(), "Out of bounds array access."); @@ -2313,51 +2632,46 @@ void TypeChecker::endVisit(ElementaryTypeNameExpression const& _expr) void TypeChecker::endVisit(Literal const& _literal) { - bool const v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - if (_literal.looksLikeAddress()) { - if (_literal.passesAddressChecksum()) - _literal.annotation().type = make_shared<IntegerType>(160, IntegerType::Modifier::Address); - else - m_errorReporter.warning( + // Assign type here if it even looks like an address. This prevents double errors for invalid addresses + _literal.annotation().type = make_shared<AddressType>(StateMutability::Payable); + + string msg; + if (_literal.valueWithoutUnderscores().length() != 42) // "0x" + 40 hex digits + // looksLikeAddress enforces that it is a hex literal starting with "0x" + msg = + "This looks like an address but is not exactly 40 hex digits. It is " + + to_string(_literal.valueWithoutUnderscores().length() - 2) + + " hex digits."; + else if (!_literal.passesAddressChecksum()) + { + msg = "This looks like an address but has an invalid checksum."; + if (!_literal.getChecksummedAddress().empty()) + msg += " Correct checksummed address: \"" + _literal.getChecksummedAddress() + "\"."; + } + + if (!msg.empty()) + m_errorReporter.syntaxError( _literal.location(), - "This looks like an address but has an invalid checksum. " - "If this is not used as an address, please prepend '00'. " + - (!_literal.getChecksummedAddress().empty() ? "Correct checksummed address: '" + _literal.getChecksummedAddress() + "'. " : "") + + msg + + " If this is not used as an address, please prepend '00'. " + "For more information please see https://solidity.readthedocs.io/en/develop/types.html#address-literals" ); } if (_literal.isHexNumber() && _literal.subDenomination() != Literal::SubDenomination::None) - { - if (v050) - m_errorReporter.fatalTypeError( - _literal.location(), - "Hexadecimal numbers cannot be used with unit denominations. " - "You can use an expression of the form \"0x1234 * 1 day\" instead." - ); - else - m_errorReporter.warning( - _literal.location(), - "Hexadecimal numbers with unit denominations are deprecated. " - "You can use an expression of the form \"0x1234 * 1 day\" instead." - ); - } + m_errorReporter.fatalTypeError( + _literal.location(), + "Hexadecimal numbers cannot be used with unit denominations. " + "You can use an expression of the form \"0x1234 * 1 day\" instead." + ); if (_literal.subDenomination() == Literal::SubDenomination::Year) - { - if (v050) - m_errorReporter.typeError( - _literal.location(), - "Using \"years\" as a unit denomination is deprecated." - ); - else - m_errorReporter.warning( - _literal.location(), - "Using \"years\" as a unit denomination is deprecated." - ); - } + m_errorReporter.typeError( + _literal.location(), + "Using \"years\" as a unit denomination is deprecated." + ); if (!_literal.annotation().type) _literal.annotation().type = Type::forLiteral(_literal); @@ -2396,7 +2710,7 @@ Declaration const& TypeChecker::dereference(UserDefinedTypeName const& _typeName return *_typeName.annotation().referencedDeclaration; } -void TypeChecker::expectType(Expression const& _expression, Type const& _expectedType) +bool TypeChecker::expectType(Expression const& _expression, Type const& _expectedType) { _expression.accept(*this); if (!type(_expression)->isImplicitlyConvertibleTo(_expectedType)) @@ -2425,23 +2739,9 @@ void TypeChecker::expectType(Expression const& _expression, Type const& _expecte _expectedType.toString() + "." ); + return false; } - - if ( - type(_expression)->category() == Type::Category::RationalNumber && - _expectedType.category() == Type::Category::FixedBytes - ) - { - auto literal = dynamic_cast<Literal const*>(&_expression); - - if (literal && !literal->isHexNumber()) - m_errorReporter.warning( - _expression.location(), - "Decimal literal assigned to bytesXX variable will be left-aligned. " - "Use an explicit conversion to silence this warning." - ); - } - + return true; } void TypeChecker::requireLValue(Expression const& _expression) diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 2245abd6..c76fa466 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -68,7 +68,7 @@ private: void checkContractDuplicateFunctions(ContractDefinition const& _contract); void checkContractDuplicateEvents(ContractDefinition const& _contract); void checkContractIllegalOverrides(ContractDefinition const& _contract); - /// Reports a type error with an appropiate message if overriden function signature differs. + /// Reports a type error with an appropriate message if overridden function signature differs. /// Also stores the direct super function in the AST annotations. void checkFunctionOverride(FunctionDefinition const& function, FunctionDefinition const& super); void overrideError(FunctionDefinition const& function, FunctionDefinition const& super, std::string message); @@ -87,13 +87,46 @@ private: /// Checks (and warns) if a tuple assignment might cause unexpected overwrites in storage. /// Should only be called if the left hand side is tuple-typed. void checkDoubleStorageAssignment(Assignment const& _assignment); + // Checks whether the expression @arg _expression can be assigned from type @arg _type + // and reports an error, if not. + void checkExpressionAssignment(Type const& _type, Expression const& _expression); + + /// Performs type checks for ``abi.decode(bytes memory, (...))`` and returns the + /// vector of return types (which is basically the second argument) if successful. It returns + /// the empty vector on error. + TypePointers typeCheckABIDecodeAndRetrieveReturnType( + FunctionCall const& _functionCall, + bool _abiEncoderV2 + ); + + /// Performs type checks and determines result types for type conversion FunctionCall nodes. + TypePointer typeCheckTypeConversionAndRetrieveReturnType( + FunctionCall const& _functionCall + ); + + /// Performs type checks on function call and struct ctor FunctionCall nodes (except for kind ABIDecode). + void typeCheckFunctionCall( + FunctionCall const& _functionCall, + FunctionTypePointer _functionType + ); + + /// Performs general number and type checks of arguments against function call and struct ctor FunctionCall node parameters. + void typeCheckFunctionGeneralChecks( + FunctionCall const& _functionCall, + FunctionTypePointer _functionType + ); + + /// Performs general checks and checks specific to ABI encode functions + void typeCheckABIEncodeFunctions( + FunctionCall const& _functionCall, + FunctionTypePointer _functionType + ); virtual void endVisit(InheritanceSpecifier const& _inheritance) override; virtual void endVisit(UsingForDirective const& _usingFor) override; virtual bool visit(StructDefinition const& _struct) override; virtual bool visit(FunctionDefinition const& _function) override; virtual bool visit(VariableDeclaration const& _variable) override; - virtual bool visit(EnumDefinition const& _enum) override; /// We need to do this manually because we want to pass the bases of the current contract in /// case this is a base constructor call. void visitManually(ModifierInvocation const& _modifier, std::vector<ContractDefinition const*> const& _bases); @@ -136,7 +169,7 @@ private: /// Runs type checks on @a _expression to infer its type and then checks that it is implicitly /// convertible to @a _expectedType. - void expectType(Expression const& _expression, Type const& _expectedType); + bool expectType(Expression const& _expression, Type const& _expectedType); /// Runs type checks on @a _expression to infer its type and then checks that it is an LValue. void requireLValue(Expression const& _expression); @@ -147,6 +180,9 @@ private: /// Flag indicating whether we are currently inside an EmitStatement. bool m_insideEmitStatement = false; + /// Flag indicating whether we are currently inside a StructDefinition. + bool m_insideStruct = false; + ErrorReporter& m_errorReporter; }; diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index d9843012..b0cacc43 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -116,31 +116,22 @@ private: bool ViewPureChecker::check() { - // The bool means "enforce view with errors". - map<ContractDefinition const*, bool> contracts; + vector<ContractDefinition const*> contracts; for (auto const& node: m_ast) { SourceUnit const* source = dynamic_cast<SourceUnit const*>(node.get()); solAssert(source, ""); - bool enforceView = source->annotation().experimentalFeatures.count(ExperimentalFeature::V050); - for (ContractDefinition const* c: source->filteredNodes<ContractDefinition>(source->nodes())) - contracts[c] = enforceView; + contracts += source->filteredNodes<ContractDefinition>(source->nodes()); } // Check modifiers first to infer their state mutability. for (auto const& contract: contracts) - { - m_enforceViewWithError = contract.second; - for (ModifierDefinition const* mod: contract.first->functionModifiers()) + for (ModifierDefinition const* mod: contract->functionModifiers()) mod->accept(*this); - } for (auto const& contract: contracts) - { - m_enforceViewWithError = contract.second; - contract.first->accept(*this); - } + contract->accept(*this); return !m_errors; } @@ -151,7 +142,7 @@ bool ViewPureChecker::visit(FunctionDefinition const& _funDef) { solAssert(!m_currentFunction, ""); m_currentFunction = &_funDef; - m_currentBestMutability = StateMutability::Pure; + m_bestMutabilityAndLocation = {StateMutability::Pure, _funDef.location()}; return true; } @@ -159,7 +150,7 @@ void ViewPureChecker::endVisit(FunctionDefinition const& _funDef) { solAssert(m_currentFunction == &_funDef, ""); if ( - m_currentBestMutability < _funDef.stateMutability() && + m_bestMutabilityAndLocation.mutability < _funDef.stateMutability() && _funDef.stateMutability() != StateMutability::Payable && _funDef.isImplemented() && !_funDef.isConstructor() && @@ -168,22 +159,22 @@ void ViewPureChecker::endVisit(FunctionDefinition const& _funDef) ) m_errorReporter.warning( _funDef.location(), - "Function state mutability can be restricted to " + stateMutabilityToString(m_currentBestMutability) + "Function state mutability can be restricted to " + stateMutabilityToString(m_bestMutabilityAndLocation.mutability) ); m_currentFunction = nullptr; } -bool ViewPureChecker::visit(ModifierDefinition const&) +bool ViewPureChecker::visit(ModifierDefinition const& _modifier) { solAssert(m_currentFunction == nullptr, ""); - m_currentBestMutability = StateMutability::Pure; + m_bestMutabilityAndLocation = {StateMutability::Pure, _modifier.location()}; return true; } void ViewPureChecker::endVisit(ModifierDefinition const& _modifierDef) { solAssert(m_currentFunction == nullptr, ""); - m_inferredMutability[&_modifierDef] = m_currentBestMutability; + m_inferredMutability[&_modifierDef] = std::move(m_bestMutabilityAndLocation); } void ViewPureChecker::endVisit(Identifier const& _identifier) @@ -228,39 +219,72 @@ void ViewPureChecker::endVisit(InlineAssembly const& _inlineAssembly) }(_inlineAssembly.operations()); } -void ViewPureChecker::reportMutability(StateMutability _mutability, SourceLocation const& _location) +void ViewPureChecker::reportMutability( + StateMutability _mutability, + SourceLocation const& _location, + boost::optional<SourceLocation> const& _nestedLocation +) { - if (m_currentFunction && m_currentFunction->stateMutability() < _mutability) + if (_mutability > m_bestMutabilityAndLocation.mutability) + m_bestMutabilityAndLocation = MutabilityAndLocation{_mutability, _location}; + if (!m_currentFunction || _mutability <= m_currentFunction->stateMutability()) + return; + + // Check for payable here, because any occurrence of `msg.value` + // will set mutability to payable. + if (_mutability == StateMutability::View || ( + _mutability == StateMutability::Payable && + m_currentFunction->stateMutability() == StateMutability::Pure + )) { - string text; - if (_mutability == StateMutability::View) - text = - "Function declared as pure, but this expression (potentially) reads from the " - "environment or state and thus requires \"view\"."; - else if (_mutability == StateMutability::NonPayable) - text = - "Function declared as " + - stateMutabilityToString(m_currentFunction->stateMutability()) + - ", but this expression (potentially) modifies the state and thus " - "requires non-payable (the default) or payable."; - else - solAssert(false, ""); - - solAssert( - m_currentFunction->stateMutability() == StateMutability::View || - m_currentFunction->stateMutability() == StateMutability::Pure, - "" + m_errorReporter.typeError( + _location, + "Function declared as pure, but this expression (potentially) reads from the " + "environment or state and thus requires \"view\"." ); - if (!m_enforceViewWithError && m_currentFunction->stateMutability() == StateMutability::View) - m_errorReporter.warning(_location, text); - else + m_errors = true; + } + else if (_mutability == StateMutability::NonPayable) + { + m_errorReporter.typeError( + _location, + "Function declared as " + + stateMutabilityToString(m_currentFunction->stateMutability()) + + ", but this expression (potentially) modifies the state and thus " + "requires non-payable (the default) or payable." + ); + m_errors = true; + } + else if (_mutability == StateMutability::Payable) + { + // We do not warn for library functions because they cannot be payable anyway. + // Also internal functions should be allowed to use `msg.value`. + if (m_currentFunction->isPublic() && m_currentFunction->inContractKind() != ContractDefinition::ContractKind::Library) { + if (_nestedLocation) + m_errorReporter.typeError( + _location, + SecondarySourceLocation().append("\"msg.value\" appears here inside the modifier.", *_nestedLocation), + "This modifier uses \"msg.value\" and thus the function has to be payable or internal." + ); + else + m_errorReporter.typeError( + _location, + "\"msg.value\" can only be used in payable public functions. Make the function " + "\"payable\" or use an internal function to avoid this error." + ); m_errors = true; - m_errorReporter.typeError(_location, text); } } - if (_mutability > m_currentBestMutability) - m_currentBestMutability = _mutability; + else + solAssert(false, ""); + + solAssert( + m_currentFunction->stateMutability() == StateMutability::View || + m_currentFunction->stateMutability() == StateMutability::Pure || + m_currentFunction->stateMutability() == StateMutability::NonPayable, + "" + ); } void ViewPureChecker::endVisit(FunctionCall const& _functionCall) @@ -268,11 +292,11 @@ void ViewPureChecker::endVisit(FunctionCall const& _functionCall) if (_functionCall.annotation().kind != FunctionCallKind::FunctionCall) return; - StateMutability mut = dynamic_cast<FunctionType const&>(*_functionCall.expression().annotation().type).stateMutability(); + StateMutability mutability = dynamic_cast<FunctionType const&>(*_functionCall.expression().annotation().type).stateMutability(); // We only require "nonpayable" to call a payble function. - if (mut == StateMutability::Payable) - mut = StateMutability::NonPayable; - reportMutability(mut, _functionCall.location()); + if (mutability == StateMutability::Payable) + mutability = StateMutability::NonPayable; + reportMutability(mutability, _functionCall.location()); } bool ViewPureChecker::visit(MemberAccess const& _memberAccess) @@ -299,19 +323,34 @@ void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) ASTString const& member = _memberAccess.memberName(); switch (_memberAccess.expression().annotation().type->category()) { - case Type::Category::Contract: - case Type::Category::Integer: - if (member == "balance" && !_memberAccess.annotation().referencedDeclaration) + case Type::Category::Address: + if (member == "balance") mutability = StateMutability::View; break; case Type::Category::Magic: { - // we can ignore the kind of magic and only look at the name of the member - set<string> static const pureMembers{ - "encode", "encodePacked", "encodeWithSelector", "encodeWithSignature", "data", "sig", "blockhash" + using MagicMember = pair<MagicType::Kind, string>; + set<MagicMember> static const pureMembers{ + {MagicType::Kind::ABI, "decode"}, + {MagicType::Kind::ABI, "encode"}, + {MagicType::Kind::ABI, "encodePacked"}, + {MagicType::Kind::ABI, "encodeWithSelector"}, + {MagicType::Kind::ABI, "encodeWithSignature"}, + {MagicType::Kind::Block, "blockhash"}, + {MagicType::Kind::Message, "data"}, + {MagicType::Kind::Message, "sig"} }; - if (!pureMembers.count(member)) + set<MagicMember> static const payableMembers{ + {MagicType::Kind::Message, "value"} + }; + + auto const& type = dynamic_cast<MagicType const&>(*_memberAccess.expression().annotation().type); + MagicMember magicMember(type.kind(), member); + + if (!pureMembers.count(magicMember)) mutability = StateMutability::View; + if (payableMembers.count(magicMember)) + mutability = StateMutability::Payable; break; } case Type::Category::Struct: @@ -351,7 +390,8 @@ void ViewPureChecker::endVisit(ModifierInvocation const& _modifier) if (ModifierDefinition const* mod = dynamic_cast<decltype(mod)>(_modifier.name()->annotation().referencedDeclaration)) { solAssert(m_inferredMutability.count(mod), ""); - reportMutability(m_inferredMutability.at(mod), _modifier.location()); + auto const& mutAndLocation = m_inferredMutability.at(mod); + reportMutability(mutAndLocation.mutability, _modifier.location(), mutAndLocation.location); } else solAssert(dynamic_cast<ContractDefinition const*>(_modifier.name()->annotation().referencedDeclaration), ""); diff --git a/libsolidity/analysis/ViewPureChecker.h b/libsolidity/analysis/ViewPureChecker.h index 0b882cd8..faa5b698 100644 --- a/libsolidity/analysis/ViewPureChecker.h +++ b/libsolidity/analysis/ViewPureChecker.h @@ -31,16 +31,6 @@ namespace dev namespace solidity { -class ASTNode; -class FunctionDefinition; -class ModifierDefinition; -class Identifier; -class MemberAccess; -class IndexAccess; -class ModifierInvocation; -class FunctionCall; -class InlineAssembly; - class ViewPureChecker: private ASTConstVisitor { public: @@ -50,6 +40,11 @@ public: bool check(); private: + struct MutabilityAndLocation + { + StateMutability mutability; + SourceLocation location; + }; virtual bool visit(FunctionDefinition const& _funDef) override; virtual void endVisit(FunctionDefinition const& _funDef) override; @@ -65,16 +60,19 @@ private: /// Called when an element of mutability @a _mutability is encountered. /// Creates appropriate warnings and errors and sets @a m_currentBestMutability. - void reportMutability(StateMutability _mutability, SourceLocation const& _location); + void reportMutability( + StateMutability _mutability, + SourceLocation const& _location, + boost::optional<SourceLocation> const& _nestedLocation = {} + ); std::vector<std::shared_ptr<ASTNode>> const& m_ast; ErrorReporter& m_errorReporter; bool m_errors = false; - bool m_enforceViewWithError = false; - StateMutability m_currentBestMutability = StateMutability::Payable; + MutabilityAndLocation m_bestMutabilityAndLocation = MutabilityAndLocation{StateMutability::Payable, SourceLocation()}; FunctionDefinition const* m_currentFunction = nullptr; - std::map<ModifierDefinition const*, StateMutability> m_inferredMutability; + std::map<ModifierDefinition const*, MutabilityAndLocation> m_inferredMutability; }; } |