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.

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

Support "calldata" as an overloaded location for reference variables. #15000

Open
Amxx opened this issue Apr 8, 2024 · 3 comments
Open

Support "calldata" as an overloaded location for reference variables. #15000

Amxx opened this issue Apr 8, 2024 · 3 comments
Labels
feature must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.

Comments

@Amxx
Copy link

Amxx commented Apr 8, 2024

Abstract

Disambiguate "memory" and "calldata" when overloading

Motivation

Lets consider the following library as a minimal example

library L {
    struct S { bytes content; bytes signature; bytes metadata; }
    function getHash(S storage s) internal view returns (bytes32) { return keccak256(s.content); }
    function getHash(S memory s) internal pure returns (bytes32) { return keccak256(s.content); }
    function getHash(S calldata s) internal pure returns (bytes32) { return keccak256(s.content); }
}

We may want to get a hash of the content, or any other value that requires reading par of (but not all of) the structure.
Depending on where the structure lives, copying all of it to memory might be a waste (in our example, signature & metadata might be very large, and are not actually needed in getHash).

In these cases, it is more efficient to pass the structure as a reference, and only load/copy to memory what is actually going to be used.

This is true if the struct is in storage, but a similar logic applies to structures in calldata (even though the saving are smaller).

So far, solidity has no issue handling

    function getHash(S storage s) internal view returns (bytes32) { return keccak256(s.content); }
    function getHash(S memory s) internal pure returns (bytes32) { return keccak256(s.content); }

but adding the third (calldata) version causes an error.

Specification

Distinguish memory and calldata datalocation so that overriden function do not clash when both localities are implemented separatly.

  • If a calldata function is available, calldata objects should use it.
  • If no calldata function is available, but a memory function is available, then the calldata objets should be copied to memory and the memory function used (as today?)

Backwards Compatibility

TBD

@Amxx Amxx added the feature label Apr 8, 2024
Copy link

github-actions bot commented Jul 8, 2024

This issue has been marked as stale due to inactivity for the last 90 days.
It will be automatically closed in 7 days.

@github-actions github-actions bot added the stale The issue/PR was marked as stale because it has been open for too long. label Jul 8, 2024
@Amxx
Copy link
Author

Amxx commented Jul 15, 2024

this is still relevant

@ekpyron ekpyron added must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it. and removed stale The issue/PR was marked as stale because it has been open for too long. labels Jul 15, 2024
@ekpyron
Copy link
Member

ekpyron commented Jul 15, 2024

Note that even if we did allow the declarations, being able to properly use the overloads would depend on #1256, so this issue depends on #1256

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature must have eventually Something we consider essential but not enough to prevent us from releasing Solidity 1.0 without it.
Projects
None yet
Development

No branches or pull requests

2 participants