Add support for generic function declarations #2463
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Cadence Benchstat comparisonThis branch with compared with the base branch onflow:master commit d7f3648 Collapsed results for better readability
|
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Codecov Report
@@ Coverage Diff @@
## master #2463 +/- ##
==========================================
+ Coverage 78.29% 78.32% +0.02%
==========================================
Files 325 325
Lines 72465 72587 +122
==========================================
+ Hits 56738 56851 +113
- Misses 13644 13649 +5
- Partials 2083 2087 +4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 2 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
dsainati1
reviewed
Apr 24, 2023
| if items.length < 1 { | ||
| return nil | ||
| } | ||
| return items[0] |
There was a problem hiding this comment.
I think we should have some more tests for these; in particular I'd like to see tests for when a generically-typed value "escapes" the scope of its function into an external/global variable. E.g. if I have a function like this that instead stores items[0] in a global array of type [AnyStruct] vs one of type [String], and how these subtyping checks interact with the type parameter bound.
Similarly imo we would also want tests to make sure that the type parameter is usable as a value of its bound type inside the function body; e.g. if I have a T: [AnyStruct] i'd expect to be able to call array functions on a value of type T.
dsainati1
reviewed
Apr 24, 2023
There was a problem hiding this comment.
This is cool! Generics is usually a pretty complicated feature in how it interacts with the rest of a language's features though, so even if it's simple to enable here I think we want to make sure we test it thoroughly before enabling it.
I also think we do need to add some interpreter tests to this PR. Even though we don't add support here for using the type variable inside the body of the function (like in a cast), it's still possible to downcast a value that is typed with a variable. E.g. if you have some parameter v: T where T is a type variable, you might have a later expression v as? X where X is some other type. IMO it's worth adding tests cases to the interpreter for these sorts of uses. Most particularly we should test the cases where T is a reference type, even if it's declared with an AnyStruct type bound we want to make sure that the previously identified bug with casting between AnyStruct and reference types doesn't remanifest itself here.
|
This will be super useful to write clean code. Will this allow multiple Type parameters? |
6 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
It turns out that adding basic support for generic functions does not take much effort, as we already have support for generic built-in functions.
Even with the following limitations, this feature should already make some programs safer and allow developer to express their intent better:
For now, type parameters may only occur in the parameter list and the return type.
We may add support for type parameter occurrences in the function body later, which would require some work in the interpreter.
Interface function declarations may not be generic (yet).
masterbranchFiles changedin the Github PR explorer…clarations