Skip to content

Conversation

@devin-ai-integration
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot commented Oct 14, 2025

Description

Adds a reduce function to arrays in Cadence with the signature:

fun reduce<U>(initial: U, _ f: fun (U, T): U): U

The implementation follows the same three-layer pattern as the existing map function:

  • sema/type.go: Type system definition with generic type parameter U for the accumulator type, allowing it to differ from the array element type T
  • interpreter/value_array.go: Runtime execution that iterates through array elements and applies the reducer function
  • bbq/vm/value_array.go: Bytecode registration for both variable-sized and constant-sized arrays

Changes

Type System

  • Added ArrayReduceFunctionType with generic type parameter U
  • Registered reduce function with resource array rejection (consistent with map)

Interpreter

  • Implemented ArrayValue.Reduce method that iterates and accumulates values
  • Added NativeArrayReduceFunction wrapper
  • Registered in GetMethod for array values

Tests

  • Moved TestInterpretArrayReduce from misc_test.go to array_test.go (per review feedback)
  • Added sema tests for:
    • Basic reduce operations (sum, product)
    • Fixed-size arrays
    • Struct arrays with value-type elements
    • Invalid arguments and type mismatches
    • Resource arrays (correctly rejected)
  • Added interpreter tests for:
    • Empty arrays (returns initial value)
    • Type conversion (Int array → String accumulator)
    • Mutation during iteration (correctly raises ContainerMutatedDuringIterationError)

Critical Review Areas

  1. Generic type parameter handling: Verify ArrayReduceFunctionType correctly handles the case where accumulator type U differs from element type T

  2. Struct array test: The test testStructArrayReduce operates on an unauthorized reference &[S] and mutates struct fields inside the reducer. This works because struct elements are passed by value (copies), not by reference. Verify this is the intended test case - it demonstrates that mutations happen on copies when using unauthorized references.

  3. Resource array rejection: Confirm that rejecting reduce on resource arrays matches the intended behavior and is consistent with map


Link to Devin run: https://app.devin.ai/sessions/f6bc7843ea324388964f82a1cdc1b1b1
Requested by: [email protected]

  • Targeted PR against master branch
  • Linked to Github issue with discussion and accepted design OR link to spec that describes this work
  • Code follows the standards mentioned here
  • Updated relevant documentation
  • Re-reviewed Files changed in the Github PR explorer
  • Added appropriate labels

Implement array reduce function with signature:
fun reduce<U>(initial: U, _ f: fun (U, T): U): U

Changes:
- sema: Define ArrayReduceFunctionType and register in getArrayMembers
- interpreter: Implement ArrayValue.Reduce and NativeArrayReduceFunction
- vm: Register reduce function for both variable and constant-sized arrays
- tests: Add tests for reduce in arrays_dictionaries_test.go and misc_test.go
@devin-ai-integration
Copy link
Contributor Author

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

@github-actions
Copy link

github-actions bot commented Oct 14, 2025

Benchstat comparison

  • Base branch: onflow:master
  • Base commit: 3ec0c24
Results

old.txtnew.txt
time/opdelta
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-4392µs ± 0%392µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
ContractImport-4215µs ± 0%222µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ExportType/composite_type-4265ns ± 0%272ns ± 0%~(p=1.000 n=1+1)
ExportType/simple_type-477.8ns ± 0%77.8ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
FTTransfer-4122µs ± 0%122µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-419.9µs ± 0%20.5µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
ImperativeFib-413.8µs ± 0%14.2µs ± 0%~(p=1.000 n=1+1)
ImperativeFibNewCompilerNewVM-435.2µs ± 0%35.5µs ± 0%~(p=1.000 n=1+1)
ImperativeFibNewVM-416.1µs ± 0%16.7µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
InterpretRecursionFib-42.09ms ± 0%2.22ms ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
InterpreterFTTransfer-4109µs ± 0%111µs ± 0%~(p=1.000 n=1+1)
InterpreterImperativeFib-420.8µs ± 0%20.6µs ± 0%~(p=1.000 n=1+1)
InterpreterNewStruct-460.3µs ± 0%61.5µs ± 0%~(p=1.000 n=1+1)
MethodCall/concrete_type_method_call-445.6µs ± 0%45.6µs ± 0%~(all equal)
MethodCall/interface_method_call-467.5µs ± 0%67.9µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
NewInterpreter/new_interpreter-41.03µs ± 0%1.04µs ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_sub-interpreter-4348ns ± 0%334ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
NewResource-485.9µs ± 0%86.8µs ± 0%~(p=1.000 n=1+1)
NewStruct-432.6µs ± 0%34.1µs ± 0%~(p=1.000 n=1+1)
NewStructRaw-43.73µs ± 0%3.69µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
QualifiedIdentifierCreation/One_level-42.49ns ± 0%2.19ns ± 0%~(p=1.000 n=1+1)
QualifiedIdentifierCreation/Three_levels-485.1ns ± 0%85.3ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
RecursionFib-4782µs ± 0%808µs ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransfer-4727µs ± 0%722µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-4578µs ± 0%574µs ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransferVM-4645µs ± 0%625µs ± 0%~(p=1.000 n=1+1)
RuntimeResourceDictionaryValues-42.63ms ± 0%2.58ms ± 0%~(p=1.000 n=1+1)
RuntimeResourceTracking-411.9ms ± 0%11.9ms ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-415.2µs ± 0%14.4µs ± 0%~(p=1.000 n=1+1)
RuntimeVMInvokeContractImperativeFib-428.7µs ± 0%28.5µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
SuperTypeInference/arrays-4231ns ± 0%228ns ± 0%~(p=1.000 n=1+1)
SuperTypeInference/composites-490.8ns ± 0%91.8ns ± 0%~(p=1.000 n=1+1)
SuperTypeInference/integers-4308ns ± 0%304ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ValueIsSubtypeOfSemaType-466.2ns ± 0%71.8ns ± 0%~(p=1.000 n=1+1)
 
alloc/opdelta
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-4152kB ± 0%152kB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
ContractImport-473.8kB ± 0%74.1kB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ExportType/composite_type-4120B ± 0%120B ± 0%~(all equal)
ExportType/simple_type-40.00B 0.00B ~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
FTTransfer-437.8kB ± 0%37.8kB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-48.30kB ± 0%8.30kB ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
ImperativeFib-41.94kB ± 0%1.94kB ± 0%~(all equal)
ImperativeFibNewCompilerNewVM-421.8kB ± 0%21.8kB ± 0%~(all equal)
ImperativeFibNewVM-44.66kB ± 0%4.66kB ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
InterpretRecursionFib-41.19MB ± 0%1.19MB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
InterpreterFTTransfer-437.6kB ± 0%37.6kB ± 0%~(p=1.000 n=1+1)
InterpreterImperativeFib-48.29kB ± 0%8.29kB ± 0%~(all equal)
InterpreterNewStruct-424.4kB ± 0%24.4kB ± 0%~(p=1.000 n=1+1)
MethodCall/concrete_type_method_call-46.94kB ± 0%6.94kB ± 0%~(p=1.000 n=1+1)
MethodCall/interface_method_call-414.5kB ± 0%14.5kB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
NewInterpreter/new_interpreter-4976B ± 0%976B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-4232B ± 0%232B ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
NewResource-440.4kB ± 0%40.4kB ± 0%~(p=1.000 n=1+1)
NewStruct-410.3kB ± 0%10.4kB ± 0%~(p=1.000 n=1+1)
NewStructRaw-42.57kB ± 0%2.61kB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
QualifiedIdentifierCreation/One_level-40.00B 0.00B ~(all equal)
QualifiedIdentifierCreation/Three_levels-464.0B ± 0%64.0B ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
RecursionFib-488.1kB ± 0%88.1kB ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransfer-4232kB ± 0%232kB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-4156kB ± 0%155kB ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransferVM-4174kB ± 0%173kB ± 0%~(p=1.000 n=1+1)
RuntimeResourceDictionaryValues-41.76MB ± 0%1.76MB ± 0%~(p=1.000 n=1+1)
RuntimeResourceTracking-49.26MB ± 0%9.26MB ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-47.99kB ± 0%7.98kB ± 0%~(p=1.000 n=1+1)
RuntimeVMInvokeContractImperativeFib-410.3kB ± 0%10.3kB ± 0%~(all equal)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
SuperTypeInference/arrays-496.0B ± 0%96.0B ± 0%~(all equal)
SuperTypeInference/composites-40.00B 0.00B ~(all equal)
SuperTypeInference/integers-40.00B 0.00B ~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ValueIsSubtypeOfSemaType-448.0B ± 0%48.0B ± 0%~(all equal)
 
allocs/opdelta
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-42.47k ± 0%2.47k ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
ContractImport-4931 ± 0%931 ± 0%~(all equal)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ExportType/composite_type-43.00 ± 0%3.00 ± 0%~(all equal)
ExportType/simple_type-40.00 0.00 ~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
FTTransfer-4965 ± 0%965 ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-4176 ± 0%176 ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
ImperativeFib-449.0 ± 0%49.0 ± 0%~(all equal)
ImperativeFibNewCompilerNewVM-4210 ± 0%210 ± 0%~(all equal)
ImperativeFibNewVM-486.0 ± 0%86.0 ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
InterpretRecursionFib-417.7k ± 0%17.7k ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
InterpreterFTTransfer-4788 ± 0%788 ± 0%~(all equal)
InterpreterImperativeFib-4175 ± 0%175 ± 0%~(all equal)
InterpreterNewStruct-4418 ± 0%418 ± 0%~(all equal)
MethodCall/concrete_type_method_call-4135 ± 0%135 ± 0%~(all equal)
MethodCall/interface_method_call-4285 ± 0%285 ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
NewInterpreter/new_interpreter-415.0 ± 0%15.0 ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-44.00 ± 0%4.00 ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
NewResource-4735 ± 0%735 ± 0%~(all equal)
NewStruct-4219 ± 0%219 ± 0%~(all equal)
NewStructRaw-438.0 ± 0%38.0 ± 0%~(all equal)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
QualifiedIdentifierCreation/One_level-40.00 0.00 ~(all equal)
QualifiedIdentifierCreation/Three_levels-42.00 ± 0%2.00 ± 0%~(all equal)
pkg:github.com/onflow/cadence/bbq/vm/test goos:linux goarch:amd64
RecursionFib-43.65k ± 0%3.65k ± 0%~(all equal)
RuntimeFungibleTokenTransfer-43.94k ± 0%3.94k ± 0%~(all equal)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-42.93k ± 0%2.93k ± 0%~(all equal)
RuntimeFungibleTokenTransferVM-43.06k ± 0%3.06k ± 0%~(all equal)
RuntimeResourceDictionaryValues-436.7k ± 0%36.7k ± 0%~(all equal)
RuntimeResourceTracking-4159k ± 0%159k ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-4113 ± 0%113 ± 0%~(all equal)
RuntimeVMInvokeContractImperativeFib-4213 ± 0%213 ± 0%~(all equal)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
SuperTypeInference/arrays-43.00 ± 0%3.00 ± 0%~(all equal)
SuperTypeInference/composites-40.00 0.00 ~(all equal)
SuperTypeInference/integers-40.00 0.00 ~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ValueIsSubtypeOfSemaType-41.00 ± 0%1.00 ± 0%~(all equal)
 

@turbolent turbolent self-assigned this Oct 14, 2025
Per review comment: add function signature comments similar to ArrayMapFunctionType
@devin-ai-integration
Copy link
Contributor Author

Added comments in commit 86d3e8e. The function now includes:

  • Signature description: // fun reduce<U>(initial: U, _ f: fun (U, T): U): U
  • Type documentation: // reducerFuncType: (U, T) -> U

Similar to how ArrayMapFunctionType is documented.

Per review comment: inline the argumentTypes slice directly in the invokeFunctionValue call
@devin-ai-integration
Copy link
Contributor Author

Done in commit a8942f8. Inlined argumentTypes directly in the invokeFunctionValue call.

turbolent and others added 3 commits October 14, 2025 20:18
function type is the same for variable-sized and constant-sized array types
Per review comment: Keep only the function signature comment
@turbolent turbolent mentioned this pull request Oct 15, 2025
18 tasks
…based inner array mutation; update expectations
@onflow onflow deleted a comment from devin-ai-integration bot Oct 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants