Skip to content
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

New "Function" syntax and current release status #30761

Closed
matanlurey opened this issue Sep 15, 2017 · 14 comments
Closed

New "Function" syntax and current release status #30761

matanlurey opened this issue Sep 15, 2017 · 14 comments
Assignees

Comments

@matanlurey
Copy link
Contributor

matanlurey commented Sep 15, 2017

It might make sense to document somewhere that in stable it is almost completely unusable (due to analyzer crashes, and due to it not being implemented in the VM), and even in dev there are a lot of quality-of-life issues that make using it over the old typedef unsuitable:

EDIT: Re-organized the list of issues as-of 2017/10/15.

Still broken:

Not specific to the new syntax:

Fixed or otherwise closed:

Some of these are admittedly P2, others have workarounds, and some are P1.

Did we use this feature at all in our libraries or SDK before releasing?

/cc @anders-sandholm @mit-mit @kevmoo

@matanlurey matanlurey changed the title New "Function" syntax should not have been released New "Function" syntax and current release status Sep 15, 2017
@jmesserly
Copy link

I fixed analyzer resolution & DDC recently, FYI. I haven't had a chance to go through and close out old bugs though. So it's worth retesting any older ones.

@matanlurey
Copy link
Contributor Author

Thanks! I think engineering effort here (and responsiveness!) has been awesome, my main issue has been with release management and verification :)

@jmesserly
Copy link

jmesserly commented Sep 16, 2017

Thanks! I think engineering effort here (and responsiveness!) has been awesome, my main issue has been with release management and verification :)

Yeah, agreed. I thought it worked already, so I used it in a test case and was really surprised by crashes. That's how I ended up implementing Analyzer and DDC support :). It's definitely in a confusing state still (I'm not sure how well it works in other implementations, for example the Dart Formatter seems to crash on the syntax).

EDIT: also, fwiw, #30180 and #30379 are not specific to the new syntax.

@vsmenon
Copy link
Member

vsmenon commented Sep 25, 2017

@anders-sandholm @kevmoo - what's the right way to track this?

@jcollins-g
Copy link
Contributor

#30146 is specific to the new syntax.

@jcollins-g
Copy link
Contributor

@matanlurey Use of the new function syntax outside of a typedef was not handled properly in dartdoc and I didn't notice until something broke our dartdoc asserts in dart-lang/dartdoc#1505. That something was 38bf70d, specifically, the use of the Function syntax here:

https://github.com/dart-lang/sdk/blame/38bf70d7ac4ea695adc65ae3d7d0fa083b4dcd46/sdk/lib/async/zone.dart#L573

Since our asserts would have started firing sooner otherwise, I can say with some confidence that we haven't used the Function syntax in this manner inside the SDK until that CL.

@kevmoo
Copy link
Member

kevmoo commented Oct 3, 2017

Another issues with this – caused by rolling out support in pkg/http2 - dart-lang/http#1349

@MichaelRFairhurst
Copy link
Contributor

Added two other bugs that seem to belong here

@MichaelRFairhurst
Copy link
Contributor

I closed
#29665
(Analyzer crash using a function type as a bound on a class type argument), seems to be fixed in latest.

Also looks like I should not have added #30147
(Analyzer: crash with partial application of generic function typed function), its actually not related to generic function types but generic methods in general.

@MichaelRFairhurst
Copy link
Contributor

#30858 is still open because the "fix" caused a secondary issue. It's a pretty deep issue related to synthetic generic element types which I've not had much luck with. Latest solution involved turning off an assertion and is up for review: https://dart-review.googlesource.com/c/sdk/+/20481

@MichaelRFairhurst
Copy link
Contributor

Added a few more bugs here. Some maybe shouldn't be here based on being maybe more general and/or low priority, but, I wanted to err on the side of tracking too much.

@MichaelRFairhurst
Copy link
Contributor

MichaelRFairhurst commented Jan 5, 2018

Closed out two more. Most of the remaining are low priority, #30858 remains the most important but also most difficult.

@MichaelRFairhurst
Copy link
Contributor

The big one (#30858) closed, though not without creating some other subtle issues which are noted in #31804

@matanlurey
Copy link
Contributor Author

I haven't run into any problems here for a while. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants