From 2c2c976697d22400d37f3b26064b6b1f6fc91dba Mon Sep 17 00:00:00 2001 From: chriseth Date: Thu, 29 Nov 2018 17:58:15 +0100 Subject: Move base constructor argument checks. --- libsolidity/analysis/ContractLevelChecker.cpp | 91 +++++++++++++++++++++++++++ libsolidity/analysis/ContractLevelChecker.h | 6 ++ libsolidity/analysis/TypeChecker.cpp | 87 ------------------------- libsolidity/analysis/TypeChecker.h | 6 -- 4 files changed, 97 insertions(+), 93 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/ContractLevelChecker.cpp b/libsolidity/analysis/ContractLevelChecker.cpp index 74ac665f..f41e569f 100644 --- a/libsolidity/analysis/ContractLevelChecker.cpp +++ b/libsolidity/analysis/ContractLevelChecker.cpp @@ -39,6 +39,7 @@ bool ContractLevelChecker::check(ContractDefinition const& _contract) checkContractDuplicateEvents(_contract); checkContractIllegalOverrides(_contract); checkContractAbstractFunctions(_contract); + checkContractBaseConstructorArguments(_contract); return Error::containsOnlyWarnings(m_errorReporter.errors()); } @@ -248,3 +249,93 @@ void ContractLevelChecker::checkContractAbstractFunctions(ContractDefinition con break; } } + + +void ContractLevelChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) +{ + vector 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( + 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 const& base: contract->baseContracts()) + { + ContractDefinition const* baseContract = dynamic_cast( + 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." + ); + } + +} diff --git a/libsolidity/analysis/ContractLevelChecker.h b/libsolidity/analysis/ContractLevelChecker.h index 1746153b..fc0d6972 100644 --- a/libsolidity/analysis/ContractLevelChecker.h +++ b/libsolidity/analysis/ContractLevelChecker.h @@ -64,6 +64,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 checkContractBaseConstructorArguments(ContractDefinition const& _contract); + void annotateBaseConstructorArguments( + ContractDefinition const& _currentContract, + FunctionDefinition const* _baseConstructor, + ASTNode const* _argumentNode + ); langutil::ErrorReporter& m_errorReporter; }; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 2b79e29b..0ae7ee7a 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -95,8 +95,6 @@ bool TypeChecker::visit(ContractDefinition const& _contract) ASTNode::listAccept(_contract.definedStructs(), *this); ASTNode::listAccept(_contract.baseContracts(), *this); - checkContractBaseConstructorArguments(_contract); - FunctionDefinition const* function = _contract.constructor(); if (function) { @@ -157,91 +155,6 @@ bool TypeChecker::visit(ContractDefinition const& _contract) return false; } -void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) -{ - vector 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(&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 const& base: contract->baseContracts()) - { - auto baseContract = dynamic_cast(&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::checkContractExternalTypeClashes(ContractDefinition const& _contract) { map>> externalDeclarations; diff --git a/libsolidity/analysis/TypeChecker.h b/libsolidity/analysis/TypeChecker.h index 61eee386..d5ccb263 100644 --- a/libsolidity/analysis/TypeChecker.h +++ b/libsolidity/analysis/TypeChecker.h @@ -66,12 +66,6 @@ public: private: bool visit(ContractDefinition const& _contract) override; - 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); -- cgit