diff options
15 files changed, 95 insertions, 100 deletions
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<ContractDefinition const*>(_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<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) - 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 (baseContract && baseContract->constructor() && !modifier->arguments().empty()) + annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); } - for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts()) { auto baseContract = dynamic_cast<ContractDefinition const*>(&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<ContractDefinition const*> contractDependencies; + /// Mapping containing the nodes that define the arguments for base constructors. + /// These can either be inheritance specifiers or modifier invocations. + std::map<FunctionDefinition const*, ASTNode const*> 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<ContractDefinition const*> 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<ContractDefinition const*>( - modifier->name()->annotation().referencedDeclaration - ); - if (baseContract && !modifier->arguments().empty()) - if (m_baseArguments.count(baseContract->constructor()) == 0) - m_baseArguments[baseContract->constructor()] = &modifier->arguments(); - } - for (ASTPointer<InheritanceSpecifier> const& base: contract->baseContracts()) - { - ContractDefinition const* baseContract = dynamic_cast<ContractDefinition const*>( - 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<ASTPointer<Expression>> const* arguments = m_baseArguments[&_constructor]; + solAssert(m_baseArguments, ""); + solAssert(m_baseArguments->count(&_constructor), ""); + std::vector<ASTPointer<Expression>> const* arguments = nullptr; + ASTNode const* baseArgumentNode = m_baseArguments->at(&_constructor); + if (auto inheritanceSpecifier = dynamic_cast<InheritanceSpecifier const*>(baseArgumentNode)) + arguments = inheritanceSpecifier->arguments(); + else if (auto modifierInvocation = dynamic_cast<ModifierInvocation const*>(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<FunctionDefinition const*, std::vector<ASTPointer<Expression>> const*> m_baseArguments; + std::map<FunctionDefinition const*, ASTNode const*> const* m_baseArguments; }; } diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_multiple_inheritance.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_multiple_inheritance.sol index f63d0f02..5483d5d7 100644 --- a/test/libsolidity/syntaxTests/inheritance/base_arguments_multiple_inheritance.sol +++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_multiple_inheritance.sol @@ -6,4 +6,4 @@ contract Derived is Base, Base1 { constructor(uint i) Base(i) public {} } // ---- -// Warning: Duplicated super constructor calls are deprecated. +// Warning: Base constructor arguments given twice. diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_ancestor_constructor_call.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/ancestor.sol index 97f3f8ff..8b1af245 100644 --- a/test/libsolidity/syntaxTests/inheritance/duplicated_ancestor_constructor_call.sol +++ b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/ancestor.sol @@ -2,4 +2,4 @@ contract A { constructor(uint) public { } } contract B is A(2) { constructor() public { } } contract C is B { constructor() A(3) public { } } // ---- -// Warning: Duplicated super constructor calls are deprecated. +// Warning: Base constructor arguments given twice. diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_ancestor_constructor_call_V050.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/ancestor_V050.sol index 933c9087..6616c9a9 100644 --- a/test/libsolidity/syntaxTests/inheritance/duplicated_ancestor_constructor_call_V050.sol +++ b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/ancestor_V050.sol @@ -4,4 +4,4 @@ contract A { constructor(uint) public { } } contract B is A(2) { constructor() public { } } contract C is B { constructor() A(3) public { } } // ---- -// DeclarationError: Duplicated super constructor call. +// DeclarationError: Base constructor arguments given twice. diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base.sol index 876b07ea..1fb504fe 100644 --- a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call.sol +++ b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base.sol @@ -1,4 +1,4 @@ contract A { constructor(uint) public { } } contract B is A(2) { constructor() A(3) public { } } // ---- -// Warning: Duplicated super constructor calls are deprecated. +// Warning: Base constructor arguments given twice. diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call_V050.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_V050.sol index 31a363fd..96eb1bb1 100644 --- a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call_V050.sol +++ b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_V050.sol @@ -3,4 +3,4 @@ pragma experimental "v0.5.0"; contract A { constructor(uint) public { } } contract B is A(2) { constructor() A(3) public { } } // ---- -// DeclarationError: Duplicated super constructor call. +// DeclarationError: Base constructor arguments given twice. diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call_multi.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi.sol index caed18eb..db9ffc85 100644 --- a/test/libsolidity/syntaxTests/inheritance/duplicated_super_constructor_call_multi.sol +++ b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi.sol @@ -3,5 +3,5 @@ contract A is C(2) {} contract B is C(2) {} contract D is A, B { constructor() C(3) public {} } // ---- -// Warning: Duplicated super constructor calls are deprecated. -// Warning: Duplicated super constructor calls are deprecated. +// Warning: Base constructor arguments given twice. +// Warning: Base constructor arguments given twice. diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi_no_constructor.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi_no_constructor.sol new file mode 100644 index 00000000..fe280ad5 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi_no_constructor.sol @@ -0,0 +1,6 @@ +contract C { constructor(uint) public {} } +contract A is C(2) {} +contract B is C(2) {} +contract D is A, B {} +// ---- +// Warning: Base constructor arguments given twice. diff --git a/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi_no_constructor_modifier_style.sol b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi_no_constructor_modifier_style.sol new file mode 100644 index 00000000..ea85aae7 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/duplicated_constructor_call/base_multi_no_constructor_modifier_style.sol @@ -0,0 +1,6 @@ +contract C { constructor(uint) public {} } +contract A is C { constructor() C(2) public {} } +contract B is C { constructor() C(2) public {} } +contract D is A, B { } +// ---- +// Warning: Base constructor arguments given twice. |