-
Notifications
You must be signed in to change notification settings - Fork 716
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
Document F-bounds and inference using bounds for 3.7 #6403
Conversation
Visit the preview URL for this PR (updated for commit a180b33): |
@parlough The error from
For the |
Yep! Now I've updated
No, I think that's good. I pushed a change that adds an |
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.
The edits LGTM. Thank you so much, Marya! I'd like to known @eernstg's opinion on this too.
Thank you Chloe! And yes I will definitely wait for Erik's input, looking forward to it! |
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.
Thanks for the kind words! I added a bunch of comments, hope they are helpful!
src/content/language/type-system.md
Outdated
|
||
void main() { | ||
f(B()); // OK. | ||
f(C()); // Inference fails, compile-time error. |
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.
Inference fails when this feature isn't available. When the feature is available, inference succeeds. I don't think that's obvious at this point. I also don't know how it could be explained in that comment without breaking the 80-or-whatever limit on the line length. ;-)
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.
Yeah I can see from all your suggestions that I wrote this section kind of on the line between treating this like a new feature/improvement, and trying to make it sound like this is "just the way it works". I'll clean up the whole section with that in mind, thank you for the thorough review!
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.
Maybe someone can help with code comment best practices, but I added a longer explanation here:
f(C()); // OK. Without using bounds, inference relying on best-effort
// approximations would fail after detecting that C is not a subtype of A<C>.
Co-authored-by: Erik Ernst <[email protected]>
Co-authored-by: Erik Ernst <[email protected]>
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.
Added a couple of comments.
src/content/language/type-system.md
Outdated
|
||
With inference using bounds, Dart can *deconstruct* type arguments, | ||
extracting type information from a generic type parameter's bound. | ||
This allows functions like `f1` in the following example to preserve both the |
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.
When we don't have f2
, perhaps a name like f
would suffice. (Unless there's a name clash with some other declaration named f
nearby, but I couldn't see any).
Co-authored-by: Erik Ernst <[email protected]>
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.
Thanks @MaryaBelanger and everyone for working so hard on figuring out what to include here and how to word it.
Looks good to me! Just one suggestion to consider:
Ahh sorry I merged it too fast! I'll reopen a pr for your fixes, thank you for checking |
Fixes #6258
I added the section "Self-referential type parameter restriction" to the generics page, to support the "Inference using bounds" section on the Type system page (staged links).
This is a difficult topic to summarize simply enough for the language tour! I did my best, but please let me know if anything needs to change / if this misses the point of the new feature.