From b0a5666b43fbc14a8b4898f385126ec8ca2e8b0a Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Mon, 12 Aug 2019 17:06:10 +0200 Subject: [PATCH 1/2] Better error messages when writing to expressions that cannot be written to. --- Changelog.md | 1 + libsolidity/analysis/TypeChecker.cpp | 65 ++++++++++++++++++- libsolidity/ast/AST.cpp | 9 ++- .../types/assign_calldata_value_type.sol | 9 +++ .../syntaxTests/array/calldata_assign.sol | 7 ++ .../syntaxTests/array/calldata_resize.sol | 7 ++ .../lvalues/calldata_index_access.sol | 7 ++ .../lvalues/calldata_member_access.sol | 10 +++ .../lvalues/external_reference_argument.sol | 7 ++ .../syntaxTests/lvalues/functions.sol | 15 +++++ .../syntaxTests/lvalues/library_mapping.sol | 7 ++ .../syntaxTests/lvalues/valid_lvalues.sol | 31 +++++++++ .../146_external_argument_assign.sol | 3 +- .../147_external_argument_increment.sol | 3 +- .../148_external_argument_delete.sol | 2 +- .../219_memory_arrays_not_resizeable.sol | 2 +- .../structs/calldata_array_assign.sol | 2 +- .../syntaxTests/structs/calldata_assign.sol | 2 +- .../structs/memory_to_calldata.sol | 4 +- .../types/bytesXX_index_assign.sol | 8 +++ 20 files changed, 187 insertions(+), 14 deletions(-) create mode 100644 test/libsolidity/semanticTests/types/assign_calldata_value_type.sol create mode 100644 test/libsolidity/syntaxTests/array/calldata_assign.sol create mode 100644 test/libsolidity/syntaxTests/array/calldata_resize.sol create mode 100644 test/libsolidity/syntaxTests/lvalues/calldata_index_access.sol create mode 100644 test/libsolidity/syntaxTests/lvalues/calldata_member_access.sol create mode 100644 test/libsolidity/syntaxTests/lvalues/external_reference_argument.sol create mode 100644 test/libsolidity/syntaxTests/lvalues/functions.sol create mode 100644 test/libsolidity/syntaxTests/lvalues/library_mapping.sol create mode 100644 test/libsolidity/syntaxTests/lvalues/valid_lvalues.sol create mode 100644 test/libsolidity/syntaxTests/types/bytesXX_index_assign.sol diff --git a/Changelog.md b/Changelog.md index d0234a6f6..14f6652f6 100644 --- a/Changelog.md +++ b/Changelog.md @@ -1,6 +1,7 @@ ### 0.5.12 (unreleased) Language Features: + * Type Checker: Allow assignment to external function arguments except for reference types. Compiler Features: diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index c1d42409d..4e1676006 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -2515,8 +2515,69 @@ void TypeChecker::requireLValue(Expression const& _expression) _expression.annotation().lValueRequested = true; _expression.accept(*this); + if (_expression.annotation().isLValue) + return; + if (_expression.annotation().isConstant) + { m_errorReporter.typeError(_expression.location(), "Cannot assign to a constant variable."); - else if (!_expression.annotation().isLValue) - m_errorReporter.typeError(_expression.location(), "Expression has to be an lvalue."); + return; + } + + if (auto indexAccess = dynamic_cast(&_expression)) + { + if (auto arrayType = dynamic_cast(type(indexAccess->baseExpression()))) + { + if (arrayType->dataStoredIn(DataLocation::CallData)) + { + m_errorReporter.typeError(_expression.location(), "Calldata arrays are read-only."); + return; + } + } + else if (type(indexAccess->baseExpression())->category() == Type::Category::FixedBytes) + { + m_errorReporter.typeError(_expression.location(), "Single bytes in fixed bytes arrays cannot be modified."); + return; + } + } + + if (auto memberAccess = dynamic_cast(&_expression)) + { + if (auto structType = dynamic_cast(type(memberAccess->expression()))) + { + if (structType->dataStoredIn(DataLocation::CallData)) + { + m_errorReporter.typeError(_expression.location(), "Calldata structs are read-only."); + return; + } + } + else if (auto arrayType = dynamic_cast(type(memberAccess->expression()))) + { + if (memberAccess->memberName() == "length") + { + switch (arrayType->location()) + { + case DataLocation::Memory: + m_errorReporter.typeError(_expression.location(), "Memory arrays cannot be resized."); + return; + break; + case DataLocation::CallData: + m_errorReporter.typeError(_expression.location(), "Calldata arrays cannot be resized."); + return; + case DataLocation::Storage: + break; + } + } + } + } + + if (auto identifier = dynamic_cast(&_expression)) + if (auto varDecl = dynamic_cast(identifier->annotation().referencedDeclaration)) + if (varDecl->isExternalCallableParameter() && dynamic_cast(identifier->annotation().type)) + { + m_errorReporter.typeError(_expression.location(), "External function arguments of reference type are read-only."); + return; + } + + m_errorReporter.typeError(_expression.location(), "Expression has to be an lvalue."); } diff --git a/libsolidity/ast/AST.cpp b/libsolidity/ast/AST.cpp index 75166d654..60703f76e 100644 --- a/libsolidity/ast/AST.cpp +++ b/libsolidity/ast/AST.cpp @@ -432,8 +432,13 @@ string Scopable::sourceUnitName() const bool VariableDeclaration::isLValue() const { - // External function parameters and constant declared variables are Read-Only - return !isExternalCallableParameter() && !m_isConstant; + // Constant declared variables are Read-Only + if (m_isConstant) + return false; + // External function arguments of reference type are Read-Only + if (isExternalCallableParameter() && dynamic_cast(type())) + return false; + return true; } bool VariableDeclaration::isLocalVariable() const diff --git a/test/libsolidity/semanticTests/types/assign_calldata_value_type.sol b/test/libsolidity/semanticTests/types/assign_calldata_value_type.sol new file mode 100644 index 000000000..8350628e4 --- /dev/null +++ b/test/libsolidity/semanticTests/types/assign_calldata_value_type.sol @@ -0,0 +1,9 @@ +contract C { + function f(uint256 x) public pure returns (uint256, uint256) { + uint256 b = x; + x = 42; + return (x, b); + } +} +// ---- +// f(uint256): 23 -> 42, 23 diff --git a/test/libsolidity/syntaxTests/array/calldata_assign.sol b/test/libsolidity/syntaxTests/array/calldata_assign.sol new file mode 100644 index 000000000..5d55dd163 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/calldata_assign.sol @@ -0,0 +1,7 @@ +pragma experimental ABIEncoderV2; +contract Test { + function f(uint256[] calldata s) external { s[0] = 4; } +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. +// TypeError: (98-102): Calldata arrays are read-only. diff --git a/test/libsolidity/syntaxTests/array/calldata_resize.sol b/test/libsolidity/syntaxTests/array/calldata_resize.sol new file mode 100644 index 000000000..f923993c2 --- /dev/null +++ b/test/libsolidity/syntaxTests/array/calldata_resize.sol @@ -0,0 +1,7 @@ +contract C { + function f (uint256[] calldata x) external pure { + x.length = 42; + } +} +// ---- +// TypeError: (75-83): Calldata arrays cannot be resized. diff --git a/test/libsolidity/syntaxTests/lvalues/calldata_index_access.sol b/test/libsolidity/syntaxTests/lvalues/calldata_index_access.sol new file mode 100644 index 000000000..e58d86041 --- /dev/null +++ b/test/libsolidity/syntaxTests/lvalues/calldata_index_access.sol @@ -0,0 +1,7 @@ +contract C { + function f(uint256[] calldata x) external pure { + x[0] = 42; + } +} +// ---- +// TypeError: (74-78): Calldata arrays are read-only. diff --git a/test/libsolidity/syntaxTests/lvalues/calldata_member_access.sol b/test/libsolidity/syntaxTests/lvalues/calldata_member_access.sol new file mode 100644 index 000000000..35655a4ac --- /dev/null +++ b/test/libsolidity/syntaxTests/lvalues/calldata_member_access.sol @@ -0,0 +1,10 @@ +pragma experimental ABIEncoderV2; +contract C { + struct S { uint256 x; } + function f(S calldata s) external pure { + s.x = 42; + } +} +// ---- +// Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. +// TypeError: (128-131): Calldata structs are read-only. diff --git a/test/libsolidity/syntaxTests/lvalues/external_reference_argument.sol b/test/libsolidity/syntaxTests/lvalues/external_reference_argument.sol new file mode 100644 index 000000000..16a37911a --- /dev/null +++ b/test/libsolidity/syntaxTests/lvalues/external_reference_argument.sol @@ -0,0 +1,7 @@ +contract C { + function f(uint256[] calldata x, uint256[] calldata y) external pure { + x = y; + } +} +// ---- +// TypeError: (96-97): External function arguments of reference type are read-only. diff --git a/test/libsolidity/syntaxTests/lvalues/functions.sol b/test/libsolidity/syntaxTests/lvalues/functions.sol new file mode 100644 index 000000000..6aa73e0f2 --- /dev/null +++ b/test/libsolidity/syntaxTests/lvalues/functions.sol @@ -0,0 +1,15 @@ +contract C { + function f() internal { + } + function g() internal { + g = f; + } + function h() external { + } + function i() external { + this.i = this.h; + } +} +// ---- +// TypeError: (83-84): Expression has to be an lvalue. +// TypeError: (166-172): Expression has to be an lvalue. diff --git a/test/libsolidity/syntaxTests/lvalues/library_mapping.sol b/test/libsolidity/syntaxTests/lvalues/library_mapping.sol new file mode 100644 index 000000000..9aa4099c9 --- /dev/null +++ b/test/libsolidity/syntaxTests/lvalues/library_mapping.sol @@ -0,0 +1,7 @@ +library L { + function f(mapping(uint=>uint) storage x, mapping(uint=>uint) storage y) external { + x = y; + } +} +// ---- +// TypeError: (108-109): Mappings cannot be assigned to. diff --git a/test/libsolidity/syntaxTests/lvalues/valid_lvalues.sol b/test/libsolidity/syntaxTests/lvalues/valid_lvalues.sol new file mode 100644 index 000000000..aaf94be33 --- /dev/null +++ b/test/libsolidity/syntaxTests/lvalues/valid_lvalues.sol @@ -0,0 +1,31 @@ +contract C { + struct S { uint256 x; } + function i() internal pure {} + function e() external pure {} + uint[] s1; + function f(uint x, bytes32 y) external { + x = 42; + y = bytes32(0); + (x, y) = (23, bytes32(0)); + S memory ms1; + S memory ms2; + ms1 = ms2; + ms1.x = x; + uint256[] memory a = new uint256[](2); + uint256[] memory b = new uint256[](3); + a = b; + a[0] = x; + s1[0] = x; + s1 = a; + } + function g(function() internal pure x) internal view { + x = i; + function(uint, bytes32) external y; + y = this.f; + } + function g(function() external pure x) external view { + x = this.e; + function(function() internal pure) internal view y; + y = g; + } +} diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/146_external_argument_assign.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/146_external_argument_assign.sol index d2c0245cf..8d01ab63a 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/146_external_argument_assign.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/146_external_argument_assign.sol @@ -1,5 +1,4 @@ contract c { - function f(uint a) external { a = 1; } + function f(uint a) external pure { a = 1; } } // ---- -// TypeError: (47-48): Expression has to be an lvalue. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/147_external_argument_increment.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/147_external_argument_increment.sol index 2bfba42bb..c80dbcd2a 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/147_external_argument_increment.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/147_external_argument_increment.sol @@ -1,5 +1,4 @@ contract c { - function f(uint a) external { a++; } + function f(uint a) external pure { a++; } } // ---- -// TypeError: (47-48): Expression has to be an lvalue. diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/148_external_argument_delete.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/148_external_argument_delete.sol index 30eb204e1..3955023ea 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/148_external_argument_delete.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/148_external_argument_delete.sol @@ -2,4 +2,4 @@ contract c { function f(uint a) external { delete a; } } // ---- -// TypeError: (54-55): Expression has to be an lvalue. +// Warning: (17-58): Function state mutability can be restricted to pure diff --git a/test/libsolidity/syntaxTests/nameAndTypeResolution/219_memory_arrays_not_resizeable.sol b/test/libsolidity/syntaxTests/nameAndTypeResolution/219_memory_arrays_not_resizeable.sol index 93d8cd429..aba0fff48 100644 --- a/test/libsolidity/syntaxTests/nameAndTypeResolution/219_memory_arrays_not_resizeable.sol +++ b/test/libsolidity/syntaxTests/nameAndTypeResolution/219_memory_arrays_not_resizeable.sol @@ -5,4 +5,4 @@ contract C { } } // ---- -// TypeError: (72-80): Expression has to be an lvalue. +// TypeError: (72-80): Memory arrays cannot be resized. diff --git a/test/libsolidity/syntaxTests/structs/calldata_array_assign.sol b/test/libsolidity/syntaxTests/structs/calldata_array_assign.sol index 70e537ad6..eb8de4963 100644 --- a/test/libsolidity/syntaxTests/structs/calldata_array_assign.sol +++ b/test/libsolidity/syntaxTests/structs/calldata_array_assign.sol @@ -7,4 +7,4 @@ contract Test { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (144-147): Expression has to be an lvalue. +// TypeError: (144-147): Calldata structs are read-only. diff --git a/test/libsolidity/syntaxTests/structs/calldata_assign.sol b/test/libsolidity/syntaxTests/structs/calldata_assign.sol index f44730b4f..728fe8705 100644 --- a/test/libsolidity/syntaxTests/structs/calldata_assign.sol +++ b/test/libsolidity/syntaxTests/structs/calldata_assign.sol @@ -5,4 +5,4 @@ contract Test { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (114-117): Expression has to be an lvalue. +// TypeError: (114-117): Calldata structs are read-only. diff --git a/test/libsolidity/syntaxTests/structs/memory_to_calldata.sol b/test/libsolidity/syntaxTests/structs/memory_to_calldata.sol index 09b3ef294..2694b9ee6 100644 --- a/test/libsolidity/syntaxTests/structs/memory_to_calldata.sol +++ b/test/libsolidity/syntaxTests/structs/memory_to_calldata.sol @@ -6,7 +6,7 @@ contract Test { } // ---- // Warning: (0-33): Experimental features are turned on. Do not use experimental features on live deployments. -// TypeError: (114-115): Expression has to be an lvalue. +// TypeError: (114-115): External function arguments of reference type are read-only. // TypeError: (118-122): Type struct Test.S memory is not implicitly convertible to expected type struct Test.S calldata. -// TypeError: (178-179): Expression has to be an lvalue. +// TypeError: (178-179): External function arguments of reference type are read-only. // TypeError: (182-183): Type struct Test.S memory is not implicitly convertible to expected type struct Test.S calldata. diff --git a/test/libsolidity/syntaxTests/types/bytesXX_index_assign.sol b/test/libsolidity/syntaxTests/types/bytesXX_index_assign.sol new file mode 100644 index 000000000..8c55a8038 --- /dev/null +++ b/test/libsolidity/syntaxTests/types/bytesXX_index_assign.sol @@ -0,0 +1,8 @@ +contract C { + function f() public pure { + bytes32 x; + x[0] = 0x42; + } +} +// ---- +// TypeError: (71-75): Single bytes in fixed bytes arrays cannot be modified. From e545103ec87c6a0c9e6c0d50392df23a2f096119 Mon Sep 17 00:00:00 2001 From: Daniel Kirchner Date: Tue, 13 Aug 2019 18:58:50 +0200 Subject: [PATCH 2/2] Extract reasonOfFailure to lambda function. --- libsolidity/analysis/TypeChecker.cpp | 82 ++++++++++------------------ 1 file changed, 30 insertions(+), 52 deletions(-) diff --git a/libsolidity/analysis/TypeChecker.cpp b/libsolidity/analysis/TypeChecker.cpp index 4e1676006..b5abe59f4 100644 --- a/libsolidity/analysis/TypeChecker.cpp +++ b/libsolidity/analysis/TypeChecker.cpp @@ -2518,66 +2518,44 @@ void TypeChecker::requireLValue(Expression const& _expression) if (_expression.annotation().isLValue) return; - if (_expression.annotation().isConstant) - { - m_errorReporter.typeError(_expression.location(), "Cannot assign to a constant variable."); - return; - } + return m_errorReporter.typeError(_expression.location(), [&]() { + if (_expression.annotation().isConstant) + return "Cannot assign to a constant variable."; - if (auto indexAccess = dynamic_cast(&_expression)) - { - if (auto arrayType = dynamic_cast(type(indexAccess->baseExpression()))) + if (auto indexAccess = dynamic_cast(&_expression)) { - if (arrayType->dataStoredIn(DataLocation::CallData)) - { - m_errorReporter.typeError(_expression.location(), "Calldata arrays are read-only."); - return; - } + if (type(indexAccess->baseExpression())->category() == Type::Category::FixedBytes) + return "Single bytes in fixed bytes arrays cannot be modified."; + else if (auto arrayType = dynamic_cast(type(indexAccess->baseExpression()))) + if (arrayType->dataStoredIn(DataLocation::CallData)) + return "Calldata arrays are read-only."; } - else if (type(indexAccess->baseExpression())->category() == Type::Category::FixedBytes) - { - m_errorReporter.typeError(_expression.location(), "Single bytes in fixed bytes arrays cannot be modified."); - return; - } - } - if (auto memberAccess = dynamic_cast(&_expression)) - { - if (auto structType = dynamic_cast(type(memberAccess->expression()))) + if (auto memberAccess = dynamic_cast(&_expression)) { - if (structType->dataStoredIn(DataLocation::CallData)) + if (auto structType = dynamic_cast(type(memberAccess->expression()))) { - m_errorReporter.typeError(_expression.location(), "Calldata structs are read-only."); - return; + if (structType->dataStoredIn(DataLocation::CallData)) + return "Calldata structs are read-only."; } + else if (auto arrayType = dynamic_cast(type(memberAccess->expression()))) + if (memberAccess->memberName() == "length") + switch (arrayType->location()) + { + case DataLocation::Memory: + return "Memory arrays cannot be resized."; + case DataLocation::CallData: + return "Calldata arrays cannot be resized."; + case DataLocation::Storage: + break; + } } - else if (auto arrayType = dynamic_cast(type(memberAccess->expression()))) - { - if (memberAccess->memberName() == "length") - { - switch (arrayType->location()) - { - case DataLocation::Memory: - m_errorReporter.typeError(_expression.location(), "Memory arrays cannot be resized."); - return; - break; - case DataLocation::CallData: - m_errorReporter.typeError(_expression.location(), "Calldata arrays cannot be resized."); - return; - case DataLocation::Storage: - break; - } - } - } - } - if (auto identifier = dynamic_cast(&_expression)) - if (auto varDecl = dynamic_cast(identifier->annotation().referencedDeclaration)) - if (varDecl->isExternalCallableParameter() && dynamic_cast(identifier->annotation().type)) - { - m_errorReporter.typeError(_expression.location(), "External function arguments of reference type are read-only."); - return; - } + if (auto identifier = dynamic_cast(&_expression)) + if (auto varDecl = dynamic_cast(identifier->annotation().referencedDeclaration)) + if (varDecl->isExternalCallableParameter() && dynamic_cast(identifier->annotation().type)) + return "External function arguments of reference type are read-only."; - m_errorReporter.typeError(_expression.location(), "Expression has to be an lvalue."); + return "Expression has to be an lvalue."; + }()); }