From 96eff0ff6abc614cb44a01137dfd0df1ef750088 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Wed, 4 Apr 2018 20:53:23 +0200 Subject: Error when using empty parenthesis for base class constructors that require arguments. --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 62 ++++++++++++++-------- libsolidity/ast/AST.h | 11 ++-- libsolidity/ast/ASTJsonConverter.cpp | 2 +- libsolidity/ast/AST_accept.h | 6 ++- libsolidity/codegen/ContractCompiler.cpp | 4 +- libsolidity/parsing/Parser.cpp | 6 +-- .../base_arguments_empty_parentheses.sol | 3 +- .../base_arguments_empty_parentheses_V050.sol | 9 ++++ .../inheritance/base_arguments_no_parentheses.sol | 5 ++ 10 files changed, 73 insertions(+), 36 deletions(-) create mode 100644 test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses_V050.sol create mode 100644 test/libsolidity/syntaxTests/inheritance/base_arguments_no_parentheses.sol diff --git a/Changelog.md b/Changelog.md index 9618dfa7..87e849c3 100644 --- a/Changelog.md +++ b/Changelog.md @@ -9,6 +9,7 @@ Features: * Optimizer: Optimize across ``mload`` if ``msize()`` is not used. * 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. Bugfixes: * Code Generator: Allow ``block.blockhash`` without being called. diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 620dfca4..a252742d 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -320,7 +320,7 @@ void TypeChecker::checkContractAbstractConstructors(ContractDefinition const& _c { auto baseContract = dynamic_cast(&dereference(base->name())); solAssert(baseContract, ""); - if (!base->arguments().empty()) + if (base->arguments() && !base->arguments()->empty()) argumentsNeeded.erase(baseContract); } } @@ -506,30 +506,46 @@ void TypeChecker::endVisit(InheritanceSpecifier const& _inheritance) // Interfaces do not have constructors, so there are zero parameters. parameterTypes = ContractType(*base).newExpressionType()->parameterTypes(); - if (!arguments.empty() && parameterTypes.size() != arguments.size()) + if (arguments) { - m_errorReporter.typeError( - _inheritance.location(), - "Wrong argument count for constructor call: " + - toString(arguments.size()) + - " arguments given but expected " + - toString(parameterTypes.size()) + - "." - ); - return; - } + bool v050 = m_scope->sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050); - for (size_t i = 0; i < arguments.size(); ++i) - if (!type(*arguments[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) - m_errorReporter.typeError( - arguments[i]->location(), - "Invalid type for argument in constructor call. " - "Invalid implicit conversion from " + - type(*arguments[i])->toString() + - " to " + - parameterTypes[i]->toString() + - " requested." - ); + if (parameterTypes.size() != arguments->size()) + { + if (arguments->size() == 0 && !v050) + m_errorReporter.warning( + _inheritance.location(), + "Wrong argument count for constructor call: " + + toString(arguments->size()) + + " arguments given but expected " + + toString(parameterTypes.size()) + + "." + ); + else + { + m_errorReporter.typeError( + _inheritance.location(), + "Wrong argument count for constructor call: " + + toString(arguments->size()) + + " arguments given but expected " + + toString(parameterTypes.size()) + + "." + ); + return; + } + } + for (size_t i = 0; i < arguments->size(); ++i) + if (!type(*(*arguments)[i])->isImplicitlyConvertibleTo(*parameterTypes[i])) + m_errorReporter.typeError( + (*arguments)[i]->location(), + "Invalid type for argument in constructor call. " + "Invalid implicit conversion from " + + type(*(*arguments)[i])->toString() + + " to " + + parameterTypes[i]->toString() + + " requested." + ); + } } void TypeChecker::endVisit(UsingForDirective const& _usingFor) diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 56bb412c..bc85349b 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -425,19 +425,22 @@ public: InheritanceSpecifier( SourceLocation const& _location, ASTPointer const& _baseName, - std::vector> _arguments + std::unique_ptr>> _arguments ): - ASTNode(_location), m_baseName(_baseName), m_arguments(_arguments) {} + ASTNode(_location), m_baseName(_baseName), m_arguments(std::move(_arguments)) {} virtual void accept(ASTVisitor& _visitor) override; virtual void accept(ASTConstVisitor& _visitor) const override; UserDefinedTypeName const& name() const { return *m_baseName; } - std::vector> const& arguments() const { return m_arguments; } + // Returns nullptr if no argument list was given (``C``). + // If an argument list is given (``C(...)``), the arguments are returned + // as a vector of expressions. Note that this vector can be empty (``C()``). + std::vector> const* arguments() const { return m_arguments.get(); } private: ASTPointer m_baseName; - std::vector> m_arguments; + std::unique_ptr>> m_arguments; }; /** diff --git a/libsolidity/ast/ASTJsonConverter.cpp b/libsolidity/ast/ASTJsonConverter.cpp index 4fef67c3..94932eca 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", toJson(_node.arguments())) + make_pair("arguments", _node.arguments() ? toJson(*_node.arguments()) : Json::Value(Json::arrayValue)) }); return false; } diff --git a/libsolidity/ast/AST_accept.h b/libsolidity/ast/AST_accept.h index 70ee997e..dac414fc 100644 --- a/libsolidity/ast/AST_accept.h +++ b/libsolidity/ast/AST_accept.h @@ -94,7 +94,8 @@ void InheritanceSpecifier::accept(ASTVisitor& _visitor) if (_visitor.visit(*this)) { m_baseName->accept(_visitor); - listAccept(m_arguments, _visitor); + if (m_arguments) + listAccept(*m_arguments, _visitor); } _visitor.endVisit(*this); } @@ -104,7 +105,8 @@ void InheritanceSpecifier::accept(ASTConstVisitor& _visitor) const if (_visitor.visit(*this)) { m_baseName->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 ebd9139a..d3a7e4ea 100644 --- a/libsolidity/codegen/ContractCompiler.cpp +++ b/libsolidity/codegen/ContractCompiler.cpp @@ -157,8 +157,8 @@ void ContractCompiler::appendInitAndConstructorCode(ContractDefinition const& _c ); solAssert(baseContract, ""); - if (!m_baseArguments.count(baseContract->constructor()) && !base->arguments().empty()) - m_baseArguments[baseContract->constructor()] = &base->arguments(); + 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. diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 3dbd4c8f..9a7731d8 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -286,17 +286,17 @@ ASTPointer Parser::parseInheritanceSpecifier() RecursionGuard recursionGuard(*this); ASTNodeFactory nodeFactory(*this); ASTPointer name(parseUserDefinedTypeName()); - 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, std::move(arguments)); } Declaration::Visibility Parser::parseVisibilitySpecifier(Token::Value _token) diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol index 9607ed60..b3fbd04a 100644 --- a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol +++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol @@ -3,4 +3,5 @@ contract Base { } contract Derived is Base(2) { } contract Derived2 is Base(), Derived() { } -contract Derived3 is Base, Derived {} +// ---- +// Warning: Wrong argument count for constructor call: 0 arguments given but expected 1. diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses_V050.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses_V050.sol new file mode 100644 index 00000000..b3728634 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses_V050.sol @@ -0,0 +1,9 @@ +pragma experimental "v0.5.0"; + +contract Base { + constructor(uint) public {} +} +contract Derived is Base(2) { } +contract Derived2 is Base(), Derived() { } +// ---- +// TypeError: Wrong argument count for constructor call: 0 arguments given but expected 1. diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_no_parentheses.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_no_parentheses.sol new file mode 100644 index 00000000..24cca8f0 --- /dev/null +++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_no_parentheses.sol @@ -0,0 +1,5 @@ +contract Base { + constructor(uint) public {} +} +contract Derived is Base(2) { } +contract Derived2 is Base, Derived {} -- cgit v1.2.3