diff options
| -rw-r--r-- | .circleci/config.yml | 3 | ||||
| -rw-r--r-- | Changelog.md | 2 | ||||
| -rw-r--r-- | docs/contracts.rst | 7 | ||||
| -rw-r--r-- | libsolidity/CMakeLists.txt | 1 | ||||
| -rw-r--r-- | libsolidity/analysis/ContractLevelChecker.cpp | 452 | ||||
| -rw-r--r-- | libsolidity/analysis/ContractLevelChecker.h | 87 | ||||
| -rw-r--r-- | libsolidity/analysis/TypeChecker.cpp | 406 | ||||
| -rw-r--r-- | libsolidity/analysis/TypeChecker.h | 24 | ||||
| -rw-r--r-- | libsolidity/interface/CompilerStack.cpp | 18 | ||||
| -rw-r--r-- | test/libsolidity/syntaxTests/constructor/abstract_creation_forward_reference.sol | 15 | ||||
| -rw-r--r-- | test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol | 11 |
11 files changed, 591 insertions, 435 deletions
diff --git a/.circleci/config.yml b/.circleci/config.yml index 975d1c7a..6c5f3e85 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -14,6 +14,7 @@ defaults: command: | mkdir -p build cd build + [ -n "$COVERAGE" -a "$CIRCLE_BRANCH" != release -a -z "$CIRCLE_TAG" ] && CMAKE_OPTIONS="$CMAKE_OPTIONS -DCOVERAGE=ON" cmake .. -DCMAKE_BUILD_TYPE=RelWithDebInfo $CMAKE_OPTIONS make -j4 - run_tests: &run_tests @@ -122,7 +123,7 @@ jobs: - image: buildpack-deps:artful environment: TERM: xterm - CMAKE_OPTIONS: -DCOVERAGE=ON + COVERAGE: "ON" steps: - checkout - run: diff --git a/Changelog.md b/Changelog.md index 1c3fcde4..630801e4 100644 --- a/Changelog.md +++ b/Changelog.md @@ -24,6 +24,8 @@ Bugfixes: * Type Checker: Disallow struct return types for getters of public state variables unless the new ABI encoder is active. * Type Checker: Fix internal compiler error when a field of a struct used as a parameter in a function type has a non-existent type. * Type Checker: Disallow functions ``sha3`` and ``suicide`` also without a function call. + * Type Checker: Fixed internal error when trying to create abstract contract in some cases. + * Type Checker: Fixed internal error related to double declaration of events. * Type Checker: Disallow inline arrays of mapping type. Build System: diff --git a/docs/contracts.rst b/docs/contracts.rst index df95883f..d6337d9a 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -1043,14 +1043,15 @@ Additional Resources for Understanding Events Inheritance *********** -Solidity supports multiple inheritance by copying code including polymorphism. +Solidity supports multiple inheritance including polymorphism. All function calls are virtual, which means that the most derived function -is called, except when the contract name is explicitly given. +is called, except when the contract name is explicitly given or the +``super`` keyword is used. When a contract inherits from other contracts, only a single contract is created on the blockchain, and the code from all the base contracts -is copied into the created contract. +is compiled into the created contract. The general inheritance system is very similar to `Python's <https://docs.python.org/3/tutorial/classes.html#inheritance>`_, diff --git a/libsolidity/CMakeLists.txt b/libsolidity/CMakeLists.txt index d2e0c854..dc4c6d15 100644 --- a/libsolidity/CMakeLists.txt +++ b/libsolidity/CMakeLists.txt @@ -1,6 +1,7 @@ # Until we have a clear separation, libyul has to be included here set(sources analysis/ConstantEvaluator.cpp + analysis/ContractLevelChecker.cpp analysis/ControlFlowAnalyzer.cpp analysis/ControlFlowBuilder.cpp analysis/ControlFlowGraph.cpp diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp new file mode 100644 index 00000000..58dcfe4d --- /dev/null +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -0,0 +1,452 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see <http://www.gnu.org/licenses/>. +*/ +/** + * Component that verifies overloads, abstract contracts, function clashes and others + * checks at contract or function level. + */ + +#include <libsolidity/analysis/ContractLevelChecker.h> +#include <libsolidity/ast/AST.h> + +#include <liblangutil/ErrorReporter.h> + +#include <boost/range/adaptor/reversed.hpp> + + +using namespace std; +using namespace dev; +using namespace langutil; +using namespace dev::solidity; + + +bool ContractLevelChecker::check(ContractDefinition const& _contract) +{ + checkDuplicateFunctions(_contract); + checkDuplicateEvents(_contract); + checkIllegalOverrides(_contract); + checkAbstractFunctions(_contract); + checkBaseConstructorArguments(_contract); + checkConstructor(_contract); + checkFallbackFunction(_contract); + checkExternalTypeClashes(_contract); + checkHashCollisions(_contract); + checkLibraryRequirements(_contract); + + return Error::containsOnlyWarnings(m_errorReporter.errors()); +} + +void ContractLevelChecker::checkDuplicateFunctions(ContractDefinition const& _contract) +{ + /// Checks that two functions with the same name defined in this contract have different + /// argument types and that there is at most one constructor. + map<string, vector<FunctionDefinition const*>> functions; + FunctionDefinition const* constructor = nullptr; + FunctionDefinition const* fallback = nullptr; + for (FunctionDefinition const* function: _contract.definedFunctions()) + if (function->isConstructor()) + { + if (constructor) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", constructor->location()), + "More than one constructor defined." + ); + constructor = function; + } + else if (function->isFallback()) + { + if (fallback) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", fallback->location()), + "Only one fallback function is allowed." + ); + fallback = function; + } + else + { + solAssert(!function->name().empty(), ""); + functions[function->name()].push_back(function); + } + + findDuplicateDefinitions(functions, "Function with same name and arguments defined twice."); +} + +void ContractLevelChecker::checkDuplicateEvents(ContractDefinition const& _contract) +{ + /// Checks that two events with the same name defined in this contract have different + /// argument types + map<string, vector<EventDefinition const*>> events; + for (EventDefinition const* event: _contract.events()) + events[event->name()].push_back(event); + + findDuplicateDefinitions(events, "Event with same name and arguments defined twice."); +} + +template <class T> +void ContractLevelChecker::findDuplicateDefinitions(map<string, vector<T>> const& _definitions, string _message) +{ + for (auto const& it: _definitions) + { + vector<T> const& overloads = it.second; + set<size_t> reported; + for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i) + { + SecondarySourceLocation ssl; + + for (size_t j = i + 1; j < overloads.size(); ++j) + if (FunctionType(*overloads[i]).asCallableFunction(false)->hasEqualParameterTypes( + *FunctionType(*overloads[j]).asCallableFunction(false)) + ) + { + ssl.append("Other declaration is here:", overloads[j]->location()); + reported.insert(j); + } + + if (ssl.infos.size() > 0) + { + ssl.limitSize(_message); + + m_errorReporter.declarationError( + overloads[i]->location(), + ssl, + _message + ); + } + } + } +} + +void ContractLevelChecker::checkIllegalOverrides(ContractDefinition const& _contract) +{ + // TODO unify this at a later point. for this we need to put the constness and the access specifier + // into the types + map<string, vector<FunctionDefinition const*>> functions; + map<string, ModifierDefinition const*> modifiers; + + // We search from derived to base, so the stored item causes the error. + for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts) + { + for (FunctionDefinition const* function: contract->definedFunctions()) + { + if (function->isConstructor()) + continue; // constructors can neither be overridden nor override anything + string const& name = function->name(); + if (modifiers.count(name)) + m_errorReporter.typeError(modifiers[name]->location(), "Override changes function to modifier."); + + for (FunctionDefinition const* overriding: functions[name]) + checkFunctionOverride(*overriding, *function); + + functions[name].push_back(function); + } + for (ModifierDefinition const* modifier: contract->functionModifiers()) + { + string const& name = modifier->name(); + ModifierDefinition const*& override = modifiers[name]; + if (!override) + override = modifier; + else if (ModifierType(*override) != ModifierType(*modifier)) + m_errorReporter.typeError(override->location(), "Override changes modifier signature."); + if (!functions[name].empty()) + m_errorReporter.typeError(override->location(), "Override changes modifier to function."); + } + } +} + +void ContractLevelChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super) +{ + FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false); + FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false); + + if (!functionType->hasEqualParameterTypes(*superType)) + return; + if (!functionType->hasEqualReturnTypes(*superType)) + overrideError(_function, _super, "Overriding function return types differ."); + + if (!_function.annotation().superFunction) + _function.annotation().superFunction = &_super; + + if (_function.visibility() != _super.visibility()) + { + // Visibility change from external to public is fine. + // Any other change is disallowed. + if (!( + _super.visibility() == FunctionDefinition::Visibility::External && + _function.visibility() == FunctionDefinition::Visibility::Public + )) + overrideError(_function, _super, "Overriding function visibility differs."); + } + if (_function.stateMutability() != _super.stateMutability()) + overrideError( + _function, + _super, + "Overriding function changes state mutability from \"" + + stateMutabilityToString(_super.stateMutability()) + + "\" to \"" + + stateMutabilityToString(_function.stateMutability()) + + "\"." + ); +} + +void ContractLevelChecker::overrideError(FunctionDefinition const& function, FunctionDefinition const& super, string message) +{ + m_errorReporter.typeError( + function.location(), + SecondarySourceLocation().append("Overridden function is here:", super.location()), + message + ); +} + +void ContractLevelChecker::checkAbstractFunctions(ContractDefinition const& _contract) +{ + // Mapping from name to function definition (exactly one per argument type equality class) and + // flag to indicate whether it is fully implemented. + using FunTypeAndFlag = std::pair<FunctionTypePointer, bool>; + map<string, vector<FunTypeAndFlag>> functions; + + // Search from base to derived + for (ContractDefinition const* contract: boost::adaptors::reverse(_contract.annotation().linearizedBaseContracts)) + for (FunctionDefinition const* function: contract->definedFunctions()) + { + // Take constructors out of overload hierarchy + if (function->isConstructor()) + continue; + auto& overloads = functions[function->name()]; + FunctionTypePointer funType = make_shared<FunctionType>(*function)->asCallableFunction(false); + auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag) + { + return funType->hasEqualParameterTypes(*_funAndFlag.first); + }); + if (it == overloads.end()) + overloads.push_back(make_pair(funType, function->isImplemented())); + else if (it->second) + { + if (!function->isImplemented()) + m_errorReporter.typeError(function->location(), "Redeclaring an already implemented function as abstract"); + } + else if (function->isImplemented()) + it->second = true; + } + + // Set to not fully implemented if at least one flag is false. + for (auto const& it: functions) + for (auto const& funAndFlag: it.second) + if (!funAndFlag.second) + { + FunctionDefinition const* function = dynamic_cast<FunctionDefinition const*>(&funAndFlag.first->declaration()); + solAssert(function, ""); + _contract.annotation().unimplementedFunctions.push_back(function); + break; + } +} + + +void ContractLevelChecker::checkBaseConstructorArguments(ContractDefinition const& _contract) +{ + vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts; + + // 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()) + if (auto baseContract = dynamic_cast<ContractDefinition const*>( + modifier->name()->annotation().referencedDeclaration + )) + { + if (modifier->arguments()) + { + if (baseContract->constructor()) + annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); + } + else + m_errorReporter.declarationError( + modifier->location(), + "Modifier-style base constructor call without arguments." + ); + } + + for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts()) + { + ContractDefinition const* baseContract = dynamic_cast<ContractDefinition const*>( + base->name().annotation().referencedDeclaration + ); + solAssert(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 ContractLevelChecker::annotateBaseConstructorArguments( + ContractDefinition const& _currentContract, + FunctionDefinition const* _baseConstructor, + ASTNode const* _argumentNode +) +{ + 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()); + } + + m_errorReporter.declarationError( + *mainLocation, + ssl, + "Base constructor arguments given twice." + ); + } + +} + +void ContractLevelChecker::checkConstructor(ContractDefinition const& _contract) +{ + FunctionDefinition const* constructor = _contract.constructor(); + if (!constructor) + return; + + if (!constructor->returnParameters().empty()) + m_errorReporter.typeError(constructor->returnParameterList()->location(), "Non-empty \"returns\" directive for constructor."); + if (constructor->stateMutability() != StateMutability::NonPayable && constructor->stateMutability() != StateMutability::Payable) + m_errorReporter.typeError( + constructor->location(), + "Constructor must be payable or non-payable, but is \"" + + stateMutabilityToString(constructor->stateMutability()) + + "\"." + ); + if (constructor->visibility() != FunctionDefinition::Visibility::Public && constructor->visibility() != FunctionDefinition::Visibility::Internal) + m_errorReporter.typeError(constructor->location(), "Constructor must be public or internal."); +} + +void ContractLevelChecker::checkFallbackFunction(ContractDefinition const& _contract) +{ + FunctionDefinition const* fallback = _contract.fallbackFunction(); + if (!fallback) + return; + + if (_contract.isLibrary()) + m_errorReporter.typeError(fallback->location(), "Libraries cannot have fallback functions."); + if (fallback->stateMutability() != StateMutability::NonPayable && fallback->stateMutability() != StateMutability::Payable) + m_errorReporter.typeError( + fallback->location(), + "Fallback function must be payable or non-payable, but is \"" + + stateMutabilityToString(fallback->stateMutability()) + + "\"." + ); + if (!fallback->parameters().empty()) + m_errorReporter.typeError(fallback->parameterList().location(), "Fallback function cannot take parameters."); + if (!fallback->returnParameters().empty()) + m_errorReporter.typeError(fallback->returnParameterList()->location(), "Fallback function cannot return values."); + if (fallback->visibility() != FunctionDefinition::Visibility::External) + m_errorReporter.typeError(fallback->location(), "Fallback function must be defined as \"external\"."); +} + +void ContractLevelChecker::checkExternalTypeClashes(ContractDefinition const& _contract) +{ + map<string, vector<pair<Declaration const*, FunctionTypePointer>>> externalDeclarations; + for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts) + { + for (FunctionDefinition const* f: contract->definedFunctions()) + if (f->isPartOfExternalInterface()) + { + auto functionType = make_shared<FunctionType>(*f); + // under non error circumstances this should be true + if (functionType->interfaceFunctionType()) + externalDeclarations[functionType->externalSignature()].push_back( + make_pair(f, functionType->asCallableFunction(false)) + ); + } + for (VariableDeclaration const* v: contract->stateVariables()) + if (v->isPartOfExternalInterface()) + { + auto functionType = make_shared<FunctionType>(*v); + // under non error circumstances this should be true + if (functionType->interfaceFunctionType()) + externalDeclarations[functionType->externalSignature()].push_back( + make_pair(v, functionType->asCallableFunction(false)) + ); + } + } + 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->hasEqualParameterTypes(*it.second[j].second)) + m_errorReporter.typeError( + it.second[j].first->location(), + "Function overload clash during conversion to external types for arguments." + ); +} + +void ContractLevelChecker::checkHashCollisions(ContractDefinition const& _contract) +{ + set<FixedHash<4>> hashes; + for (auto const& it: _contract.interfaceFunctionList()) + { + FixedHash<4> const& hash = it.first; + if (hashes.count(hash)) + m_errorReporter.typeError( + _contract.location(), + string("Function signature hash collision for ") + it.second->externalSignature() + ); + hashes.insert(hash); + } +} + +void ContractLevelChecker::checkLibraryRequirements(ContractDefinition const& _contract) +{ + if (!_contract.isLibrary()) + return; + + if (!_contract.baseContracts().empty()) + m_errorReporter.typeError(_contract.location(), "Library is not allowed to inherit."); + + for (auto const& var: _contract.stateVariables()) + if (!var->isConstant()) + m_errorReporter.typeError(var->location(), "Library cannot have non-constant state variables"); +} diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h new file mode 100644 index 00000000..15cbf45d --- /dev/null +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -0,0 +1,87 @@ +/* + This file is part of solidity. + + solidity is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + solidity is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with solidity. If not, see <http://www.gnu.org/licenses/>. +*/ +/** + * Component that verifies overloads, abstract contracts, function clashes and others + * checks at contract or function level. + */ + +#pragma once + +#include <libsolidity/ast/ASTForward.h> + +#include <map> + +namespace langutil +{ +class ErrorReporter; +} + +namespace dev +{ +namespace solidity +{ + +/** + * Component that verifies overloads, abstract contracts, function clashes and others + * checks at contract or function level. + */ +class ContractLevelChecker +{ +public: + /// @param _errorReporter provides the error logging functionality. + explicit ContractLevelChecker(langutil::ErrorReporter& _errorReporter): + m_errorReporter(_errorReporter) + {} + + /// Performs checks on the given contract. + /// @returns true iff all checks passed. Note even if all checks passed, errors() can still contain warnings + bool check(ContractDefinition const& _contract); + +private: + /// Checks that two functions defined in this contract with the same name have different + /// arguments and that there is at most one constructor. + void checkDuplicateFunctions(ContractDefinition const& _contract); + void checkDuplicateEvents(ContractDefinition const& _contract); + template <class T> + void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions, std::string _message); + void checkIllegalOverrides(ContractDefinition const& _contract); + /// 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); + void checkAbstractFunctions(ContractDefinition const& _contract); + void checkBaseConstructorArguments(ContractDefinition const& _contract); + void annotateBaseConstructorArguments( + ContractDefinition const& _currentContract, + FunctionDefinition const* _baseConstructor, + ASTNode const* _argumentNode + ); + void checkConstructor(ContractDefinition const& _contract); + void checkFallbackFunction(ContractDefinition const& _contract); + /// Checks that different functions with external visibility end up having different + /// external argument types (i.e. different signature). + void checkExternalTypeClashes(ContractDefinition const& _contract); + /// Checks for hash collisions in external function signatures. + void checkHashCollisions(ContractDefinition const& _contract); + /// Checks that all requirements for a library are fulfilled if this is a library. + void checkLibraryRequirements(ContractDefinition const& _contract); + + langutil::ErrorReporter& m_errorReporter; +}; + +} +} diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index a80ca7d6..9350df05 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -35,7 +35,6 @@ #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 <memory> #include <vector> @@ -89,417 +88,14 @@ bool TypeChecker::visit(ContractDefinition const& _contract) { m_scope = &_contract; - // We force our own visiting order here. The structs have to be excluded below. - set<ASTNode const*> visited; - for (auto const& s: _contract.definedStructs()) - visited.insert(s); - ASTNode::listAccept(_contract.definedStructs(), *this); ASTNode::listAccept(_contract.baseContracts(), *this); - checkContractDuplicateFunctions(_contract); - checkContractDuplicateEvents(_contract); - checkContractIllegalOverrides(_contract); - checkContractAbstractFunctions(_contract); - checkContractBaseConstructorArguments(_contract); - - FunctionDefinition const* function = _contract.constructor(); - if (function) - { - if (!function->returnParameters().empty()) - m_errorReporter.typeError(function->returnParameterList()->location(), "Non-empty \"returns\" directive for constructor."); - if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) - m_errorReporter.typeError( - function->location(), - "Constructor must be payable or non-payable, but is \"" + - stateMutabilityToString(function->stateMutability()) + - "\"." - ); - if (function->visibility() != FunctionDefinition::Visibility::Public && function->visibility() != FunctionDefinition::Visibility::Internal) - m_errorReporter.typeError(function->location(), "Constructor must be public or internal."); - } - - for (FunctionDefinition const* function: _contract.definedFunctions()) - if (function->isFallback()) - { - if (_contract.isLibrary()) - m_errorReporter.typeError(function->location(), "Libraries cannot have fallback functions."); - if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) - m_errorReporter.typeError( - function->location(), - "Fallback function must be payable or non-payable, but is \"" + - stateMutabilityToString(function->stateMutability()) + - "\"." - ); - if (!function->parameters().empty()) - 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 (function->visibility() != FunctionDefinition::Visibility::External) - m_errorReporter.typeError(function->location(), "Fallback function must be defined as \"external\"."); - } - for (auto const& n: _contract.subNodes()) - if (!visited.count(n.get())) - n->accept(*this); - - checkContractExternalTypeClashes(_contract); - // check for hash collisions in function signatures - set<FixedHash<4>> hashes; - for (auto const& it: _contract.interfaceFunctionList()) - { - FixedHash<4> const& hash = it.first; - if (hashes.count(hash)) - m_errorReporter.typeError( - _contract.location(), - string("Function signature hash collision for ") + it.second->externalSignature() - ); - hashes.insert(hash); - } - - if (_contract.isLibrary()) - checkLibraryRequirements(_contract); + n->accept(*this); return false; } -void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _contract) -{ - /// Checks that two functions with the same name defined in this contract have different - /// argument types and that there is at most one constructor. - map<string, vector<FunctionDefinition const*>> functions; - FunctionDefinition const* constructor = nullptr; - FunctionDefinition const* fallback = nullptr; - for (FunctionDefinition const* function: _contract.definedFunctions()) - if (function->isConstructor()) - { - if (constructor) - m_errorReporter.declarationError( - function->location(), - SecondarySourceLocation().append("Another declaration is here:", constructor->location()), - "More than one constructor defined." - ); - constructor = function; - } - else if (function->isFallback()) - { - if (fallback) - m_errorReporter.declarationError( - function->location(), - SecondarySourceLocation().append("Another declaration is here:", fallback->location()), - "Only one fallback function is allowed." - ); - fallback = function; - } - else - { - solAssert(!function->name().empty(), ""); - functions[function->name()].push_back(function); - } - - findDuplicateDefinitions(functions, "Function with same name and arguments defined twice."); -} - -void TypeChecker::checkContractDuplicateEvents(ContractDefinition const& _contract) -{ - /// Checks that two events with the same name defined in this contract have different - /// argument types - map<string, vector<EventDefinition const*>> events; - for (EventDefinition const* event: _contract.events()) - events[event->name()].push_back(event); - - findDuplicateDefinitions(events, "Event with same name and arguments defined twice."); -} - -template <class T> -void TypeChecker::findDuplicateDefinitions(map<string, vector<T>> const& _definitions, string _message) -{ - for (auto const& it: _definitions) - { - vector<T> const& overloads = it.second; - set<size_t> reported; - for (size_t i = 0; i < overloads.size() && !reported.count(i); ++i) - { - SecondarySourceLocation ssl; - - for (size_t j = i + 1; j < overloads.size(); ++j) - if (FunctionType(*overloads[i]).asCallableFunction(false)->hasEqualParameterTypes( - *FunctionType(*overloads[j]).asCallableFunction(false)) - ) - { - ssl.append("Other declaration is here:", overloads[j]->location()); - reported.insert(j); - } - - if (ssl.infos.size() > 0) - { - ssl.limitSize(_message); - - m_errorReporter.declarationError( - overloads[i]->location(), - ssl, - _message - ); - } - } - } -} - -void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _contract) -{ - // Mapping from name to function definition (exactly one per argument type equality class) and - // flag to indicate whether it is fully implemented. - using FunTypeAndFlag = std::pair<FunctionTypePointer, bool>; - map<string, vector<FunTypeAndFlag>> functions; - - // Search from base to derived - for (ContractDefinition const* contract: boost::adaptors::reverse(_contract.annotation().linearizedBaseContracts)) - for (FunctionDefinition const* function: contract->definedFunctions()) - { - // Take constructors out of overload hierarchy - if (function->isConstructor()) - continue; - auto& overloads = functions[function->name()]; - FunctionTypePointer funType = make_shared<FunctionType>(*function)->asCallableFunction(false); - auto it = find_if(overloads.begin(), overloads.end(), [&](FunTypeAndFlag const& _funAndFlag) - { - return funType->hasEqualParameterTypes(*_funAndFlag.first); - }); - if (it == overloads.end()) - overloads.push_back(make_pair(funType, function->isImplemented())); - else if (it->second) - { - if (!function->isImplemented()) - m_errorReporter.typeError(function->location(), "Redeclaring an already implemented function as abstract"); - } - else if (function->isImplemented()) - it->second = true; - } - - // Set to not fully implemented if at least one flag is false. - for (auto const& it: functions) - for (auto const& funAndFlag: it.second) - if (!funAndFlag.second) - { - FunctionDefinition const* function = dynamic_cast<FunctionDefinition const*>(&funAndFlag.first->declaration()); - solAssert(function, ""); - _contract.annotation().unimplementedFunctions.push_back(function); - break; - } -} - -void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) -{ - vector<ContractDefinition const*> const& bases = _contract.annotation().linearizedBaseContracts; - - // 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()) - if (auto baseContract = dynamic_cast<ContractDefinition const*>(&dereference(*modifier->name()))) - { - if (modifier->arguments()) - { - if (baseContract->constructor()) - annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); - } - else - m_errorReporter.declarationError( - 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 (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 -) -{ - 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()); - } - - m_errorReporter.declarationError( - *mainLocation, - ssl, - "Base constructor arguments given twice." - ); - } - -} - -void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contract) -{ - // TODO unify this at a later point. for this we need to put the constness and the access specifier - // into the types - map<string, vector<FunctionDefinition const*>> functions; - map<string, ModifierDefinition const*> modifiers; - - // We search from derived to base, so the stored item causes the error. - for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts) - { - for (FunctionDefinition const* function: contract->definedFunctions()) - { - if (function->isConstructor()) - continue; // constructors can neither be overridden nor override anything - string const& name = function->name(); - if (modifiers.count(name)) - m_errorReporter.typeError(modifiers[name]->location(), "Override changes function to modifier."); - - for (FunctionDefinition const* overriding: functions[name]) - checkFunctionOverride(*overriding, *function); - - functions[name].push_back(function); - } - for (ModifierDefinition const* modifier: contract->functionModifiers()) - { - string const& name = modifier->name(); - ModifierDefinition const*& override = modifiers[name]; - if (!override) - override = modifier; - else if (ModifierType(*override) != ModifierType(*modifier)) - m_errorReporter.typeError(override->location(), "Override changes modifier signature."); - if (!functions[name].empty()) - m_errorReporter.typeError(override->location(), "Override changes modifier to function."); - } - } -} - -void TypeChecker::checkFunctionOverride(FunctionDefinition const& _function, FunctionDefinition const& _super) -{ - FunctionTypePointer functionType = FunctionType(_function).asCallableFunction(false); - FunctionTypePointer superType = FunctionType(_super).asCallableFunction(false); - - if (!functionType->hasEqualParameterTypes(*superType)) - return; - if (!functionType->hasEqualReturnTypes(*superType)) - overrideError(_function, _super, "Overriding function return types differ."); - - if (!_function.annotation().superFunction) - _function.annotation().superFunction = &_super; - - if (_function.visibility() != _super.visibility()) - { - // Visibility change from external to public is fine. - // Any other change is disallowed. - if (!( - _super.visibility() == FunctionDefinition::Visibility::External && - _function.visibility() == FunctionDefinition::Visibility::Public - )) - overrideError(_function, _super, "Overriding function visibility differs."); - } - if (_function.stateMutability() != _super.stateMutability()) - overrideError( - _function, - _super, - "Overriding function changes state mutability from \"" + - stateMutabilityToString(_super.stateMutability()) + - "\" to \"" + - stateMutabilityToString(_function.stateMutability()) + - "\"." - ); -} - -void TypeChecker::overrideError(FunctionDefinition const& function, FunctionDefinition const& super, string message) -{ - m_errorReporter.typeError( - function.location(), - SecondarySourceLocation().append("Overridden function is here:", super.location()), - message - ); -} - -void TypeChecker::checkContractExternalTypeClashes(ContractDefinition const& _contract) -{ - map<string, vector<pair<Declaration const*, FunctionTypePointer>>> externalDeclarations; - for (ContractDefinition const* contract: _contract.annotation().linearizedBaseContracts) - { - for (FunctionDefinition const* f: contract->definedFunctions()) - if (f->isPartOfExternalInterface()) - { - auto functionType = make_shared<FunctionType>(*f); - // under non error circumstances this should be true - if (functionType->interfaceFunctionType()) - externalDeclarations[functionType->externalSignature()].push_back( - make_pair(f, functionType->asCallableFunction(false)) - ); - } - for (VariableDeclaration const* v: contract->stateVariables()) - if (v->isPartOfExternalInterface()) - { - auto functionType = make_shared<FunctionType>(*v); - // under non error circumstances this should be true - if (functionType->interfaceFunctionType()) - externalDeclarations[functionType->externalSignature()].push_back( - make_pair(v, functionType->asCallableFunction(false)) - ); - } - } - 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->hasEqualParameterTypes(*it.second[j].second)) - m_errorReporter.typeError( - it.second[j].first->location(), - "Function overload clash during conversion to external types for arguments." - ); -} - -void TypeChecker::checkLibraryRequirements(ContractDefinition const& _contract) -{ - solAssert(_contract.isLibrary(), ""); - if (!_contract.baseContracts().empty()) - m_errorReporter.typeError(_contract.location(), "Library is not allowed to inherit."); - - for (auto const& var: _contract.stateVariables()) - if (!var->isConstant()) - m_errorReporter.typeError(var->location(), "Library cannot have non-constant state variables"); -} - void TypeChecker::checkDoubleStorageAssignment(Assignment const& _assignment) { TupleType const& lhs = dynamic_cast<TupleType const&>(*type(_assignment.leftHandSide())); diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index c98a4c7f..ebfcdadc 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -66,27 +66,6 @@ public: private: bool visit(ContractDefinition const& _contract) override; - /// Checks that two functions defined in this contract with the same name have different - /// arguments and that there is at most one constructor. - void checkContractDuplicateFunctions(ContractDefinition const& _contract); - void checkContractDuplicateEvents(ContractDefinition const& _contract); - void checkContractIllegalOverrides(ContractDefinition const& _contract); - /// 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); - void checkContractAbstractFunctions(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); - /// Checks that all requirements for a library are fulfilled if this is a library. - void checkLibraryRequirements(ContractDefinition const& _contract); /// 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); @@ -157,9 +136,6 @@ private: void endVisit(ElementaryTypeNameExpression const& _expr) override; void endVisit(Literal const& _literal) override; - template <class T> - void findDuplicateDefinitions(std::map<std::string, std::vector<T>> const& _definitions, std::string _message); - bool contractDependenciesAreCyclic( ContractDefinition const& _contract, std::set<ContractDefinition const*> const& _seenContracts = std::set<ContractDefinition const*>() diff --git a/libsolidity/interface/CompilerStack.cpp b/libsolidity/interface/CompilerStack.cpp index de4a7ec2..623ccca8 100644 --- a/libsolidity/interface/CompilerStack.cpp +++ b/libsolidity/interface/CompilerStack.cpp @@ -28,6 +28,7 @@ #include <libsolidity/analysis/SemVerHandler.h> #include <libsolidity/ast/AST.h> #include <libsolidity/parsing/Parser.h> +#include <libsolidity/analysis/ContractLevelChecker.h> #include <libsolidity/analysis/ControlFlowAnalyzer.h> #include <libsolidity/analysis/ControlFlowGraph.h> #include <libsolidity/analysis/GlobalContext.h> @@ -225,8 +226,21 @@ bool CompilerStack::analyze() m_contracts[contract->fullyQualifiedName()].contract = contract; } - // This cannot be done in the above loop, because cross-contract types couldn't be resolved. - // A good example is `LibraryName.TypeName x;`. + // Next, we check inheritance, overrides, function collisions and other things at + // contract or function level. + // This also calculates whether a contract is abstract, which is needed by the + // type checker. + ContractLevelChecker contractLevelChecker(m_errorReporter); + for (Source const* source: m_sourceOrder) + for (ASTPointer<ASTNode> const& node: source->ast->nodes()) + if (ContractDefinition* contract = dynamic_cast<ContractDefinition*>(node.get())) + if (!contractLevelChecker.check(*contract)) + noErrors = false; + + // New we run full type checks that go down to the expression level. This + // cannot be done earlier, because we need cross-contract types and information + // about whether a contract is abstract for the `new` expression. + // This populates the `type` annotation for all expressions. // // Note: this does not resolve overloaded functions. In order to do that, types of arguments are needed, // which is only done one step later. diff --git a/test/libsolidity/syntaxTests/constructor/abstract_creation_forward_reference.sol b/test/libsolidity/syntaxTests/constructor/abstract_creation_forward_reference.sol new file mode 100644 index 00000000..2e6aeaa5 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/abstract_creation_forward_reference.sol @@ -0,0 +1,15 @@ +// This used to cause an internal error because of the visitation order. +contract Test { + function createChild() public { + Child asset = new Child(); + } +} + +contract Parent { + constructor(address _address) public {} +} + +contract Child is Parent { +} +// ---- +// TypeError: (146-155): Trying to create an instance of an abstract contract. diff --git a/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol new file mode 100644 index 00000000..d2a411ec --- /dev/null +++ b/test/libsolidity/syntaxTests/structs/recursion/recursive_struct_forward_reference.sol @@ -0,0 +1,11 @@ +pragma experimental ABIEncoderV2; + +contract C { + function f(Data.S memory a) public {} +} +contract Data { + struct S { S x; } +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. +// TypeError: (63-78): Internal or recursive type is not allowed for public or external functions. |