Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Allow mapping arguments for public and external library functions. #5382

Merged
merged 1 commit into from
Nov 26, 2018
Merged

Allow mapping arguments for public and external library functions. #5382

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Changelog.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
### 0.5.1 (unreleased)

Language Features:
* Allow mapping type for parameters and return variables of public and external library functions.
* Allow public functions to override external functions.


Expand Down
28 changes: 15 additions & 13 deletions libsolidity/analysis/TypeChecker.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,20 +695,22 @@ bool TypeChecker::visit(FunctionDefinition const& _function)
}
for (ASTPointer<VariableDeclaration> const& var: _function.parameters() + _function.returnParameters())
{
if (type(*var)->category() == Type::Category::Mapping)
{
if (!type(*var)->dataStoredIn(DataLocation::Storage))
m_errorReporter.typeError(var->location(), "Mapping types can only have a data location of \"storage\"." );
else if (!isLibraryFunction && _function.isPublic())
m_errorReporter.typeError(var->location(), "Mapping types for parameters or return variables can only be used in internal or library functions.");
}
else
{
if (!type(*var)->canLiveOutsideStorage() && _function.isPublic())
m_errorReporter.typeError(var->location(), "Type is required to live outside storage.");
ekpyron marked this conversation as resolved.
Show resolved Hide resolved
if (_function.isPublic() && !(type(*var)->interfaceType(isLibraryFunction)))
m_errorReporter.fatalTypeError(var->location(), "Internal or recursive type is not allowed for public or external functions.");
}
if (
type(*var)->category() == Type::Category::Mapping &&
!type(*var)->dataStoredIn(DataLocation::Storage)
)
m_errorReporter.typeError(var->location(), "Mapping types can only have a data location of \"storage\".");
else if (
!type(*var)->canLiveOutsideStorage() &&
_function.visibility() > FunctionDefinition::Visibility::Internal
)
m_errorReporter.typeError(var->location(), "Type is required to live outside storage.");
if (_function.visibility() >= FunctionDefinition::Visibility::Public && !(type(*var)->interfaceType(isLibraryFunction)))
m_errorReporter.fatalTypeError(var->location(), "Internal or recursive type is not allowed for public or external functions.");
if (
_function.visibility() > FunctionDefinition::Visibility::Internal &&
_function.isPublic() &&
!_function.sourceUnit().annotation().experimentalFeatures.count(ExperimentalFeature::ABIEncoderV2) &&
!typeSupportedByOldABIEncoder(*type(*var))
)
Expand Down
2 changes: 2 additions & 0 deletions libsolidity/codegen/ExpressionCompiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1873,6 +1873,8 @@ void ExpressionCompiler::appendExternalFunctionCall(
retSize = 0;
break;
chriseth marked this conversation as resolved.
Show resolved Hide resolved
}
else if (retType->decodingType())
retSize += retType->decodingType()->calldataEncodedSize();
else
retSize += retType->calldataEncodedSize();
}
Expand Down
84 changes: 84 additions & 0 deletions test/libsolidity/SolidityEndToEndTest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8658,6 +8658,90 @@ BOOST_AUTO_TEST_CASE(mapping_returns_in_library_named)
ABI_CHECK(callContractFunction("g()"), encodeArgs(u256(0), u256(42), u256(0), u256(0), u256(21), u256(17)));
}

BOOST_AUTO_TEST_CASE(using_library_mappings_public)
{
char const* sourceCode = R"(
library Lib {
function set(mapping(uint => uint) storage m, uint key, uint value) public
{
m[key] = value;
}
}
contract Test {
mapping(uint => uint) m1;
mapping(uint => uint) m2;
function f() public returns (uint, uint, uint, uint, uint, uint)
{
Lib.set(m1, 0, 1);
Lib.set(m1, 2, 42);
Lib.set(m2, 0, 23);
Lib.set(m2, 2, 99);
return (m1[0], m1[1], m1[2], m2[0], m2[1], m2[2]);
}
}
)";
compileAndRun(sourceCode, 0, "Lib");
compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(1), u256(0), u256(42), u256(23), u256(0), u256(99)));
}

BOOST_AUTO_TEST_CASE(using_library_mappings_external)
{
char const* libSourceCode = R"(
library Lib {
function set(mapping(uint => uint) storage m, uint key, uint value) external
{
m[key] = value * 2;
}
}
)";
char const* sourceCode = R"(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation seems to be off in tests, but that might be only on Github.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is just how github displays tabs.

library Lib {
function set(mapping(uint => uint) storage m, uint key, uint value) external;
}
contract Test {
mapping(uint => uint) m1;
mapping(uint => uint) m2;
function f() public returns (uint, uint, uint, uint, uint, uint)
{
Lib.set(m1, 0, 1);
Lib.set(m1, 2, 42);
Lib.set(m2, 0, 23);
Lib.set(m2, 2, 99);
return (m1[0], m1[1], m1[2], m2[0], m2[1], m2[2]);
}
}
)";
compileAndRun(libSourceCode, 0, "Lib");
compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(2), u256(0), u256(84), u256(46), u256(0), u256(198)));
}

BOOST_AUTO_TEST_CASE(using_library_mappings_return)
{
char const* sourceCode = R"(
library Lib {
function choose(mapping(uint => mapping(uint => uint)) storage m, uint key) external returns (mapping(uint => uint) storage) {
return m[key];
}
}
contract Test {
mapping(uint => mapping(uint => uint)) m;
function f() public returns (uint, uint, uint, uint, uint, uint)
{
Lib.choose(m, 0)[0] = 1;
Lib.choose(m, 0)[2] = 42;
Lib.choose(m, 1)[0] = 23;
Lib.choose(m, 1)[2] = 99;
return (m[0][0], m[0][1], m[0][2], m[1][0], m[1][1], m[1][2]);
}
}
)";
compileAndRun(sourceCode, 0, "Lib");
compileAndRun(sourceCode, 0, "Test", bytes(), map<string, Address>{{"Lib", m_contractAddress}});
ABI_CHECK(callContractFunction("f()"), encodeArgs(u256(1), u256(0), u256(42), u256(23), u256(0), u256(99)));
}

BOOST_AUTO_TEST_CASE(using_library_structs)
{
char const* sourceCode = R"(
Expand Down
1 change: 0 additions & 1 deletion test/libsolidity/syntaxTests/types/mapping/library_argument_external.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@ library L {
}
}
// ----
// TypeError: (27-56): Type is required to live outside storage.
1 change: 0 additions & 1 deletion test/libsolidity/syntaxTests/types/mapping/library_argument_public.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,3 @@ library L {
}
}
// ----
// TypeError: (27-56): Type is required to live outside storage.
3 changes: 0 additions & 3 deletions test/libsolidity/syntaxTests/types/mapping/library_return_external.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,3 @@ library L
}
}
// ----
// TypeError: (27-58): Type is required to live outside storage.
// TypeError: (60-91): Type is required to live outside storage.
// TypeError: (123-152): Type is required to live outside storage.
3 changes: 0 additions & 3 deletions test/libsolidity/syntaxTests/types/mapping/library_return_public.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,3 @@ library L
}
}
// ----
// TypeError: (27-58): Type is required to live outside storage.
// TypeError: (60-91): Type is required to live outside storage.
// TypeError: (121-150): Type is required to live outside storage.
4 changes: 2 additions & 2 deletions test/libsolidity/syntaxTests/types/mapping/mapping_data_location_function_param_external.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ contract c {
function f1(mapping(uint => uint) calldata) pure external returns (mapping(uint => uint) memory) {}
}
// ----
// TypeError: (29-59): Type is required to live outside storage.
// TypeError: (29-59): Internal or recursive type is not allowed for public or external functions.
// TypeError: (29-59): Mapping types for parameters or return variables can only be used in internal or library functions.
// TypeError: (84-112): Mapping types for parameters or return variables can only be used in internal or library functions.
3 changes: 1 addition & 2 deletions test/libsolidity/syntaxTests/types/mapping/mapping_data_location_function_param_public.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,4 @@ contract c {
function f3(mapping(uint => uint) memory) view public {}
}
// ----
// TypeError: (29-57): Type is required to live outside storage.
// TypeError: (29-57): Internal or recursive type is not allowed for public or external functions.
// TypeError: (29-57): Mapping types for parameters or return variables can only be used in internal or library functions.
3 changes: 1 addition & 2 deletions test/libsolidity/syntaxTests/types/mapping/mapping_return_public_memory.sol
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,5 +3,4 @@ contract C {
}
}
// ----
// TypeError: (51-79): Type is required to live outside storage.
// TypeError: (51-79): Internal or recursive type is not allowed for public or external functions.
// TypeError: (51-79): Mapping types for parameters or return variables can only be used in internal or library functions.