-
Notifications
You must be signed in to change notification settings - Fork 138
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
Extend type bounds #3059
base: master
Are you sure you want to change the base?
Extend type bounds #3059
Conversation
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit a0dd6cf Collapsed results for better readability
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3059 +/- ##
==========================================
- Coverage 80.74% 80.61% -0.14%
==========================================
Files 380 381 +1
Lines 93307 93886 +579
==========================================
+ Hits 75339 75684 +345
- Misses 15260 15499 +239
+ Partials 2708 2703 -5
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
cc @turbolent do we need to implement importing/exporting if the only places we are currently using the expanded type bounds is in builtin types? |
// TODO: implement this for generalized bounds | ||
return cadence.NewTypeParameter(name, typeBound), nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be sufficient to support decoding any function types that aren't one of the small set of builtins that this PR changes.
// TODO: perhaps generalize this? | ||
if typeParameter.TypeBound == nil { | ||
return e.encodeNullableTypeValue(nil, visited) | ||
} | ||
|
||
subTypeBound, isSubtypeBound := typeParameter.TypeBound.(cadence.SubtypeTypeBound) | ||
if !isSubtypeBound { | ||
panic(cadenceErrors.NewUnexpectedError("cannot store non-subtype bounded function type parameter %s", typeParameter.TypeBound)) | ||
} | ||
|
||
// element 1: type as type-bound. | ||
return e.encodeNullableTypeValue(typeParameter.TypeBound, visited) | ||
return e.encodeNullableTypeValue(subTypeBound.Type, visited) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be sufficient to support encoding any function types that aren't one of the small set of builtins that this PR changes.
if typeBound != nil { | ||
convertedParameterTypeBound = ExportMeteredType(gauge, typeBound, results) | ||
convertedParameterTypeBound = exportTypeBound(gauge, typeBound, results) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Perhaps I just couldn't find it, but is there an equivalent importFunctionType
or similar that also needs to change here?
Work towards https://github.com/dapperlabs/cadence-internal/issues/188
Description
See https://github.com/dapperlabs/cadence-internal/issues/188#issuecomment-1906752366
Currently, type bounds for type parameters may only be subtype relationships.
Generalize the type bounds feature to support:
T <: U
)T <: U && T != U
)U <: T
)U <: T && T != U
)X && Y
)X || Y
)This also replaces some existing cases that were using
TypeArgumentsCheck
to use this new generalized type bound infrastructure. In particular:Capabilities.get
,InclusiveRange
andrevertibleRandom
.TODO
master
branchFiles changed
in the Github PR explorer