diff options
6 files changed, 61 insertions, 50 deletions
diff --git a/Changelog.md b/Changelog.md index cfd23ad0..a0df396d 100644 --- a/Changelog.md +++ b/Changelog.md @@ -4,7 +4,8 @@ Features: * SMTChecker: Integration with CVC4 SMT solver Bugfixes: - + * Type Checker: Do not complain about new-style constructor and fallback function to have the same name. + * Type Checker: Detect multiple constructor declarations in the new syntax and old syntax. ### 0.4.22 (2018-04-16) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 47a551dc..87d69d97 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -109,39 +109,28 @@ bool TypeChecker::visit(ContractDefinition const& _contract) m_errorReporter.typeError(function->location(), "Constructor must be public or internal."); } - FunctionDefinition const* fallbackFunction = nullptr; for (FunctionDefinition const* function: _contract.definedFunctions()) - { if (function->isFallback()) { - if (fallbackFunction) - { - m_errorReporter.declarationError(function->location(), "Only one fallback function is allowed."); - } - else - { - fallbackFunction = function; - if (_contract.isLibrary()) - m_errorReporter.typeError(fallbackFunction->location(), "Libraries cannot have fallback functions."); - if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) - m_errorReporter.typeError( - function->location(), - "Fallback function must be payable or non-payable, but is \"" + - stateMutabilityToString(function->stateMutability()) + - "\"." - ); - if (!fallbackFunction->parameters().empty()) - m_errorReporter.typeError(fallbackFunction->parameterList().location(), "Fallback function cannot take parameters."); - if (!fallbackFunction->returnParameters().empty()) - m_errorReporter.typeError(fallbackFunction->returnParameterList()->location(), "Fallback function cannot return values."); - if ( - _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) && - fallbackFunction->visibility() != FunctionDefinition::Visibility::External - ) - m_errorReporter.typeError(fallbackFunction->location(), "Fallback function must be defined as \"external\"."); - } + if (_contract.isLibrary()) + m_errorReporter.typeError(function->location(), "Libraries cannot have fallback functions."); + if (function->stateMutability() != StateMutability::NonPayable && function->stateMutability() != StateMutability::Payable) + m_errorReporter.typeError( + function->location(), + "Fallback function must be payable or non-payable, but is \"" + + stateMutabilityToString(function->stateMutability()) + + "\"." + ); + if (!function->parameters().empty()) + m_errorReporter.typeError(function->parameterList().location(), "Fallback function cannot take parameters."); + if (!function->returnParameters().empty()) + m_errorReporter.typeError(function->returnParameterList()->location(), "Fallback function cannot return values."); + if ( + _contract.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::V050) && + function->visibility() != FunctionDefinition::Visibility::External + ) + m_errorReporter.typeError(function->location(), "Fallback function must be defined as \"external\"."); } - } for (auto const& n: _contract.subNodes()) if (!visited.count(n.get())) @@ -172,25 +161,34 @@ void TypeChecker::checkContractDuplicateFunctions(ContractDefinition const& _con /// Checks that two functions with the same name defined in this contract have different /// argument types and that there is at most one constructor. map<string, vector<FunctionDefinition const*>> functions; + FunctionDefinition const* constructor = nullptr; + FunctionDefinition const* fallback = nullptr; for (FunctionDefinition const* function: _contract.definedFunctions()) - functions[function->name()].push_back(function); - - // Constructor - if (functions[_contract.name()].size() > 1) - { - SecondarySourceLocation ssl; - auto it = ++functions[_contract.name()].begin(); - for (; it != functions[_contract.name()].end(); ++it) - ssl.append("Another declaration is here:", (*it)->location()); - - string msg = "More than one constructor defined."; - ssl.limitSize(msg); - m_errorReporter.declarationError( - functions[_contract.name()].front()->location(), - ssl, - msg - ); - } + if (function->isConstructor()) + { + if (constructor) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", constructor->location()), + "More than one constructor defined." + ); + constructor = function; + } + else if (function->isFallback()) + { + if (fallback) + m_errorReporter.declarationError( + function->location(), + SecondarySourceLocation().append("Another declaration is here:", fallback->location()), + "Only one fallback function is allowed." + ); + fallback = function; + } + else + { + solAssert(!function->name().empty(), ""); + functions[function->name()].push_back(function); + } findDuplicateDefinitions(functions, "Function with same name and arguments defined twice."); } diff --git a/test/libsolidity/SolidityNameAndTypeResolution.cpp b/test/libsolidity/SolidityNameAndTypeResolution.cpp index 301250f0..7c0e8643 100644 --- a/test/libsolidity/SolidityNameAndTypeResolution.cpp +++ b/test/libsolidity/SolidityNameAndTypeResolution.cpp @@ -1140,7 +1140,6 @@ BOOST_AUTO_TEST_CASE(fallback_function_twice) } )"; CHECK_ERROR_ALLOW_MULTI(text, DeclarationError, (vector<string>{ - "Function with same name and arguments defined twice.", "Only one fallback function is" })); } diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol new file mode 100644 index 00000000..c757354e --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol @@ -0,0 +1,7 @@ +contract test { + function test(uint) public { } + constructor() public {} +} +// ---- +// Warning: (17-47): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. +// DeclarationError: (49-72): More than one constructor defined. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol new file mode 100644 index 00000000..42c0de28 --- /dev/null +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_new.sol @@ -0,0 +1,6 @@ +contract test { + constructor(uint) public { } + constructor() public {} +} +// ---- +// DeclarationError: (47-70): More than one constructor defined. diff --git a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol index 40341c4d..db632ced 100644 --- a/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol +++ b/test/libsolidity/syntaxTests/constructor/two_constructors_old.sol @@ -5,4 +5,4 @@ contract test { // ---- // Warning: (17-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. // Warning: (51-76): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead. -// DeclarationError: (17-49): More than one constructor defined. +// DeclarationError: (51-76): More than one constructor defined. |