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

Array and Dictionary mutation functions on value typed collections can't be used in a view context #3222

Open
dete opened this issue Apr 5, 2024 · 3 comments

Comments

@dete
Copy link

dete commented Apr 5, 2024

Current Behavior

The array and dictionary self modification functions aren't marked as view, and thus trigger an error when used in a view function, even if those arrays/dictionaries are on primitive/value types and local to the function (and thus not actually mutating storage).

Expected Behavior

Modifying arrays and dictionaries in other ways is allowed in a view context (eg. assignment), and so I would assume that calling methods on these types that modify those specific instances would also be okay.

Steps To Reproduce

        access(all) view fun happyFunction() {
            var myArray = [0, 1, 2]
            myArray[0] = 1          // modifying an element is okay
            myArray.append(5)       // adding a new element is an error
            myArray.remove(at:0)    // removing an element is an error

            var myDict = {0: 0, 1: 1, 2: 2}
            myDict[0] = 1           // modifying an element is okay
            myDict[3] = 3           // adding a new element is okay
            myDict.remove(key: 0)   // removing an element is an error
        }

Environment

- Cadence version: 1.0
- Network: n/a
@dete dete added Bug Something isn't working Feedback labels Apr 5, 2024
@turbolent turbolent added Feature and removed Bug Something isn't working labels Apr 5, 2024
@turbolent
Copy link
Member

The new notion of view can probably extended further, but this is not a bug in the sense that it was promised to be supported.

@dete Is this blocking any work for the Crescendo release? If not, we can schedule it after that milestone. If it is, we'll need to investigate how this could be supported, which will likely be non-trivial in terms of effort required for including the whole feature (design, implementation, testing, documentation, release) and shift timelines accordingly

@dete
Copy link
Author

dete commented Apr 5, 2024

Definitely not a blocker for Crescendo.

I would disagree about "not a bug", however. Having the built-in library be "const correct" is expected behaviour (at least expected by me, I guess!). Not a high priority, but I'm working on a PoC where I ran into this, and the work around using filter is not pretty!

@bluesign
Copy link
Contributor

yeah this is one of the most common errors ( second most common actually ) when upgrading mainnet contracts with cadence upgrader ( https://github.com/bluesign/cadenceUpgrader )

best work I came up with is: myArray = myArray.concat([5]) but I was hesitant to add it to converter.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants