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

Borrowing arguments ("Alan writes a web framework") #101

Merged
merged 14 commits into from
Apr 5, 2021

Conversation

alsuren
Copy link
Contributor

@alsuren alsuren commented Mar 26, 2021

This PR contains a write-up of my experiences from #78 (comment)

There is a bit of solutionising towards the end. I can rip that out if you want:

Alan longs to be able to say "this function takes an async function as a callback" (fn register_handler(handler: impl async Fn(state: &mut State) -> Result<Response, Error>)) and have Rust elide the lifetimes for him, like how they are elided for async functions.

It's also super-trimmed-down compared to my full experiences, and we could probably flesh out the morals a bit better.

@jbr
Copy link

jbr commented Mar 26, 2021

A big plus one on this one. Not being able to describe the trait of async functions with non-static lifetimes in the arguments is by far the biggest pain in async rust right now. I'm excited to see how GATs improve this situation, but that's about solutions. Whole web framework designs might be different if mutable borrows could be passed into handler functions instead of owned types

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! A few comments based on a quick read! The short version is that more examples would help bring the story to life for me.

src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
Copy link
Member

@rylev rylev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's definitely some really great insights here, but it looks like it's missing a bit of detail to really make the lessons obvious. Adding example errors and a bit more explanation of what exactly was confusing will go a long way.

src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
@tanriol
Copy link

tanriol commented Apr 1, 2021

You're missing one more problem here: the final to_async_borrowing works nicely for async functions, but not for closures because the closure type inference is not powerful enough (rust-lang/rust#70263), or at least the developers haven't managed to trick rustc into handling it yet.

Copy link
Contributor

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like this is pretty good now! Let me review the outstanding comments, but I might just merge.

src/vision/status_quo/alan_writes_a_web_framework.md Outdated Show resolved Hide resolved
@nikomatsakis nikomatsakis marked this pull request as ready for review April 5, 2021 15:54
@nikomatsakis nikomatsakis merged commit cf718d9 into rust-lang:master Apr 5, 2021
@nikomatsakis nikomatsakis added the status-quo-story-ideas "Status quo" user story ideas label Apr 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status-quo-story-ideas "Status quo" user story ideas
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants