diff options
Diffstat (limited to 'libsolidity/analysis')
| -rw-r--r-- | libsolidity/analysis/ConstantEvaluator.cpp | 6 | ||||
| -rw-r--r-- | libsolidity/analysis/ConstantEvaluator.h | 1 | ||||
| -rw-r--r-- | libsolidity/analysis/DeclarationContainer.cpp | 8 | ||||
| -rw-r--r-- | libsolidity/analysis/GlobalContext.cpp | 4 | ||||
| -rw-r--r-- | libsolidity/analysis/NameAndTypeResolver.cpp | 9 | ||||
| -rw-r--r-- | libsolidity/analysis/PostTypeChecker.cpp | 36 | ||||
| -rw-r--r-- | libsolidity/analysis/PostTypeChecker.h | 5 | ||||
| -rw-r--r-- | libsolidity/analysis/StaticAnalyzer.cpp | 111 | ||||
| -rw-r--r-- | libsolidity/analysis/StaticAnalyzer.h | 6 | ||||
| -rw-r--r-- | libsolidity/analysis/SyntaxChecker.cpp | 37 | ||||
| -rw-r--r-- | libsolidity/analysis/SyntaxChecker.h | 2 | ||||
| -rw-r--r-- | libsolidity/analysis/TypeChecker.cpp | 253 | ||||
| -rw-r--r-- | libsolidity/analysis/TypeChecker.h | 7 | ||||
| -rw-r--r-- | libsolidity/analysis/ViewPureChecker.cpp | 7 |
14 files changed, 384 insertions, 108 deletions
diff --git a/libsolidity/analysis/ConstantEvaluator.cpp b/libsolidity/analysis/ConstantEvaluator.cpp index 83f37f47..8659bbfd 100644 --- a/libsolidity/analysis/ConstantEvaluator.cpp +++ b/libsolidity/analysis/ConstantEvaluator.cpp @@ -87,6 +87,12 @@ void ConstantEvaluator::endVisit(Identifier const& _identifier) setType(_identifier, type(*value)); } +void ConstantEvaluator::endVisit(TupleExpression const& _tuple) +{ + if (!_tuple.isInlineArray() && _tuple.components().size() == 1) + setType(_tuple, type(*_tuple.components().front())); +} + void ConstantEvaluator::setType(ASTNode const& _node, TypePointer const& _type) { if (_type && _type->category() == Type::Category::RationalNumber) diff --git a/libsolidity/analysis/ConstantEvaluator.h b/libsolidity/analysis/ConstantEvaluator.h index 77a357b6..ac3a24a1 100644 --- a/libsolidity/analysis/ConstantEvaluator.h +++ b/libsolidity/analysis/ConstantEvaluator.h @@ -56,6 +56,7 @@ private: virtual void endVisit(UnaryOperation const& _operation); virtual void endVisit(Literal const& _literal); virtual void endVisit(Identifier const& _identifier); + virtual void endVisit(TupleExpression const& _tuple); void setType(ASTNode const& _node, TypePointer const& _type); TypePointer type(ASTNode const& _node); diff --git a/libsolidity/analysis/DeclarationContainer.cpp b/libsolidity/analysis/DeclarationContainer.cpp index c7ba78d6..786272e4 100644 --- a/libsolidity/analysis/DeclarationContainer.cpp +++ b/libsolidity/analysis/DeclarationContainer.cpp @@ -45,7 +45,8 @@ Declaration const* DeclarationContainer::conflictingDeclaration( if ( dynamic_cast<FunctionDefinition const*>(&_declaration) || - dynamic_cast<EventDefinition const*>(&_declaration) + dynamic_cast<EventDefinition const*>(&_declaration) || + dynamic_cast<MagicVariableDeclaration const*>(&_declaration) ) { // check that all other declarations with the same name are functions or a public state variable or events. @@ -68,6 +69,11 @@ Declaration const* DeclarationContainer::conflictingDeclaration( !dynamic_cast<EventDefinition const*>(declaration) ) return declaration; + if ( + dynamic_cast<MagicVariableDeclaration const*>(&_declaration) && + !dynamic_cast<MagicVariableDeclaration const*>(declaration) + ) + return declaration; // Or, continue. } } diff --git a/libsolidity/analysis/GlobalContext.cpp b/libsolidity/analysis/GlobalContext.cpp index 34cb61d8..756bb540 100644 --- a/libsolidity/analysis/GlobalContext.cpp +++ b/libsolidity/analysis/GlobalContext.cpp @@ -35,9 +35,11 @@ namespace solidity GlobalContext::GlobalContext(): m_magicVariables(vector<shared_ptr<MagicVariableDeclaration const>>{ + make_shared<MagicVariableDeclaration>("abi", make_shared<MagicType>(MagicType::Kind::ABI)), make_shared<MagicVariableDeclaration>("addmod", make_shared<FunctionType>(strings{"uint256", "uint256", "uint256"}, strings{"uint256"}, FunctionType::Kind::AddMod, false, StateMutability::Pure)), make_shared<MagicVariableDeclaration>("assert", make_shared<FunctionType>(strings{"bool"}, strings{}, FunctionType::Kind::Assert, false, StateMutability::Pure)), make_shared<MagicVariableDeclaration>("block", make_shared<MagicType>(MagicType::Kind::Block)), + 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)), @@ -50,7 +52,9 @@ m_magicVariables(vector<shared_ptr<MagicVariableDeclaration const>>{ make_shared<MagicVariableDeclaration>("mulmod", make_shared<FunctionType>(strings{"uint256", "uint256", "uint256"}, strings{"uint256"}, FunctionType::Kind::MulMod, false, StateMutability::Pure)), make_shared<MagicVariableDeclaration>("now", make_shared<IntegerType>(256)), make_shared<MagicVariableDeclaration>("require", make_shared<FunctionType>(strings{"bool"}, strings{}, FunctionType::Kind::Require, false, StateMutability::Pure)), + 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)), diff --git a/libsolidity/analysis/NameAndTypeResolver.cpp b/libsolidity/analysis/NameAndTypeResolver.cpp index 2f675135..0a356f04 100644 --- a/libsolidity/analysis/NameAndTypeResolver.cpp +++ b/libsolidity/analysis/NameAndTypeResolver.cpp @@ -47,7 +47,9 @@ NameAndTypeResolver::NameAndTypeResolver( if (!m_scopes[nullptr]) m_scopes[nullptr].reset(new DeclarationContainer()); for (Declaration const* declaration: _globals) - m_scopes[nullptr]->registerDeclaration(*declaration); + { + solAssert(m_scopes[nullptr]->registerDeclaration(*declaration), "Unable to register global declaration."); + } } bool NameAndTypeResolver::registerDeclarations(SourceUnit& _sourceUnit, ASTNode const* _currentScope) @@ -202,8 +204,9 @@ vector<Declaration const*> NameAndTypeResolver::cleanedDeclarations( solAssert( dynamic_cast<FunctionDefinition const*>(declaration) || dynamic_cast<EventDefinition const*>(declaration) || - dynamic_cast<VariableDeclaration const*>(declaration), - "Found overloading involving something not a function or a variable." + dynamic_cast<VariableDeclaration const*>(declaration) || + dynamic_cast<MagicVariableDeclaration const*>(declaration), + "Found overloading involving something not a function, event or a (magic) variable." ); FunctionTypePointer functionType { declaration->functionType(false) }; diff --git a/libsolidity/analysis/PostTypeChecker.cpp b/libsolidity/analysis/PostTypeChecker.cpp index fbc72e52..19d0b708 100644 --- a/libsolidity/analysis/PostTypeChecker.cpp +++ b/libsolidity/analysis/PostTypeChecker.cpp @@ -21,6 +21,8 @@ #include <libsolidity/interface/ErrorReporter.h> #include <libsolidity/interface/Version.h> +#include <libdevcore/Algorithms.h> + #include <boost/range/adaptor/map.hpp> #include <memory> @@ -47,7 +49,7 @@ void PostTypeChecker::endVisit(ContractDefinition const&) { solAssert(!m_currentConstVariable, ""); for (auto declaration: m_constVariables) - if (auto identifier = findCycle(declaration)) + if (auto identifier = findCycle(*declaration)) m_errorReporter.typeError( declaration->location(), "The value of the constant " + declaration->name() + @@ -87,20 +89,24 @@ bool PostTypeChecker::visit(Identifier const& _identifier) return true; } -VariableDeclaration const* PostTypeChecker::findCycle( - VariableDeclaration const* _startingFrom, - set<VariableDeclaration const*> const& _seen -) +VariableDeclaration const* PostTypeChecker::findCycle(VariableDeclaration const& _startingFrom) { - if (_seen.count(_startingFrom)) - return _startingFrom; - else if (m_constVariableDependencies.count(_startingFrom)) + auto visitor = [&](VariableDeclaration const& _variable, CycleDetector<VariableDeclaration>& _cycleDetector) { - set<VariableDeclaration const*> seen(_seen); - seen.insert(_startingFrom); - for (auto v: m_constVariableDependencies[_startingFrom]) - if (findCycle(v, seen)) - return v; - } - return nullptr; + // 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. + vector<VariableDeclaration const*> dependencies( + m_constVariableDependencies[&_variable].begin(), + m_constVariableDependencies[&_variable].end() + ); + sort(dependencies.begin(), dependencies.end(), [](VariableDeclaration const* _a, VariableDeclaration const* _b) -> bool + { + return _a->id() < _b->id(); + }); + for (auto v: dependencies) + if (_cycleDetector.run(*v)) + return; + }; + return CycleDetector<VariableDeclaration>(visitor).run(_startingFrom); } diff --git a/libsolidity/analysis/PostTypeChecker.h b/libsolidity/analysis/PostTypeChecker.h index bafc1ae6..4f9dac6e 100644 --- a/libsolidity/analysis/PostTypeChecker.h +++ b/libsolidity/analysis/PostTypeChecker.h @@ -55,10 +55,7 @@ private: virtual bool visit(Identifier const& _identifier) override; - VariableDeclaration const* findCycle( - VariableDeclaration const* _startingFrom, - std::set<VariableDeclaration const*> const& _seen = std::set<VariableDeclaration const*>{} - ); + VariableDeclaration const* findCycle(VariableDeclaration const& _startingFrom); ErrorReporter& m_errorReporter; diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index d4de219a..00a581d0 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -21,6 +21,7 @@ */ #include <libsolidity/analysis/StaticAnalyzer.h> +#include <libsolidity/analysis/ConstantEvaluator.h> #include <libsolidity/ast/AST.h> #include <libsolidity/interface/ErrorReporter.h> #include <memory> @@ -50,6 +51,16 @@ 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 @@ -68,13 +79,13 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&) for (auto const& var: m_localVarUseCount) if (var.second == 0) { - if (var.first->isCallableParameter()) + if (var.first.second->isCallableParameter()) m_errorReporter.warning( - var.first->location(), + var.first.second->location(), "Unused function parameter. Remove or comment out the variable name to silence this warning." ); else - m_errorReporter.warning(var.first->location(), "Unused local variable."); + m_errorReporter.warning(var.first.second->location(), "Unused local variable."); } m_localVarUseCount.clear(); @@ -87,7 +98,7 @@ bool StaticAnalyzer::visit(Identifier const& _identifier) { solAssert(!var->name().empty(), ""); if (var->isLocalVariable()) - m_localVarUseCount[var] += 1; + m_localVarUseCount[make_pair(var->id(), var)] += 1; } return true; } @@ -99,7 +110,7 @@ bool StaticAnalyzer::visit(VariableDeclaration const& _variable) solAssert(_variable.isLocalVariable(), ""); if (_variable.name() != "") // This is not a no-op, the entry might pre-exist. - m_localVarUseCount[&_variable] += 0; + m_localVarUseCount[make_pair(_variable.id(), &_variable)] += 0; } else if (_variable.isStateVariable()) { @@ -122,7 +133,7 @@ bool StaticAnalyzer::visit(Return const& _return) if (m_currentFunction && _return.expression()) for (auto const& var: m_currentFunction->returnParameters()) if (!var->name().empty()) - m_localVarUseCount[var.get()] += 1; + m_localVarUseCount[make_pair(var->id(), var.get())] += 1; return true; } @@ -142,6 +153,7 @@ 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) @@ -155,6 +167,20 @@ bool StaticAnalyzer::visit(MemberAccess const& _memberAccess) "\"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()\"" + ); + } + } if (m_nonPayablePublic && !m_library) if (MagicType const* type = dynamic_cast<MagicType const*>(_memberAccess.expression().annotation().type.get())) @@ -180,10 +206,32 @@ bool StaticAnalyzer::visit(MemberAccess const& _memberAccess) ); } - if (m_constructor && m_currentContract) - if (ContractType const* type = dynamic_cast<ContractType const*>(_memberAccess.expression().annotation().type.get())) - if (type->contractDefinition() == *m_currentContract) - m_errorReporter.warning(_memberAccess.location(), "\"this\" used in constructor."); + if (m_constructor) + { + auto const* expr = &_memberAccess.expression(); + while(expr) + { + if (auto id = dynamic_cast<Identifier const*>(expr)) + { + if (id->name() == "this") + m_errorReporter.warning( + id->location(), + "\"this\" used in constructor. " + "Note that external functions of a contract " + "cannot be called while it is being constructed."); + break; + } + else if (auto tuple = dynamic_cast<TupleExpression const*>(expr)) + { + if (tuple->components().size() == 1) + expr = tuple->components().front().get(); + else + break; + } + else + break; + } + } return true; } @@ -199,13 +247,54 @@ bool StaticAnalyzer::visit(InlineAssembly const& _inlineAssembly) { solAssert(!var->name().empty(), ""); if (var->isLocalVariable()) - m_localVarUseCount[var] += 1; + m_localVarUseCount[make_pair(var->id(), var)] += 1; } } return true; } +bool StaticAnalyzer::visit(BinaryOperation const& _operation) +{ + if ( + _operation.rightExpression().annotation().isPure && + (_operation.getOperator() == Token::Div || _operation.getOperator() == Token::Mod) + ) + if (auto rhs = dynamic_pointer_cast<RationalNumberType const>( + ConstantEvaluator(m_errorReporter).evaluate(_operation.rightExpression()) + )) + if (rhs->isZero()) + m_errorReporter.typeError( + _operation.location(), + (_operation.getOperator() == Token::Div) ? "Division by zero." : "Modulo zero." + ); + + return true; +} + +bool StaticAnalyzer::visit(FunctionCall const& _functionCall) +{ + if (_functionCall.annotation().kind == FunctionCallKind::FunctionCall) + { + auto functionType = dynamic_pointer_cast<FunctionType const>(_functionCall.expression().annotation().type); + solAssert(functionType, ""); + if (functionType->kind() == FunctionType::Kind::AddMod || functionType->kind() == FunctionType::Kind::MulMod) + { + solAssert(_functionCall.arguments().size() == 3, ""); + if (_functionCall.arguments()[2]->annotation().isPure) + if (auto lastArg = dynamic_pointer_cast<RationalNumberType const>( + ConstantEvaluator(m_errorReporter).evaluate(*(_functionCall.arguments())[2]) + )) + if (lastArg->isZero()) + m_errorReporter.typeError( + _functionCall.location(), + "Arithmetic modulo zero." + ); + } + } + return true; +} + bigint StaticAnalyzer::structureSizeEstimate(Type const& _type, set<StructDefinition const*>& _structsSeen) { switch (_type.category()) diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index 124c4e7c..2a62e391 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -64,6 +64,8 @@ private: virtual bool visit(Return const& _return) override; virtual bool visit(MemberAccess const& _memberAccess) override; virtual bool visit(InlineAssembly const& _inlineAssembly) override; + virtual bool visit(BinaryOperation const& _operation) override; + virtual bool visit(FunctionCall const& _functionCall) override; /// @returns the size of this type in storage, including all sub-types. static bigint structureSizeEstimate(Type const& _type, std::set<StructDefinition const*>& _structsSeen); @@ -77,7 +79,9 @@ private: bool m_nonPayablePublic = false; /// Number of uses of each (named) local variable in a function, counter is initialized with zero. - std::map<VariableDeclaration const*, int> m_localVarUseCount; + /// Pairs of AST ids and pointers are used as keys to ensure a deterministic order + /// when traversing. + std::map<std::pair<size_t, VariableDeclaration const*>, int> m_localVarUseCount; FunctionDefinition const* m_currentFunction = nullptr; diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index ddac194b..f648e5b4 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -214,18 +214,31 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function) { bool const v050 = m_sourceUnit->annotation().experimentalFeatures.count(ExperimentalFeature::V050); - if (_function.noVisibilitySpecified()) + if (v050 && _function.noVisibilitySpecified()) + m_errorReporter.syntaxError(_function.location(), "No visibility specified."); + + if (_function.isOldStyleConstructor()) { if (v050) - m_errorReporter.syntaxError(_function.location(), "No visibility specified."); + 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(), - "No visibility specified. Defaulting to \"" + - Declaration::visibilityToString(_function.visibility()) + - "\"." + "Defining constructors as functions with the same name as the contract is deprecated. " + "Use \"constructor(...) { ... }\" instead." ); } + if (!_function.isImplemented() && !_function.modifiers().empty()) + { + 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." ); + } return true; } @@ -255,3 +268,17 @@ bool SyntaxChecker::visit(VariableDeclaration const& _declaration) } 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."); + } + return true; +} diff --git a/libsolidity/analysis/SyntaxChecker.h b/libsolidity/analysis/SyntaxChecker.h index 871bf0a9..1579df57 100644 --- a/libsolidity/analysis/SyntaxChecker.h +++ b/libsolidity/analysis/SyntaxChecker.h @@ -71,6 +71,8 @@ private: virtual bool visit(VariableDeclaration const& _declaration) override; + virtual bool visit(StructDefinition const& _struct) override; + ErrorReporter& m_errorReporter; /// Flag that indicates whether a function modifier actually contains '_'. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index bebdb9b6..47a551dc 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -60,17 +60,7 @@ bool typeSupportedByOldABIEncoder(Type const& _type) bool TypeChecker::checkTypeRequirements(ASTNode const& _contract) { - try - { - _contract.accept(*this); - } - catch (FatalError const&) - { - // We got a fatal error which required to stop further type checking, but we can - // continue normally from here. - if (m_errorReporter.errors().empty()) - throw; // Something is weird here, rather throw again. - } + _contract.accept(*this); return Error::containsOnlyWarnings(m_errorReporter.errors()); } @@ -101,7 +91,7 @@ bool TypeChecker::visit(ContractDefinition const& _contract) checkContractDuplicateEvents(_contract); checkContractIllegalOverrides(_contract); checkContractAbstractFunctions(_contract); - checkContractAbstractConstructors(_contract); + checkContractBaseConstructorArguments(_contract); FunctionDefinition const* function = _contract.constructor(); if (function) @@ -291,42 +281,108 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont } } -void TypeChecker::checkContractAbstractConstructors(ContractDefinition const& _contract) +void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) { - set<ContractDefinition const*> argumentsNeeded; - // check that we get arguments for all base constructors that need it. - // If not mark the contract as abstract (not fully implemented) + bool const v050 = _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts; - for (ContractDefinition const* contract: bases) - if (FunctionDefinition const* constructor = contract->constructor()) - if (contract != &_contract && !constructor->parameters().empty()) - argumentsNeeded.insert(contract); + // Determine the arguments that are used for the base constructors. for (ContractDefinition const* contract: bases) { if (FunctionDefinition const* constructor = contract->constructor()) for (auto const& modifier: constructor->modifiers()) { - auto baseContract = dynamic_cast<ContractDefinition const*>( - &dereference(*modifier->name()) - ); - if (baseContract) - argumentsNeeded.erase(baseContract); + auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(*modifier->name())); + if (modifier->arguments()) + { + 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." + ); + else + m_errorReporter.warning( + modifier->location(), + "Modifier-style base constructor call without arguments." + ); + } } - for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts()) { auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(base->name())); solAssert(baseContract, ""); - if (!base->arguments().empty()) - argumentsNeeded.erase(baseContract); + + if (baseContract->constructor() && base->arguments() && !base->arguments()->empty()) + annotateBaseConstructorArguments(_contract, baseContract->constructor(), base.get()); + } + } + + // check that we get arguments for all base constructors that need it. + // If not mark the contract as abstract (not fully implemented) + for (ContractDefinition const* contract: bases) + if (FunctionDefinition const* constructor = contract->constructor()) + if (contract != &_contract && !constructor->parameters().empty()) + if (!_contract.annotation().baseConstructorArguments.count(constructor)) + _contract.annotation().unimplementedFunctions.push_back(constructor); +} + +void TypeChecker::annotateBaseConstructorArguments( + ContractDefinition const& _currentContract, + FunctionDefinition const* _baseConstructor, + ASTNode const* _argumentNode +) +{ + bool const v050 = _currentContract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); + + solAssert(_baseConstructor, ""); + solAssert(_argumentNode, ""); + + auto insertionResult = _currentContract.annotation().baseConstructorArguments.insert( + std::make_pair(_baseConstructor, _argumentNode) + ); + if (!insertionResult.second) + { + ASTNode const* previousNode = insertionResult.first->second; + + SourceLocation const* mainLocation = nullptr; + SecondarySourceLocation ssl; + + if ( + _currentContract.location().contains(previousNode->location()) || + _currentContract.location().contains(_argumentNode->location()) + ) + { + mainLocation = &previousNode->location(); + ssl.append("Second constructor call is here:", _argumentNode->location()); } + else + { + mainLocation = &_currentContract.location(); + ssl.append("First constructor call is here: ", _argumentNode->location()); + 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 + ); } - if (!argumentsNeeded.empty()) - for (ContractDefinition const* contract: argumentsNeeded) - _contract.annotation().unimplementedFunctions.push_back(contract->constructor()); + } void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contract) @@ -378,7 +434,16 @@ void TypeChecker::checkFunctionOverride(FunctionDefinition const& function, Func function.annotation().superFunction = &super; if (function.visibility() != super.visibility()) + { + // visibility is enforced to be external in interfaces, but a contract can override that with public + if ( + super.inContractKind() == ContractDefinition::ContractKind::Interface && + function.inContractKind() != ContractDefinition::ContractKind::Interface && + function.visibility() == FunctionDefinition::Visibility::Public + ) + return; overrideError(function, super, "Overriding function visibility differs."); + } else if (function.stateMutability() != super.stateMutability()) overrideError( @@ -497,30 +562,46 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance) // Interfaces do not have constructors, so there are zero parameters. parameterTypes = ContractType(*base).newExpressionType()->parameterTypes(); - if (!arguments.empty() && parameterTypes.size() != arguments.size()) + if (arguments) { - m_errorReporter.typeError( - _inheritance.location(), - "Wrong argument count for constructor call: " + - toString(arguments.size()) + - " arguments given but expected " + - toString(parameterTypes.size()) + - "." - ); - return; - } + bool v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - for (size_t i = 0; i < arguments.size(); ++i) - if (!type(*arguments[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) - m_errorReporter.typeError( - arguments[i]->location(), - "Invalid type for argument in constructor call. " - "Invalid implicit conversion from " + - type(*arguments[i])->toString() + - " to " + - parameterTypes[i]->toString() + - " requested." - ); + 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; + } + } + for (size_t i = 0; i < arguments->size(); ++i) + if (!type(*(*arguments)[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) + m_errorReporter.typeError( + (*arguments)[i]->location(), + "Invalid type for argument in constructor call. " + "Invalid implicit conversion from " + + type(*(*arguments)[i])->toString() + + " to " + + parameterTypes[i]->toString() + + " requested." + ); + } } void TypeChecker::endVisit(UsingForDirective const& _usingFor) @@ -731,7 +812,8 @@ void TypeChecker::visitManually( vector<ContractDefinition const*> const& _bases ) { - std::vector<ASTPointer<Expression>> const& arguments = _modifier.arguments(); + std::vector<ASTPointer<Expression>> const& arguments = + _modifier.arguments() ? *_modifier.arguments() : std::vector<ASTPointer<Expression>>(); for (ASTPointer<Expression> const& argument: arguments) argument->accept(*this); _modifier.name()->accept(*this); @@ -769,7 +851,7 @@ void TypeChecker::visitManually( ); return; } - for (size_t i = 0; i < _modifier.arguments().size(); ++i) + for (size_t i = 0; i < arguments.size(); ++i) if (!type(*arguments[i])->isImplicitlyConvertibleTo(*type(*(*parameters)[i]))) m_errorReporter.typeError( arguments[i]->location(), @@ -1551,16 +1633,22 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) _functionCall.expression().annotation().isPure && functionType->isPure(); + bool allowDynamicTypes = m_evmVersion.supportsReturndata(); if (!functionType) { m_errorReporter.typeError(_functionCall.location(), "Type is not callable"); _functionCall.annotation().type = make_shared<TupleType>(); return false; } - else if (functionType->returnParameterTypes().size() == 1) - _functionCall.annotation().type = functionType->returnParameterTypes().front(); + + auto returnTypes = + allowDynamicTypes ? + functionType->returnParameterTypes() : + functionType->returnParameterTypesWithoutDynamicTypes(); + if (returnTypes.size() == 1) + _functionCall.annotation().type = returnTypes.front(); else - _functionCall.annotation().type = make_shared<TupleType>(functionType->returnParameterTypes()); + _functionCall.annotation().type = make_shared<TupleType>(returnTypes); if (auto functionName = dynamic_cast<Identifier const*>(&_functionCall.expression())) { @@ -1600,7 +1688,19 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) } } - if (!functionType->takesArbitraryParameters() && parameterTypes.size() != arguments.size()) + if (functionType->takesArbitraryParameters() && arguments.size() < parameterTypes.size()) + { + 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()) + + "." + ); + } + else if (!functionType->takesArbitraryParameters() && parameterTypes.size() != arguments.size()) { bool isStructConstructorCall = _functionCall.annotation().kind == FunctionCallKind::StructConstructorCall; @@ -1623,15 +1723,36 @@ bool TypeChecker::visit(FunctionCall const& _functionCall) } else if (isPositionalCall) { - // call by positional arguments + bool const abiEncodeV2 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2); + for (size_t i = 0; i < arguments.size(); ++i) { auto const& argType = type(*arguments[i]); - if (functionType->takesArbitraryParameters()) + 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( @@ -1867,7 +1988,8 @@ bool TypeChecker::visit(MemberAccess const& _memberAccess) 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." + " Convert the contract to \"address\" type to access the member," + " for example use \"address(contract)." + memberName + "\" instead." ); } @@ -2025,10 +2147,9 @@ bool TypeChecker::visit(Identifier const& _identifier) for (Declaration const* declaration: annotation.overloadedDeclarations) { - TypePointer function = declaration->type(); - solAssert(!!function, "Requested type not present."); - auto const* functionType = dynamic_cast<FunctionType const*>(function.get()); - if (functionType && functionType->canTakeArguments(*annotation.argumentTypes)) + FunctionTypePointer functionType = declaration->functionType(true); + solAssert(!!functionType, "Requested type not present."); + if (functionType->canTakeArguments(*annotation.argumentTypes)) candidates.push_back(declaration); } if (candidates.empty()) diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 2ba31232..2245abd6 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -73,7 +73,12 @@ private: void checkFunctionOverride(FunctionDefinition const& function, FunctionDefinition const& super); void overrideError(FunctionDefinition const& function, FunctionDefinition const& super, std::string message); void checkContractAbstractFunctions(ContractDefinition const& _contract); - void checkContractAbstractConstructors(ContractDefinition const& _contract); + void checkContractBaseConstructorArguments(ContractDefinition const& _contract); + void annotateBaseConstructorArguments( + ContractDefinition const& _currentContract, + FunctionDefinition const* _baseConstructor, + ASTNode const* _argumentNode + ); /// Checks that different functions with external visibility end up having different /// external argument types (i.e. different signature). void checkContractExternalTypeClashes(ContractDefinition const& _contract); diff --git a/libsolidity/analysis/ViewPureChecker.cpp b/libsolidity/analysis/ViewPureChecker.cpp index 13c3ab68..d9843012 100644 --- a/libsolidity/analysis/ViewPureChecker.cpp +++ b/libsolidity/analysis/ViewPureChecker.cpp @@ -305,10 +305,15 @@ void ViewPureChecker::endVisit(MemberAccess const& _memberAccess) mutability = StateMutability::View; break; case Type::Category::Magic: + { // we can ignore the kind of magic and only look at the name of the member - if (member != "data" && member != "sig" && member != "blockhash") + set<string> static const pureMembers{ + "encode", "encodePacked", "encodeWithSelector", "encodeWithSignature", "data", "sig", "blockhash" + }; + if (!pureMembers.count(member)) mutability = StateMutability::View; break; + } case Type::Category::Struct: { if (_memberAccess.expression().annotation().type->dataStoredIn(DataLocation::Storage)) |