From 3eedbc6a9c60888dd967d6673a34511947da4aa1 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 10 Apr 2018 11:22:26 +0200 Subject: Error when using no parentheses in modifier-style constructor calls. --- Changelog.md | 3 ++- libsolidity/analysis/TypeChecker.cpp | 27 ++++++++++++++++++---- libsolidity/ast/AST.h | 11 +++++---- libsolidity/ast/ASTJsonConverter.cpp | 4 ++-- libsolidity/ast/AST_accept.h | 6 +++-- libsolidity/codegen/ContractCompiler.cpp | 9 +++++--- libsolidity/parsing/Parser.cpp | 6 ++--- ...low_empty_duplicated_super_constructor_call.sol | 3 +-- ...disallow_modifier_style_without_parentheses.sol | 4 ++++ 9 files changed, 52 insertions(+), 21 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inheritance/disallow_modifier_style_without_parentheses.sol diff --git a/Changelog.md b/Changelog.md index 3b8cba1d..eb35652f 100644 --- a/Changelog.md +++ b/Changelog.md @@ -12,7 +12,8 @@ Features: * Static Analyzer: Error on duplicated super constructor calls as experimental 0.5.0 feature. * Syntax Checker: Issue warning for empty structs (or error as experimental 0.5.0 feature). * General: Introduce new constructor syntax using the ``constructor`` keyword as experimental 0.5.0 feature. - * Inheritance: Error when using empty parenthesis for base class constructors that require arguments as experimental 0.5.0 feature. + * Inheritance: Error when using empty parentheses for base class constructors that require arguments as experimental 0.5.0 feature. + * Inheritance: Error when using no parentheses in modifier-style constructor calls as experimental 0.5.0 feature. Bugfixes: * Code Generator: Allow ``block.blockhash`` without being called. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 1d8fd82d..abe57778 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -293,6 +293,8 @@ void TypeChecker::checkContractAbstractFunctions(ContractDefinition const& _cont void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const& _contract) { + bool const v050 = _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); + vector const& bases = _contract.annotation().linearizedBaseContracts; // Determine the arguments that are used for the base constructors. @@ -302,8 +304,24 @@ void TypeChecker::checkContractBaseConstructorArguments(ContractDefinition const for (auto const& modifier: constructor->modifiers()) { auto baseContract = dynamic_cast(&dereference(*modifier->name())); - if (baseContract && baseContract->constructor() && !modifier->arguments().empty()) - annotateBaseConstructorArguments(_contract, baseContract->constructor(), modifier.get()); + 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 const& base: contract->baseContracts()) @@ -804,7 +822,8 @@ void TypeChecker::visitManually( vector const& _bases ) { - std::vector> const& arguments = _modifier.arguments(); + std::vector> const& arguments = + _modifier.arguments() ? *_modifier.arguments() : std::vector>(); for (ASTPointer const& argument: arguments) argument->accept(*this); _modifier.name()->accept(*this); @@ -842,7 +861,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(), diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index bc85349b..ae253f0c 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -762,19 +762,22 @@ public: ModifierInvocation( SourceLocation const& _location, ASTPointer const& _name, - std::vector> _arguments + std::unique_ptr>> _arguments ): - ASTNode(_location), m_modifierName(_name), m_arguments(_arguments) {} + ASTNode(_location), m_modifierName(_name), m_arguments(std::move(_arguments)) {} virtual void accept(ASTVisitor& _visitor) override; virtual void accept(ASTConstVisitor& _visitor) const override; ASTPointer const& name() const { return m_modifierName; } - std::vector> const& arguments() const { return m_arguments; } + // Returns nullptr if no argument list was given (``mod``). + // If an argument list is given (``mod(...)``), the arguments are returned + // as a vector of expressions. Note that this vector can be empty (``mod()``). + std::vector> const* arguments() const { return m_arguments.get(); } private: ASTPointer m_modifierName; - std::vector> m_arguments; + std::unique_ptr>> m_arguments; }; /** diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp index 94932eca..95ba3089 100644 --- a/libsolidity/ast/ASTJsonConverter.cpp +++ b/libsolidity/ast/ASTJsonConverter.cpp @@ -268,7 +268,7 @@ bool ASTJsonConverter::visit(InheritanceSpecifier const& _node) { setJsonNode(_node, "InheritanceSpecifier", { make_pair("baseName", toJson(_node.name())), - make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::Value(Json::arrayValue)) + make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::nullValue) }); return false; } @@ -378,7 +378,7 @@ bool ASTJsonConverter::visit(ModifierInvocation const& _node) { setJsonNode(_node, "ModifierInvocation", { make_pair("modifierName", toJson(*_node.name())), - make_pair("arguments", toJson(_node.arguments())) + make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::nullValue) }); return false; } diff --git a/libsolidity/ast/AST_accept.h b/libsolidity/ast/AST_accept.h index dac414fc..aeff6e4a 100644 --- a/libsolidity/ast/AST_accept.h +++ b/libsolidity/ast/AST_accept.h @@ -264,7 +264,8 @@ void ModifierInvocation::accept(ASTVisitor& _visitor) if (_visitor.visit(*this)) { m_modifierName->accept(_visitor); - listAccept(m_arguments, _visitor); + if (m_arguments) + listAccept(*m_arguments, _visitor); } _visitor.endVisit(*this); } @@ -274,7 +275,8 @@ void ModifierInvocation::accept(ASTConstVisitor& _visitor) const if (_visitor.visit(*this)) { m_modifierName->accept(_visitor); - listAccept(m_arguments, _visitor); + if (m_arguments) + listAccept(*m_arguments, _visitor); } _visitor.endVisit(*this); } diff --git a/libsolidity/codegen/ContractCompiler.cpp b/libsolidity/codegen/ContractCompiler.cpp index 3ca0b69d..5cb37103 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -222,7 +222,7 @@ void ContractCompiler::appendBaseConstructor(FunctionDefinition const& _construc if (auto inheritanceSpecifier = dynamic_cast(baseArgumentNode)) arguments = inheritanceSpecifier->arguments(); else if (auto modifierInvocation = dynamic_cast(baseArgumentNode)) - arguments = &modifierInvocation->arguments(); + arguments = modifierInvocation->arguments(); solAssert(arguments, ""); solAssert(arguments->size() == constructorType.parameterTypes().size(), ""); for (unsigned i = 0; i < arguments->size(); ++i) @@ -897,13 +897,16 @@ void ContractCompiler::appendModifierOrFunctionCode() ); ModifierDefinition const& modifier = m_context.resolveVirtualFunctionModifier(nonVirtualModifier); CompilerContext::LocationSetter locationSetter(m_context, modifier); - solAssert(modifier.parameters().size() == modifierInvocation->arguments().size(), ""); + std::vector> const& modifierArguments = + modifierInvocation->arguments() ? *modifierInvocation->arguments() : std::vector>(); + + solAssert(modifier.parameters().size() == modifierArguments.size(), ""); for (unsigned i = 0; i < modifier.parameters().size(); ++i) { m_context.addVariable(*modifier.parameters()[i]); addedVariables.push_back(modifier.parameters()[i].get()); compileExpression( - *modifierInvocation->arguments()[i], + *modifierArguments[i], modifier.parameters()[i]->annotation().type ); } diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 9a7731d8..18ef740a 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -701,17 +701,17 @@ ASTPointer Parser::parseModifierInvocation() RecursionGuard recursionGuard(*this); ASTNodeFactory nodeFactory(*this); ASTPointer name(parseIdentifier()); - vector> arguments; + unique_ptr>> arguments; if (m_scanner->currentToken() == Token::LParen) { m_scanner->next(); - arguments = parseFunctionCallListArguments(); + arguments.reset(new vector>(parseFunctionCallListArguments())); nodeFactory.markEndPosition(); expectToken(Token::RParen); } else nodeFactory.setEndPositionFromNode(name); - return nodeFactory.createNode(name, arguments); + return nodeFactory.createNode(name, move(arguments)); } ASTPointer Parser::parseIdentifier() diff --git a/test/libsolidity/syntaxTests/inheritance/allow_empty_duplicated_super_constructor_call.sol b/test/libsolidity/syntaxTests/inheritance/allow_empty_duplicated_super_constructor_call.sol index 1f580b1d..ce9d5f5f 100644 --- a/test/libsolidity/syntaxTests/inheritance/allow_empty_duplicated_super_constructor_call.sol +++ b/test/libsolidity/syntaxTests/inheritance/allow_empty_duplicated_super_constructor_call.sol @@ -1,3 +1,2 @@ contract A { constructor() public { } } -contract B1 is A { constructor() A() public { } } -contract B2 is A { constructor() A public { } } +contract B is A { constructor() A() public { } } diff --git a/test/libsolidity/syntaxTests/inheritance/disallow_modifier_style_without_parentheses.sol b/test/libsolidity/syntaxTests/inheritance/disallow_modifier_style_without_parentheses.sol new file mode 100644 index 00000000..0c159a22 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/disallow_modifier_style_without_parentheses.sol @@ -0,0 +1,4 @@ +contract A { constructor() public { } } +contract B is A { constructor() A public { } } +// ---- +// Warning: Modifier-style base constructor call without arguments. -- cgit v1.2.3