cadence icon indicating copy to clipboard operation
cadence copied to clipboard

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

Open dete opened this issue 1 year ago • 4 comments

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 avatar Apr 05 '24 17:04 dete

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

turbolent avatar Apr 05 '24 17:04 turbolent

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!

dete avatar Apr 05 '24 20:04 dete

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.

bluesign avatar Apr 18 '24 07:04 bluesign

We need a prioritization call with product team to determine priority of this.

j1010001 avatar Jul 19 '24 18:07 j1010001