From d664a599e68291166c47fcece464cb8d0af31df8 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Thu, 1 Mar 2018 18:39:01 +0100 Subject: Constructors are defined using the ``constructor`` keyword. --- Changelog.md | 1 + libsolidity/analysis/SyntaxChecker.cpp | 17 +++++++++- libsolidity/ast/AST.h | 3 +- libsolidity/parsing/Parser.cpp | 26 +++++++++++---- libsolidity/parsing/Parser.h | 1 + test/libsolidity/SolidityNameAndTypeResolution.cpp | 39 ++++++++++++++++++++-- .../base_arguments_empty_parentheses.sol | 2 +- .../inheritance/too_few_base_arguments.sol | 4 +-- 8 files changed, 80 insertions(+), 13 deletions(-) diff --git a/Changelog.md b/Changelog.md index 34c3b0e9..b9619c27 100644 --- a/Changelog.md +++ b/Changelog.md @@ -7,6 +7,7 @@ Features: * Interfaces: Allow overriding external functions in interfaces with public in an implementing contract. * 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. Bugfixes: * Code Generator: Allow ``block.blockhash`` without being called. diff --git a/libsolidity/analysis/SyntaxChecker.cpp b/libsolidity/analysis/SyntaxChecker.cpp index b595c4d1..343b4ba8 100644 --- a/libsolidity/analysis/SyntaxChecker.cpp +++ b/libsolidity/analysis/SyntaxChecker.cpp @@ -216,7 +216,22 @@ bool SyntaxChecker::visit(FunctionDefinition const& _function) if (v050 && _function.noVisibilitySpecified()) m_errorReporter.syntaxError(_function.location(), "No visibility specified."); - + + if (_function.isOldStyleConstructor()) + { + if (v050) + m_errorReporter.syntaxError( + _function.location(), + "Functions are not allowed to have the same name as the contract. " + "If you intend this to be a constructor, use \"constructor(...) { ... }\" to define it." + ); + else + m_errorReporter.warning( + _function.location(), + "Defining constructors as functions with the same name as the contract is deprecated. " + "Use \"constructor(...) { ... }\" instead." + ); + } return true; } diff --git a/libsolidity/ast/AST.h b/libsolidity/ast/AST.h index 9c67d354..56bb412c 100644 --- a/libsolidity/ast/AST.h +++ b/libsolidity/ast/AST.h @@ -607,7 +607,8 @@ public: StateMutability stateMutability() const { return m_stateMutability; } bool isConstructor() const { return m_isConstructor; } - bool isFallback() const { return name().empty(); } + bool isOldStyleConstructor() const { return m_isConstructor && !name().empty(); } + bool isFallback() const { return !m_isConstructor && name().empty(); } bool isPayable() const { return m_stateMutability == StateMutability::Payable; } std::vector> const& modifiers() const { return m_functionModifiers; } std::vector> const& returnParameters() const { return m_returnParameters->parameters(); } diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 8c97f55f..e5cc69d8 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -238,7 +238,10 @@ ASTPointer Parser::parseContractDefinition(Token::Value _exp Token::Value currentTokenValue = m_scanner->currentToken(); if (currentTokenValue == Token::RBrace) break; - else if (currentTokenValue == Token::Function) + else if ( + currentTokenValue == Token::Function || + (currentTokenValue == Token::Identifier && m_scanner->currentLiteral() == "constructor") + ) // This can be a function or a state variable of function type (especially // complicated to distinguish fallback function from function type state variable) subNodes.push_back(parseFunctionDefinitionOrFunctionTypeStateVariable(name.get())); @@ -333,9 +336,19 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _forceEmptyN { RecursionGuard recursionGuard(*this); FunctionHeaderParserResult result; - expectToken(Token::Function); - if (_forceEmptyName || m_scanner->currentToken() == Token::LParen) - result.name = make_shared(); // anonymous function + + if (m_scanner->currentToken() == Token::Function) + // In case of old style constructors, i.e. functions with the same name as the contract, + // this is set to true later in parseFunctionDefinitionOrFunctionTypeStateVariable. + result.isConstructor = false; + else if (m_scanner->currentToken() == Token::Identifier && m_scanner->currentLiteral() == "constructor") + result.isConstructor = true; + else + solAssert(false, "Function or constructor expected."); + m_scanner->next(); + + if (result.isConstructor || _forceEmptyName || m_scanner->currentToken() == Token::LParen) + result.name = make_shared(); else result.name = expectIdentifierToken(); VarDeclParserOptions options; @@ -426,12 +439,13 @@ ASTPointer Parser::parseFunctionDefinitionOrFunctionTypeStateVariable(A } else m_scanner->next(); // just consume the ';' - bool const c_isConstructor = (_contractName && *header.name == *_contractName); + if (_contractName && *header.name == *_contractName) + header.isConstructor = true; return nodeFactory.createNode( header.name, header.visibility, header.stateMutability, - c_isConstructor, + header.isConstructor, docstring, header.parameters, header.modifiers, diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h index 3f780af9..2d0e52e1 100644 --- a/libsolidity/parsing/Parser.h +++ b/libsolidity/parsing/Parser.h @@ -56,6 +56,7 @@ private: /// This struct is shared for parsing a function header and a function type. struct FunctionHeaderParserResult { + bool isConstructor; ASTPointer name; ASTPointer parameters; ASTPointer returnParameters; diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index dcdc1519..874cc0a6 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -962,6 +962,41 @@ BOOST_AUTO_TEST_CASE(base_constructor_arguments_override) CHECK_SUCCESS(text); } +BOOST_AUTO_TEST_CASE(new_constructor_syntax) +{ + char const* text = R"( + contract A { constructor() public {} } + )"; + CHECK_SUCCESS(text); + + text = R"( + pragma experimental "v0.5.0"; + contract A { constructor() public {} } + )"; + CHECK_SUCCESS(text); +} + +BOOST_AUTO_TEST_CASE(old_constructor_syntax) +{ + char const* text = R"( + contract A { function A() public {} } + )"; + CHECK_WARNING( + text, + "Defining constructors as functions with the same name as the contract is deprecated." + ); + + text = R"( + pragma experimental "v0.5.0"; + contract A { function A() public {} } + )"; + CHECK_ERROR( + text, + SyntaxError, + "Functions are not allowed to have the same name as the contract." + ); +} + BOOST_AUTO_TEST_CASE(implicit_derived_to_base_conversion) { char const* text = R"( @@ -6916,7 +6951,7 @@ BOOST_AUTO_TEST_CASE(shadowing_builtins_ignores_constructor) { char const* text = R"( contract C { - function C() public {} + constructor() public {} } )"; CHECK_SUCCESS_NO_WARNINGS(text); @@ -7328,7 +7363,7 @@ BOOST_AUTO_TEST_CASE(using_this_in_constructor) { char const* text = R"( contract C { - function C() public { + constructor() public { this.f(); } function f() pure public { diff --git a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol index 76df0657..9607ed60 100644 --- a/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol +++ b/test/libsolidity/syntaxTests/inheritance/base_arguments_empty_parentheses.sol @@ -1,5 +1,5 @@ contract Base { - function Base(uint) public {} + constructor(uint) public {} } contract Derived is Base(2) { } contract Derived2 is Base(), Derived() { } diff --git a/test/libsolidity/syntaxTests/inheritance/too_few_base_arguments.sol b/test/libsolidity/syntaxTests/inheritance/too_few_base_arguments.sol index 82aba308..45a0770f 100644 --- a/test/libsolidity/syntaxTests/inheritance/too_few_base_arguments.sol +++ b/test/libsolidity/syntaxTests/inheritance/too_few_base_arguments.sol @@ -1,9 +1,9 @@ contract Base { - function Base(uint, uint) public {} + constructor(uint, uint) public {} } contract Derived is Base(2) { } contract Derived2 is Base { - function Derived2() Base(2) public { } + constructor() Base(2) public { } } // ---- // TypeError: Wrong argument count for constructor call: 1 arguments given but expected 2. -- cgit From 07c74ef9241b1fe4fdec5cf4bd97e2c1aedb8d0d Mon Sep 17 00:00:00 2001 From: bitshift Date: Fri, 2 Mar 2018 16:44:35 +0100 Subject: Updates docs to new constructor syntax. --- docs/contracts.rst | 32 ++++++++++++++++++++++++++------ 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index 8cc4f6b2..c4eda8dc 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -40,7 +40,7 @@ This means that cyclic creation dependencies are impossible. :: - pragma solidity ^0.4.16; + pragma solidity ^0.4.20; // should actually be 0.4.21 contract OwnedToken { // TokenCreator is a contract type that is defined below. @@ -52,7 +52,7 @@ This means that cyclic creation dependencies are impossible. // This is the constructor which registers the // creator and the assigned name. - function OwnedToken(bytes32 _name) public { + constructor(bytes32 _name) public { // State variables are accessed via their name // and not via e.g. this.owner. This also applies // to functions and especially in the constructors, @@ -976,9 +976,30 @@ virtual method lookup. Constructors ============ -A constructor is an optional function with the same name as the contract which is executed upon contract creation. +A constructor is an optional function declared with the ``constructor`` keyword which is executed upon contract creation. Constructor functions can be either ``public`` or ``internal``. +:: + + pragma solidity ^0.4.20; // should actually be 0.4.21 + + contract A { + uint public a; + + constructor(uint _a) internal { + a = _a; + } + } + + contract B is A(1) { + constructor() public {} + } + +A constructor set as ``internal`` causes the contract to be marked as :ref:`abstract `. + +.. note :: + Prior to version 0.4.21, constructors were defined as functions with the same name as the contract. This syntax is now deprecated. + :: pragma solidity ^0.4.11; @@ -995,7 +1016,6 @@ Constructor functions can be either ``public`` or ``internal``. function B() public {} } -A constructor set as ``internal`` causes the contract to be marked as :ref:`abstract `. .. index:: ! base;constructor @@ -1009,11 +1029,11 @@ the base constructors. This can be done in two ways:: contract Base { uint x; - function Base(uint _x) public { x = _x; } + constructor(uint _x) public { x = _x; } } contract Derived is Base(7) { - function Derived(uint _y) Base(_y * _y) public { + constructor(uint _y) Base(_y * _y) public { } } -- cgit From c98464db0607f62aa791aa1f7559a76623e254e7 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 5 Mar 2018 16:35:31 +0100 Subject: Remove redundant test and enforce success without warnings. --- test/libsolidity/SolidityNameAndTypeResolution.cpp | 8 +------- 1 file changed, 1 insertion(+), 7 deletions(-) diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 874cc0a6..b6596327 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -967,13 +967,7 @@ BOOST_AUTO_TEST_CASE(new_constructor_syntax) char const* text = R"( contract A { constructor() public {} } )"; - CHECK_SUCCESS(text); - - text = R"( - pragma experimental "v0.5.0"; - contract A { constructor() public {} } - )"; - CHECK_SUCCESS(text); + CHECK_SUCCESS_NO_WARNINGS(text); } BOOST_AUTO_TEST_CASE(old_constructor_syntax) -- cgit From f855c78a0839d6f569cb88eda290439b69a680e3 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 6 Mar 2018 17:37:47 +0100 Subject: Update version pragma and use new constructor syntax in std/ contracts. --- docs/contracts.rst | 6 +++--- std/StandardToken.sol | 4 ++-- std/owned.sol | 4 ++-- 3 files changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index c4eda8dc..e3f5bbac 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -40,7 +40,7 @@ This means that cyclic creation dependencies are impossible. :: - pragma solidity ^0.4.20; // should actually be 0.4.21 + pragma solidity >0.4.21; contract OwnedToken { // TokenCreator is a contract type that is defined below. @@ -981,7 +981,7 @@ Constructor functions can be either ``public`` or ``internal``. :: - pragma solidity ^0.4.20; // should actually be 0.4.21 + pragma solidity >0.4.21; contract A { uint public a; @@ -998,7 +998,7 @@ Constructor functions can be either ``public`` or ``internal``. A constructor set as ``internal`` causes the contract to be marked as :ref:`abstract `. .. note :: - Prior to version 0.4.21, constructors were defined as functions with the same name as the contract. This syntax is now deprecated. + Prior to version 0.4.22, constructors were defined as functions with the same name as the contract. This syntax is now deprecated. :: diff --git a/std/StandardToken.sol b/std/StandardToken.sol index 1b218d67..5afc9747 100644 --- a/std/StandardToken.sol +++ b/std/StandardToken.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.0; +pragma solidity >0.4.21; import "./Token.sol"; @@ -8,7 +8,7 @@ contract StandardToken is Token { mapping (address => mapping (address => uint256)) m_allowance; - function StandardToken(address _initialOwner, uint256 _supply) public { + constructor(address _initialOwner, uint256 _supply) public { supply = _supply; balance[_initialOwner] = _supply; } diff --git a/std/owned.sol b/std/owned.sol index ee9860d3..8e1d5917 100644 --- a/std/owned.sol +++ b/std/owned.sol @@ -1,4 +1,4 @@ -pragma solidity ^0.4.0; +pragma solidity >0.4.21; contract owned { address owner; @@ -9,7 +9,7 @@ contract owned { } } - function owned() public { + constructor() public { owner = msg.sender; } } -- cgit From e2dac9ed397c29bfe426912c28ef2d419b2324c8 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Wed, 7 Mar 2018 16:06:09 +0100 Subject: Set header.isConstructor for old style constructors in parseFunctionHeader as well. --- libsolidity/parsing/Parser.cpp | 16 +++++++++++----- libsolidity/parsing/Parser.h | 6 +++++- 2 files changed, 16 insertions(+), 6 deletions(-) diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index e5cc69d8..6ae66eee 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -332,14 +332,18 @@ StateMutability Parser::parseStateMutability(Token::Value _token) return stateMutability; } -Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _forceEmptyName, bool _allowModifiers) +Parser::FunctionHeaderParserResult Parser::parseFunctionHeader( + bool _forceEmptyName, + bool _allowModifiers, + ASTString const* _contractName +) { RecursionGuard recursionGuard(*this); FunctionHeaderParserResult result; if (m_scanner->currentToken() == Token::Function) // In case of old style constructors, i.e. functions with the same name as the contract, - // this is set to true later in parseFunctionDefinitionOrFunctionTypeStateVariable. + // this is set to true below. result.isConstructor = false; else if (m_scanner->currentToken() == Token::Identifier && m_scanner->currentLiteral() == "constructor") result.isConstructor = true; @@ -351,6 +355,10 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader(bool _forceEmptyN result.name = make_shared(); else result.name = expectIdentifierToken(); + + if (!result.name->empty() && _contractName && *result.name == *_contractName) + result.isConstructor = true; + VarDeclParserOptions options; options.allowLocationSpecifier = true; result.parameters = parseParameterList(options); @@ -420,7 +428,7 @@ ASTPointer Parser::parseFunctionDefinitionOrFunctionTypeStateVariable(A if (m_scanner->currentCommentLiteral() != "") docstring = make_shared(m_scanner->currentCommentLiteral()); - FunctionHeaderParserResult header = parseFunctionHeader(false, true); + FunctionHeaderParserResult header = parseFunctionHeader(false, true, _contractName); if ( !header.modifiers.empty() || @@ -439,8 +447,6 @@ ASTPointer Parser::parseFunctionDefinitionOrFunctionTypeStateVariable(A } else m_scanner->next(); // just consume the ';' - if (_contractName && *header.name == *_contractName) - header.isConstructor = true; return nodeFactory.createNode( header.name, header.visibility, diff --git a/libsolidity/parsing/Parser.h b/libsolidity/parsing/Parser.h index 2d0e52e1..eb120a61 100644 --- a/libsolidity/parsing/Parser.h +++ b/libsolidity/parsing/Parser.h @@ -74,7 +74,11 @@ private: ASTPointer parseInheritanceSpecifier(); Declaration::Visibility parseVisibilitySpecifier(Token::Value _token); StateMutability parseStateMutability(Token::Value _token); - FunctionHeaderParserResult parseFunctionHeader(bool _forceEmptyName, bool _allowModifiers); + FunctionHeaderParserResult parseFunctionHeader( + bool _forceEmptyName, + bool _allowModifiers, + ASTString const* _contractName = nullptr + ); ASTPointer parseFunctionDefinitionOrFunctionTypeStateVariable(ASTString const* _contractName); ASTPointer parseFunctionDefinition(ASTString const* _contractName); ASTPointer parseStructDefinition(); -- cgit From 8f66390f56c866f70ab6617dac1f0713ace2d7ff Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Fri, 9 Mar 2018 14:23:48 +0100 Subject: Set isConstructor to false unconditionally and update to true later for constructors. --- libsolidity/parsing/Parser.cpp | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/libsolidity/parsing/Parser.cpp b/libsolidity/parsing/Parser.cpp index 6ae66eee..3dbd4c8f 100644 --- a/libsolidity/parsing/Parser.cpp +++ b/libsolidity/parsing/Parser.cpp @@ -341,13 +341,11 @@ Parser::FunctionHeaderParserResult Parser::parseFunctionHeader( RecursionGuard recursionGuard(*this); FunctionHeaderParserResult result; - if (m_scanner->currentToken() == Token::Function) - // In case of old style constructors, i.e. functions with the same name as the contract, - // this is set to true below. - result.isConstructor = false; - else if (m_scanner->currentToken() == Token::Identifier && m_scanner->currentLiteral() == "constructor") + result.isConstructor = false; + + if (m_scanner->currentToken() == Token::Identifier && m_scanner->currentLiteral() == "constructor") result.isConstructor = true; - else + else if (m_scanner->currentToken() != Token::Function) solAssert(false, "Function or constructor expected."); m_scanner->next(); -- cgit From 3ae326139a505bed877d5b9ac9b4b3ed84496c3d Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Fri, 9 Mar 2018 14:25:57 +0100 Subject: Document absence of constructors. --- docs/contracts.rst | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/contracts.rst b/docs/contracts.rst index e3f5bbac..9ae80209 100644 --- a/docs/contracts.rst +++ b/docs/contracts.rst @@ -977,7 +977,9 @@ virtual method lookup. Constructors ============ A constructor is an optional function declared with the ``constructor`` keyword which is executed upon contract creation. -Constructor functions can be either ``public`` or ``internal``. +Constructor functions can be either ``public`` or ``internal``. If there is no constructor, the contract will assume the +default constructor: ``contructor() public {}``. + :: -- cgit