Skip to content

Add comparison functions#4430

Open
turbolent wants to merge 9 commits intomasterfrom
bastian/min-max-functions
Open

Add comparison functions#4430
turbolent wants to merge 9 commits intomasterfrom
bastian/min-max-functions

Conversation

@turbolent
Copy link
Copy Markdown
Member

@turbolent turbolent commented Feb 6, 2026

Description

Implement FLIP 357: Add minOf and maxOf Functions to Cadence.

Add new functions minOf<T>(T, T) and maxOf<T>(T, T), where T must be comparable (we can't express this with a type bound yet).

The tests are currently only run with the interpreter, just like the rest of the stdlib tests. I left a TODO in #3804 to refactor all tests to be also run with the compiler/VM.

The names are not min/max, as existing contracts commonly use these names e.g. in variable declarations, and would break, as we do not allow shadowing of built-in functions. The names are taken from Kotlin: https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.comparisons/min-of.html


  • 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

@turbolent turbolent self-assigned this Feb 6, 2026
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 6, 2026

Dependency Review

✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.

Scanned Files

None

@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 6, 2026

Benchstat comparison

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

old.txtnew.txt
time/opdelta
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ByteArrayValueToByteSlice-478.0ns ± 0%78.2ns ± 0%~(p=1.000 n=1+1)
ByteSliceToByteArrayValue-4888ns ± 0%842ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-4413µs ± 0%401µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
Emit-44.52ms ± 0%4.56ms ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ExportType/composite_type-4265ns ± 0%262ns ± 0%~(p=1.000 n=1+1)
ExportType/simple_type-478.1ns ± 0%77.7ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
FlowTokenContract-4611µs ± 0%603µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-423.3µs ± 0%23.3µs ± 0%~(p=1.000 n=1+1)
InterpretRecursionFib-42.24ms ± 0%2.27ms ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_interpreter-4904ns ± 0%918ns ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_sub-interpreter-4327ns ± 0%333ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
QualifiedIdentifierCreation/One_level-41.87ns ± 0%2.50ns ± 0%~(p=1.000 n=1+1)
QualifiedIdentifierCreation/Three_levels-478.7ns ± 0%81.5ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-4626µs ± 0%635µs ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransferVM-4682µs ± 0%738µs ± 0%~(p=1.000 n=1+1)
RuntimeResourceDictionaryValues-42.66ms ± 0%2.65ms ± 0%~(p=1.000 n=1+1)
RuntimeResourceTracking-411.3ms ± 0%11.4ms ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-414.7µs ± 0%14.7µs ± 0%~(p=1.000 n=1+1)
RuntimeVMInvokeContractImperativeFib-440.4µs ± 0%40.1µs ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
SuperTypeInference/arrays-4217ns ± 0%218ns ± 0%~(p=1.000 n=1+1)
SuperTypeInference/composites-498.0ns ± 0%92.2ns ± 0%~(p=1.000 n=1+1)
SuperTypeInference/integers-4313ns ± 0%320ns ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ValueIsSubtypeOfSemaType-460.5ns ± 0%60.6ns ± 0%~(p=1.000 n=1+1)
 
alloc/opdelta
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ByteArrayValueToByteSlice-432.0B ± 0%32.0B ± 0%~(all equal)
ByteSliceToByteArrayValue-4851B ± 0%846B ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-4153kB ± 0%152kB ± 0%~(p=1.000 n=1+1)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
Emit-41.50MB ± 0%1.50MB ± 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/sema goos:linux goarch:amd64
FlowTokenContract-4223kB ± 0%223kB ± 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)
InterpretRecursionFib-41.19MB ± 0%1.19MB ± 0%~(p=1.000 n=1+1)
NewInterpreter/new_interpreter-4976B ± 0%976B ± 0%~(all equal)
NewInterpreter/new_sub-interpreter-4232B ± 0%232B ± 0%~(all equal)
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/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-4170kB ± 0%170kB ± 0%~(p=1.000 n=1+1)
RuntimeFungibleTokenTransferVM-4192kB ± 0%193kB ± 0%~(p=1.000 n=1+1)
RuntimeResourceDictionaryValues-41.76MB ± 0%1.76MB ± 0%~(p=1.000 n=1+1)
RuntimeResourceTracking-48.58MB ± 0%8.60MB ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-48.05kB ± 0%8.05kB ± 0%~(p=1.000 n=1+1)
RuntimeVMInvokeContractImperativeFib-413.3kB ± 0%13.3kB ± 0%~(all equal)
pkg:github.com/onflow/cadence/sema goos:linux goarch:amd64
SuperTypeInference/arrays-472.0B ± 0%72.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-432.0B ± 0%32.0B ± 0%~(all equal)
 
allocs/opdelta
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ByteArrayValueToByteSlice-41.00 ± 0%1.00 ± 0%~(all equal)
ByteSliceToByteArrayValue-45.00 ± 0%5.00 ± 0%~(all equal)
pkg:github.com/onflow/cadence/runtime goos:linux goarch:amd64
ContractFunctionInvocation-42.47k ± 0%2.47k ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
Emit-440.0k ± 0%40.0k ± 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/sema goos:linux goarch:amd64
FlowTokenContract-43.58k ± 0%3.58k ± 0%~(all equal)
pkg:github.com/onflow/cadence/interpreter goos:linux goarch:amd64
ImperativeFib-4176 ± 0%176 ± 0%~(all equal)
InterpretRecursionFib-417.7k ± 0%17.7k ± 0%~(all equal)
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/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/runtime goos:linux goarch:amd64
RuntimeFungibleTokenTransferInterpreter-43.27k ± 0%3.27k ± 0%~(all equal)
RuntimeFungibleTokenTransferVM-43.73k ± 0%3.73k ± 0%~(all equal)
RuntimeResourceDictionaryValues-436.7k ± 0%36.7k ± 0%~(all equal)
RuntimeResourceTracking-4144k ± 0%144k ± 0%~(p=1.000 n=1+1)
RuntimeScriptNoop-4114 ± 0%114 ± 0%~(all equal)
RuntimeVMInvokeContractImperativeFib-4424 ± 0%424 ± 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)
 

Copy link
Copy Markdown
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

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

Nice!

@turbolent
Copy link
Copy Markdown
Member Author

Unfortunately, adding new built-ins causes checking errors for several deployed contracts, as we do not allow shadowing of built-ins, and min and max are frequently used variable names.

I guess we could introduce a Math contract and have Math.min and Math.max?

@SupunS
Copy link
Copy Markdown
Member

SupunS commented Feb 6, 2026

ah I see!

I guess we could introduce a Math contract and have Math.min and Math.max?

This sounds like a good idea. I guess we would have to limit it for Number types only (rather than all comparable types - mainly because the of the naming).

@turbolent
Copy link
Copy Markdown
Member Author

We already ran into this problem in #3055, and had started discussing a solution there: #3055 (comment)

From @dete:

I’ve never seen it before, but you could implement max, min, (and maybe clamp) as a member variable on the numeric types.

let x: Fix64 = ...
x = x.max(10.0)
x = x.clamp(0.0, 1.0)

Math.min and Math.max would be more typical, but now that I’m thinking about it, the max, min, clamp functions seem really quite clean. Could be clampMax, clampMin and clamp since the semantics are different.

Swift does that for some functions, like sqrt: https://developer.apple.com/documentation/swift/double/squareroot(), and uses static functions for e.g min/max: https://developer.apple.com/documentation/swift/double/maximum(_:_:). We have static functions for built-in types, so that would also be an option for us

@turbolent
Copy link
Copy Markdown
Member Author

Regarding min and max specifically, we already have min and max fields on the types which return the minimum and maximum value representable by the type (e.g. UInt8.max == 255), so we would have to name the functions differently, e.g. minimum and maximum.

These functions are also not tied to numeric types, but useful for any comparable type. In that sense defining them in a Math module is not really appropriate.

@turbolent turbolent changed the title Add min and max functions Add minOf and maxOf functions Feb 9, 2026
@turbolent turbolent force-pushed the bastian/min-max-functions branch from fc97a02 to 32abb30 Compare February 9, 2026 22:27
@turbolent
Copy link
Copy Markdown
Member Author

turbolent commented Feb 9, 2026

I kept it simple and took inspiration from Kotlin, where the functions have an Of suffix, e.g. https://kotlinlang.org/api/core/kotlin-stdlib/kotlin.comparisons/min-of.html. This does not break any currently deployed contracts.

Copy link
Copy Markdown
Member

@SupunS SupunS left a comment

Choose a reason for hiding this comment

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

kept it simple and took inspiration from Kotlin, where the functions have a Of suffix

That's a great idea!

@codecov
Copy link
Copy Markdown

codecov bot commented Feb 17, 2026

Codecov Report

❌ Patch coverage is 89.18919% with 16 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
stdlib/comparison.go 89.18% 10 Missing and 6 partials ⚠️

📢 Thoughts on this report? Let us know!

@turbolent turbolent changed the title Add minOf and maxOf functions Add Comparison functions Feb 17, 2026
@turbolent turbolent changed the title Add Comparison functions Add comparison functions Feb 17, 2026
@turbolent turbolent requested review from RZhang05 and SupunS March 10, 2026 20:53
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