From b918a105a40aa90fe9b89eecbcdfc7ac2937c141 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 5 Apr 2018 16:25:20 +0200 Subject: Move constructor argument override check to TypeChecker and reuse annotations in ContractCompiler. --- libsolidity/analysis/StaticAnalyzer.cpp | 42 ------------------ libsolidity/analysis/StaticAnalyzer.h | 1 - libsolidity/analysis/TypeChecker.cpp | 75 +++++++++++++++++++++++--------- libsolidity/analysis/TypeChecker.h | 7 ++- libsolidity/ast/ASTAnnotations.h | 3 ++ libsolidity/codegen/ContractCompiler.cpp | 39 +++++------------ libsolidity/codegen/ContractCompiler.h | 2 +- 7 files changed, 76 insertions(+), 93 deletions(-) (limited to 'libsolidity') diff --git a/libsolidity/analysis/StaticAnalyzer.cpp b/libsolidity/analysis/StaticAnalyzer.cpp index 700c8a71..33b0e296 100644 --- a/libsolidity/analysis/StaticAnalyzer.cpp +++ b/libsolidity/analysis/StaticAnalyzer.cpp @@ -90,48 +90,6 @@ void StaticAnalyzer::endVisit(FunctionDefinition const&) m_localVarUseCount.clear(); } -bool StaticAnalyzer::visit(ModifierInvocation const& _modifier) -{ - if (!m_constructor) - return true; - - bool const v050 = m_currentContract->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - - if (auto contract = dynamic_cast(_modifier.name()->annotation().referencedDeclaration)) - for (auto const& base: m_currentContract->annotation().linearizedBaseContracts) - for (auto const& specifier: base->baseContracts()) - { - Declaration const* parent = specifier->name().annotation().referencedDeclaration; - if (contract == parent && !specifier->arguments().empty()) - { - if (v050) - { - SecondarySourceLocation ssl; - ssl.append("Second constructor call is here:", specifier->location()); - - m_errorReporter.declarationError( - _modifier.location(), - ssl, - "Duplicated super constructor call." - ); - } - else - { - SecondarySourceLocation ssl; - ssl.append("Overridden constructor call is here:", specifier->location()); - - m_errorReporter.warning( - _modifier.location(), - "Duplicated super constructor calls are deprecated.", - ssl - ); - } - } - } - - return true; -} - bool StaticAnalyzer::visit(Identifier const& _identifier) { if (m_currentFunction) diff --git a/libsolidity/analysis/StaticAnalyzer.h b/libsolidity/analysis/StaticAnalyzer.h index e68325bc..0a806bbd 100644 --- a/libsolidity/analysis/StaticAnalyzer.h +++ b/libsolidity/analysis/StaticAnalyzer.h @@ -57,7 +57,6 @@ private: virtual bool visit(FunctionDefinition const& _function) override; virtual void endVisit(FunctionDefinition const& _function) override; - virtual bool visit(ModifierInvocation const& _modifier) override; virtual bool visit(ExpressionStatement const& _statement) override; virtual bool visit(VariableDeclaration const& _variable) override; diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index a252742d..c6868a0a 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -101,7 +101,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 +291,75 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont } } -void TypeChecker::checkContractAbstractConstructors(ContractDefinition const& _contract) +void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) { - set argumentsNeeded; - // check that we get arguments for all base constructors that need it. - // If not mark the contract as abstract (not fully implemented) - vector 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( - &dereference(*modifier->name()) - ); - if (baseContract) - argumentsNeeded.erase(baseContract); + auto baseContract = dynamic_cast(&dereference(*modifier->name())); + if (baseContract && baseContract->constructor() && !modifier->arguments().empty()) + annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); } - for (ASTPointer const& base: contract->baseContracts()) { auto baseContract = dynamic_cast(&dereference(base->name())); solAssert(baseContract, ""); - if (base->arguments() && !base->arguments()->empty()) - argumentsNeeded.erase(baseContract); + + if (baseContract->constructor() && base->arguments() && !base->arguments()->empty()) + annotateBaseConstructorArguments(_contract, baseContract->constructor(), base.get()); } } - if (!argumentsNeeded.empty()) - for (ContractDefinition const* contract: argumentsNeeded) - _contract.annotation().unimplementedFunctions.push_back(contract->constructor()); + + // 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; + + SecondarySourceLocation ssl; + ssl.append("Second constructor call is here:", _argumentNode->location()); + + if (v050) + m_errorReporter.declarationError( + previousNode->location(), + ssl, + "Base constructor arguments given twice." + ); + else + m_errorReporter.warning( + previousNode->location(), + "Base constructor arguments given twice.", + ssl + ); + } + } void TypeChecker::checkContractIllegalOverrides(ContractDefinition const& _contract) 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/ast/ASTAnnotations.h b/libsolidity/ast/ASTAnnotations.h index 3d4236cc..5cbe42bd 100644 --- a/libsolidity/ast/ASTAnnotations.h +++ b/libsolidity/ast/ASTAnnotations.h @@ -90,6 +90,9 @@ struct ContractDefinitionAnnotation: TypeDeclarationAnnotation, DocumentedAnnota /// List of contracts this contract creates, i.e. which need to be compiled first. /// Also includes all contracts from @a linearizedBaseContracts. std::set contractDependencies; + /// Mapping containing the nodes that define the arguments for base constructors. + /// These can either be inheritance specifiers or modifier invocations. + std::map baseConstructorArguments; }; struct FunctionDefinitionAnnotation: ASTAnnotation, DocumentedAnnotation diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index d3a7e4ea..3ca0b69d 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -135,34 +135,13 @@ void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _c { solAssert(!_contract.isLibrary(), "Tried to initialize library."); CompilerContext::LocationSetter locationSetter(m_context, _contract); - // Determine the arguments that are used for the base constructors. - std::vector const& bases = _contract.annotation().linearizedBaseContracts; - for (ContractDefinition const* contract: bases) - { - if (FunctionDefinition const* constructor = contract->constructor()) - for (auto const& modifier: constructor->modifiers()) - { - auto baseContract = dynamic_cast( - modifier->name()->annotation().referencedDeclaration - ); - if (baseContract && !modifier->arguments().empty()) - if (m_baseArguments.count(baseContract->constructor()) == 0) - m_baseArguments[baseContract->constructor()] = &modifier->arguments(); - } - for (ASTPointer const& base: contract->baseContracts()) - { - ContractDefinition const* baseContract = dynamic_cast( - base->name().annotation().referencedDeclaration - ); - solAssert(baseContract, ""); + m_baseArguments = &_contract.annotation().baseConstructorArguments; - if (!m_baseArguments.count(baseContract->constructor()) && base->arguments() && !base->arguments()->empty()) - m_baseArguments[baseContract->constructor()] = base->arguments(); - } - } // Initialization of state variables in base-to-derived order. - for (ContractDefinition const* contract: boost::adaptors::reverse(bases)) + for (ContractDefinition const* contract: boost::adaptors::reverse( + _contract.annotation().linearizedBaseContracts + )) initializeStateVariables(*contract); if (FunctionDefinition const* constructor = _contract.constructor()) @@ -236,8 +215,14 @@ void ContractCompiler::appendBaseConstructor(FunctionDefinition const& _construc FunctionType constructorType(_constructor); if (!constructorType.parameterTypes().empty()) { - solAssert(m_baseArguments.count(&_constructor), ""); - std::vector> const* arguments = m_baseArguments[&_constructor]; + solAssert(m_baseArguments, ""); + solAssert(m_baseArguments->count(&_constructor), ""); + std::vector> const* arguments = nullptr; + ASTNode const* baseArgumentNode = m_baseArguments->at(&_constructor); + if (auto inheritanceSpecifier = dynamic_cast(baseArgumentNode)) + arguments = inheritanceSpecifier->arguments(); + else if (auto modifierInvocation = dynamic_cast(baseArgumentNode)) + arguments = &modifierInvocation->arguments(); solAssert(arguments, ""); solAssert(arguments->size() == constructorType.parameterTypes().size(), ""); for (unsigned i = 0; i < arguments->size(); ++i) diff --git a/libsolidity/codegen/ContractCompiler.h b/libsolidity/codegen/ContractCompiler.h index e04a56fb..02a3452f 100644 --- a/libsolidity/codegen/ContractCompiler.h +++ b/libsolidity/codegen/ContractCompiler.h @@ -135,7 +135,7 @@ private: FunctionDefinition const* m_currentFunction = nullptr; unsigned m_stackCleanupForReturn = 0; ///< this number of stack elements need to be removed before jump to m_returnTag // arguments for base constructors, filled in derived-to-base order - std::map> const*> m_baseArguments; + std::map const* m_baseArguments; }; } -- cgit v1.2.3