cerc-io/solidity
16
0
Fork 0
mirror of https://github.com/ethereum/solidity synced 2023-10-03 13:03:40 +00:00
Code Issues Actions Packages Projects Releases Wiki Activity

Merge pull request #3912 from ethereum/fixFallbackConstructor

Browse Source 
Fix fallback constructor
This commit is contained in:
chriseth 2018-04-18 15:00:35 +02:00 committed by GitHub
commit 85687a37eb
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
30 changed files with 257 additions and 231 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
Changelog.md
View File

@ -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)

92
libsolidity/analysis/TypeChecker.cpp
View File

@ -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.");
}

183
test/libsolidity/SolidityNameAndTypeResolution.cpp
View File

@ -243,16 +243,6 @@ BOOST_AUTO_TEST_CASE(assignment_to_struct)
CHECK_SUCCESS(text);
}
BOOST_AUTO_TEST_CASE(returns_in_constructor)
{
char const* text = R"(
contract test {
function test() public returns (uint a) { }
}
)";
CHECK_ERROR(text, TypeError, "Non-empty \"returns\" directive for constructor.");
}
BOOST_AUTO_TEST_CASE(forward_function_reference)
{
char const* text = R"(
@ -869,26 +859,6 @@ BOOST_AUTO_TEST_CASE(complex_inheritance)
CHECK_SUCCESS(text);
}
BOOST_AUTO_TEST_CASE(constructor_visibility)
{
// The constructor of a base class should not be visible in the derived class
char const* text = R"(
contract A { function A() public { } }
contract B is A { function f() public { A x = A(0); } }
)";
CHECK_SUCCESS(text);
}
BOOST_AUTO_TEST_CASE(overriding_constructor)
{
// It is fine to "override" constructor of a base class since it is invisible
char const* text = R"(
contract A { function A() public { } }
contract B is A { function A() public returns (uint8 r) {} }
)";
CHECK_SUCCESS(text);
}
BOOST_AUTO_TEST_CASE(missing_base_constructor_arguments)
{
char const* text = R"(
@ -907,35 +877,6 @@ 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_NO_WARNINGS(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"(
@ -1199,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"
}));
}
@ -2588,17 +2528,6 @@ BOOST_AUTO_TEST_CASE(override_changes_return_types)
CHECK_ERROR(sourceCode, TypeError, "Overriding function return types differ");
}
BOOST_AUTO_TEST_CASE(multiple_constructors)
{
char const* sourceCode = R"(
contract test {
function test(uint a) public { }
function test() public {}
}
)";
CHECK_ERROR(sourceCode, DeclarationError, "More than one constructor defined");
}
BOOST_AUTO_TEST_CASE(equal_overload)
{
char const* sourceCode = R"(
@ -3122,19 +3051,6 @@ BOOST_AUTO_TEST_CASE(library_having_variables)
CHECK_ERROR(text, TypeError, "Library cannot have non-constant state variables");
}
BOOST_AUTO_TEST_CASE(library_constructor)
{
char const* text = R"(
library Lib {
function Lib();
}
)";
CHECK_ERROR_ALLOW_MULTI(text, TypeError, (vector<std::string>{
"Constructor cannot be defined in libraries.",
"Constructor must be implemented if declared."
}));
}
BOOST_AUTO_TEST_CASE(valid_library)
{
char const* text = R"(
@ -5030,38 +4946,6 @@ BOOST_AUTO_TEST_CASE(unsatisfied_version)
BOOST_CHECK(searchErrorMessage(*sourceAndError.second.front(), "Source file requires different compiler version"));
}
BOOST_AUTO_TEST_CASE(invalid_constructor_statemutability)
{
char const* text = R"(
contract test {
function test() constant {}
}
)";
CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable");
text = R"(
contract test {
function test() view {}
}
)";
CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable");
text = R"(
contract test {
function test() pure {}
}
)";
CHECK_ERROR(text, TypeError, "Constructor must be payable or non-payable");
}
BOOST_AUTO_TEST_CASE(external_constructor)
{
char const* text = R"(
contract test {
function test() external {}
}
)";
CHECK_ERROR(text, TypeError, "Constructor must be public or internal.");
}
BOOST_AUTO_TEST_CASE(invalid_array_as_statement)
{
char const* text = R"(
@ -5608,50 +5492,6 @@ BOOST_AUTO_TEST_CASE(assignment_to_constant)
CHECK_ERROR(text, TypeError, "Cannot assign to a constant variable.");
}
BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor)
{
char const* text = R"(
contract C {
function C() internal {}
}
contract D {
function f() public { var x = new C(); }
}
)";
CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly.");
}
BOOST_AUTO_TEST_CASE(inconstructible_internal_constructor_inverted)
{
// Previously, the type information for A was not yet available at the point of
// "new A".
char const* text = R"(
contract B {
A a;
function B() public {
a = new A(this);
}
}
contract A {
function A(address a) internal {}
}
)";
CHECK_ERROR(text, TypeError, "Contract with internal constructor cannot be created directly.");
}
BOOST_AUTO_TEST_CASE(constructible_internal_constructor)
{
char const* text = R"(
contract C {
function C() internal {}
}
contract D is C {
function D() public { }
}
)";
CHECK_SUCCESS(text);
}
BOOST_AUTO_TEST_CASE(return_structs)
{
char const* text = R"(
@ -5813,19 +5653,6 @@ BOOST_AUTO_TEST_CASE(interface)
CHECK_SUCCESS(text);
}
BOOST_AUTO_TEST_CASE(interface_constructor)
{
char const* text = R"(
interface I {
function I();
}
)";
CHECK_ERROR_ALLOW_MULTI(text, TypeError, (std::vector<std::string>{
"Constructor cannot be defined in interfaces",
"Constructor must be implemented if declared.",
}));
}
BOOST_AUTO_TEST_CASE(interface_functions)
{
char const* text = R"(
@ -6907,16 +6734,6 @@ BOOST_AUTO_TEST_CASE(builtin_reject_value)
CHECK_ERROR(text, TypeError, "Member \"value\" not found or not visible after argument-dependent lookup");
}
BOOST_AUTO_TEST_CASE(constructor_without_implementation)
{
char const* text = R"(
contract C {
function C();
}
)";
CHECK_ERROR(text, TypeError, "Constructor must be implemented if declared.");
}
BOOST_AUTO_TEST_CASE(large_storage_array_fine)
{
char const* text = R"(

6
test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_new.sol Normal file
View File

@ -0,0 +1,6 @@
contract C {
constructor() internal {}
}
contract D is C {
constructor() public { }
}

9
test/libsolidity/syntaxTests/constructor/constructible_internal_constructor_old.sol Normal file
View File

@ -0,0 +1,9 @@
contract C {
function C() internal {}
}
contract D is C {
function D() public {}
}
// ----
// Warning: (14-38): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// Warning: (60-82): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.

1
test/libsolidity/syntaxTests/constructor/constructor_new.sol Normal file
View File

@ -0,0 +1 @@
contract A { constructor() public {} }

3
test/libsolidity/syntaxTests/constructor/constructor_old.sol Normal file
View File

@ -0,0 +1,3 @@
contract A { function A() public {} }
// ----
// Warning: (13-35): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.

4
test/libsolidity/syntaxTests/constructor/constructor_old_050.sol Normal file
View File

@ -0,0 +1,4 @@
pragma experimental "v0.5.0";
contract A { function A() public {} }
// ----
// SyntaxError: (43-65): 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.

13
test/libsolidity/syntaxTests/constructor/constructor_state_mutability_new.sol Normal file
View File

@ -0,0 +1,13 @@
contract test1 {
constructor() constant {}
}
contract test2 {
constructor() view {}
}
contract test3 {
constructor() pure {}
}
// ----
// TypeError: (19-44): Constructor must be payable or non-payable, but is "view".
// TypeError: (66-87): Constructor must be payable or non-payable, but is "view".
// TypeError: (109-130): Constructor must be payable or non-payable, but is "pure".

16
test/libsolidity/syntaxTests/constructor/constructor_state_mutability_old.sol Normal file
View File

@ -0,0 +1,16 @@
contract test1 {
function test1() constant {}
}
contract test2 {
function test2() view {}
}
contract test3 {
function test3() pure {}
}
// ----
// Warning: (21-49): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// Warning: (73-97): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// Warning: (121-145): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (21-49): Constructor must be payable or non-payable, but is "view".
// TypeError: (73-97): Constructor must be payable or non-payable, but is "view".
// TypeError: (121-145): Constructor must be payable or non-payable, but is "pure".

12
test/libsolidity/syntaxTests/constructor/constructor_visibility_new.sol Normal file
View File

@ -0,0 +1,12 @@
// The constructor of a base class should not be visible in the derived class
contract A { constructor(string) public { } }
contract B is A {
function f() pure public {
A x = A(0); // convert from address
string memory y = "ab";
A(y); // call as a function is invalid
x;
}
}
// ----
// TypeError: (243-247): Explicit type conversion not allowed from "string memory" to "contract A".

13
test/libsolidity/syntaxTests/constructor/constructor_visibility_old.sol Normal file
View File

@ -0,0 +1,13 @@
// The constructor of a base class should not be visible in the derived class
contract A { function A(string s) public { } }
contract B is A {
function f() pure public {
A x = A(0); // convert from address
string memory y = "ab";
A(y); // call as a function is invalid
x;
}
}
// ----
// Warning: (91-122): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (244-248): Explicit type conversion not allowed from "string memory" to "contract A".

5
test/libsolidity/syntaxTests/constructor/constructor_without_implementation_new.sol Normal file
View File

@ -0,0 +1,5 @@
contract C {
constructor();
}
// ----
// TypeError: (14-28): Constructor must be implemented if declared.

6
test/libsolidity/syntaxTests/constructor/constructor_without_implementation_old.sol Normal file
View File

@ -0,0 +1,6 @@
contract C {
function C();
}
// ----
// Warning: (14-27): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (14-27): Constructor must be implemented if declared.

5
test/libsolidity/syntaxTests/constructor/external_constructor_new.sol Normal file
View File

@ -0,0 +1,5 @@
contract test {
constructor() external {}
}
// ----
// TypeError: (17-42): Constructor must be public or internal.

6
test/libsolidity/syntaxTests/constructor/external_constructor_old.sol Normal file
View File

@ -0,0 +1,6 @@
contract test {
function test() external {}
}
// ----
// Warning: (17-44): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (17-44): Constructor must be public or internal.

13
test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_new.sol Normal file
View File

@ -0,0 +1,13 @@
// Previously, the type information for A was not yet available at the point of
// "new A".
contract B {
A a;
constructor() public {
a = new A(this);
}
}
contract A {
constructor(address a) internal {}
}
// ----
// TypeError: (141-146): Contract with internal constructor cannot be created directly.

15
test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_inverted_old.sol Normal file
View File

@ -0,0 +1,15 @@
// Previously, the type information for A was not yet available at the point of
// "new A".
contract B {
A a;
function B() public {
a = new A(this);
}
}
contract A {
function A(address a) internal {}
}
// ----
// Warning: (112-155): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// Warning: (172-205): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (140-145): Contract with internal constructor cannot be created directly.

8
test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_new.sol Normal file
View File

@ -0,0 +1,8 @@
contract C {
constructor() internal {}
}
contract D {
function f() public { C c = new C(); c; }
}
// ----
// TypeError: (84-89): Contract with internal constructor cannot be created directly.

9
test/libsolidity/syntaxTests/constructor/inconstructible_internal_constructor_old.sol Normal file
View File

@ -0,0 +1,9 @@
contract C {
function C() internal {}
}
contract D {
function f() public { C x = new C(); x; }
}
// ----
// Warning: (14-38): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (83-88): Contract with internal constructor cannot be created directly.

7
test/libsolidity/syntaxTests/constructor/interface_constructor_new.sol Normal file
View File

@ -0,0 +1,7 @@
interface I {
constructor();
}
// ----
// Warning: (15-29): Functions in interfaces should be declared external.
// TypeError: (15-29): Constructor cannot be defined in interfaces.
// TypeError: (15-29): Constructor must be implemented if declared.

8
test/libsolidity/syntaxTests/constructor/interface_constructor_old.sol Normal file
View File

@ -0,0 +1,8 @@
interface I {
function I();
}
// ----
// Warning: (15-28): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// Warning: (15-28): Functions in interfaces should be declared external.
// TypeError: (15-28): Constructor cannot be defined in interfaces.
// TypeError: (15-28): Constructor must be implemented if declared.

6
test/libsolidity/syntaxTests/constructor/library_constructor_new.sol Normal file
View File

@ -0,0 +1,6 @@
library Lib {
constructor();
}
// ----
// TypeError: (15-29): Constructor cannot be defined in libraries.
// TypeError: (15-29): Constructor must be implemented if declared.

7
test/libsolidity/syntaxTests/constructor/library_constructor_old.sol Normal file
View File

@ -0,0 +1,7 @@
library Lib {
function Lib();
}
// ----
// Warning: (15-30): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (15-30): Constructor cannot be defined in libraries.
// TypeError: (15-30): Constructor must be implemented if declared.

6
test/libsolidity/syntaxTests/constructor/overriding_constructor.sol Normal file
View File

@ -0,0 +1,6 @@
// It is fine to "override" constructor of a base class since it is invisible
contract A { function A() public { } }
contract B is A { function A() public pure returns (uint8) {} }
// ----
// Warning: (91-114): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// Warning: (135-178): This declaration shadows an existing declaration.

5
test/libsolidity/syntaxTests/constructor/returns_in_constructor_new.sol Normal file
View File

@ -0,0 +1,5 @@
contract test {
constructor() public returns (uint a) { }
}
// ----
// TypeError: (46-54): Non-empty "returns" directive for constructor.

6
test/libsolidity/syntaxTests/constructor/returns_in_constructor_old.sol Normal file
View File

@ -0,0 +1,6 @@
contract test {
function test() public returns (uint a) { }
}
// ----
// Warning: (17-60): Defining constructors as functions with the same name as the contract is deprecated. Use "constructor(...) { ... }" instead.
// TypeError: (48-56): Non-empty "returns" directive for constructor.

7
test/libsolidity/syntaxTests/constructor/two_constructors_mixed.sol Normal file
View File

@ -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.

6
test/libsolidity/syntaxTests/constructor/two_constructors_new.sol Normal file
View File

@ -0,0 +1,6 @@
contract test {
constructor(uint) public { }
constructor() public {}
}
// ----
// DeclarationError: (47-70): More than one constructor defined.

8
test/libsolidity/syntaxTests/constructor/two_constructors_old.sol Normal file
View File

@ -0,0 +1,8 @@
contract test {
function test(uint a) public { }
function test() public {}
}
// ----
// 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: (51-76): More than one constructor defined.