Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Snapshot WarningsEnsure that dependencies are being submitted on PR branches and consider enabling retry-on-snapshot-warnings. See the documentation for more information and troubleshooting advice. Scanned FilesNone |
Benchstat comparison
Results
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Unfortunately, adding new built-ins causes checking errors for several deployed contracts, as we do not allow shadowing of built-ins, and I guess we could introduce a |
|
ah I see!
This sounds like a good idea. I guess we would have to limit it for |
|
We already ran into this problem in #3055, and had started discussing a solution there: #3055 (comment) From @dete:
Swift does that for some functions, like |
|
Regarding These functions are also not tied to numeric types, but useful for any comparable type. In that sense defining them in a |
fc97a02 to
32abb30
Compare
|
I kept it simple and took inspiration from Kotlin, where the functions have an |
32abb30 to
5e12934
Compare
SupunS
left a comment
There was a problem hiding this comment.
kept it simple and took inspiration from Kotlin, where the functions have a Of suffix
That's a great idea!
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
Co-authored-by: Raymond Zhang <zhangraymond05@gmail.com>
| _, err := parseAndCheck(t, ` | ||
| import Comparison | ||
|
|
||
| let result: Int = min(5, 10) |
There was a problem hiding this comment.
Doesn't the function name need to be import-qualified? e.g: Comparison.min(...)?
There was a problem hiding this comment.
No, in the FLIP the proposal was to define a new built-in location Comparison, but it exports the functions directly, not nested in a contract.
I still think that makes sense for such ubiquitous functions. Requiring the functions to be qualified would make the expressions where they are used very verbose, to the point where developers likely will just opt to not use them.
Description
Implement FLIP 357: Add minOf and maxOf Functions to Cadence.
Add new functions
minOf<T>(T, T)andmaxOf<T>(T, T), whereTmust 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.htmlmasterbranchFiles changedin the Github PR explorer