Unboxed closures #14539

pcwalton · 2014-05-30T06:18:52Z

bstrie · 2014-05-30T13:46:02Z

src/compiletest/header.rs

+        let force_host_ptr = &mut force_host;
+        let check_stdout_ptr = &mut check_stdout;
+        let no_prefer_dynamic_ptr = &mut no_prefer_dynamic;
+        let no_pretty_expanded_ptr = &mut no_pretty_expanded;


Are we still considering the syntax sugar from @thestinger's RFC to potentially make this less horrifying?

Or perhaps I'm misunderstanding, is this only a temporary affair until |&mut: foo| is implemented?

It's not temporary, it's just how you need to capture by-reference and by-mutable-reference when there's not already a reference. This is the case the sugar would improve, but it doesn't appear to be very common.

pcwalton · 2014-05-31T06:19:58Z

This is getting pretty close. Needs a rebase and some tests.

pcwalton · 2014-05-31T18:01:01Z

OK, this is ready for initial review. There are several known problems, but I figured as this patch is over 1KLOC already it's best to fix them in followups. Known issues:

Some stuff may be broken cross-crate.
Trans fails with an assertion if you don't specify the return type on the closure. Probably a simple writeback issue.
Only FnMut is implemented, not Fn or FnOnce (again, probably not hard of a fix).
You have to explicitly call the closure with .call_mut() instead of using the () operator (probably the largest remaining work item).
Complex destructors on unboxed closures are untested and may be hilariously broken.

This patch includes simple unit tests for direct calls, calls through a generic type parameter, and boxed calls.

r? @nikomatsakis (or whoever)

alexcrichton · 2014-05-31T18:06:38Z

(drive by comment)

Would it be easy to put this all behind a feature gate until the feature is ready for prime-time?

pcwalton · 2014-05-31T20:03:17Z

This is now behind a gate. Also fixed the formatting issues.

huonw · 2014-06-03T21:51:23Z

src/llvm

@@ -1 +1 @@
-Subproject commit 0a894645cf120539876e9eb4eb0d7b572dfa9d14
+Subproject commit 4b4d0533b4f76cc3fbba31bd9e7ac02e0c738b1d


Accidental?

nikomatsakis · 2014-06-07T01:43:25Z

Started reading this today. Looks pretty nice! Not finished yet.

nikomatsakis · 2014-06-10T17:03:09Z

src/test/run-pass/unboxed-closures-generic.rs

+pub fn main() {
+    let f = |&mut: x: int, y: int| -> int { x + y };
+    let z = call_it(3, f);
+    println!("{}", z);


change this to assert that z has the correct value

this comment seems like it was not addressed.

nikomatsakis · 2014-06-10T17:14:42Z

OK, left a few comments on things that need addressing.

@jbclements

the leading quote part of the identifier for the purposes of hygiene. This adopts @jbclements' solution to rust-lang#14539. I'm not sure if this is a breaking change or not. Closes rust-lang#12512. [breaking-change]

@jbclements

the leading quote part of the identifier for the purposes of hygiene. This adopts @jbclements' solution to rust-lang#14539. I'm not sure if this is a breaking change or not. Closes rust-lang#12512. [breaking-change]

alexcrichton · 2014-06-30T00:33:26Z

ping @pcwalton, what's the status of this?

pcwalton · 2014-06-30T00:34:05Z

It's in my queue of P-backcompat-lang things to do.

pcwalton · 2014-07-09T04:47:11Z

Current status: I have rebased the patch and addressed about half of @nikomatsakis' comments. However, this collided badly with the other unboxed closures work, necessitating some extra functionality. Specifically, I need to add tupling and untupling machinery in several places in trans in order to keep the tests passing, as they call the traits by name and implement them manually.

I think that, semantically, @eddyb had the right idea all along: exposing the tupling/untupling is pretty messy, as is the rust-call ABI that I had to add. The elegant solution is to say that Fn/FnMut/FnOnce are actually an unbounded family of traits, only nameable via the |int| -> int syntax. I think that it may well be the case that the lang items should be marked experimental and behind a feature gate forever (unless we get variadic generics, I guess). The guts of the implementation are pretty gross in places, and I want to confine this ugliness to the implementation and not expose it as part of the real semantics of the language.

@eddyb, you are perfectly within your rights to say "I told you so". :)

nikomatsakis · 2014-07-09T13:27:52Z

Hmm. It's possible @eddyb's solution of having variadic generics seems preferable to a family of unnamable traits. That is, it'd be a generic and reusable mechanism. Certainly worth talking through in more detail.

pcwalton · 2014-07-12T02:53:53Z

Getting closer here. One test failing. I had to rework most of the way unboxed closures work in the previous PRs. Still need to address one major issue in method resolution.

pcwalton · 2014-07-12T05:44:48Z

Tests oughta' be passing now.

pcwalton · 2014-07-17T01:20:05Z

This should be ready now; all of @nikomatsakis' comments addressed. r? @pnkfelix

sfackler · 2014-07-17T02:18:21Z

Tidy failures: https://travis-ci.org/rust-lang/rust/jobs/30130629

pcwalton · 2014-07-17T03:09:48Z

fixed tidy failures.

pnkfelix · 2014-07-17T04:39:54Z

src/test/run-pass/overloaded-calls-simple.rs

+    #[rust_call_abi_hack]
+    fn call_once(self, args: Args) -> Result;
+}
+*/


why did you add these bits of code in comments? e.g. was it to make it easier to convert the test into something that uses #[no_std] (so that you could feed it into a stage1 rustc) ? I would still be inclined to remove them...

Sorry about that, I forgot to remove it.

pnkfelix · 2014-07-17T05:22:41Z

@pcwalton LGTM, though my knowledge of trans is still pretty weak. I will try to at least give it a whirl locally, but I suspect that if you address the minor comments above, then it will be good to go.

pcwalton · 2014-07-17T22:07:15Z

re-r? @pnkfelix

pcwalton · 2014-07-17T22:07:20Z

Comments addressed.

pcwalton · 2014-07-18T05:04:23Z

Rebased.

@nikomatsakis

@nikomatsakis r?

bstrie reviewed May 30, 2014
View reviewed changes

huonw reviewed Jun 3, 2014
View reviewed changes

nikomatsakis reviewed Jun 10, 2014
View reviewed changes

pcwalton mentioned this pull request Jun 10, 2014

librustc: Fix the issue with labels shadowing variable names by making #14797

Closed

huonw mentioned this pull request Jun 30, 2014

Remove old (boxed) closures from the language #14798

Closed

jeremyletang mentioned this pull request Jul 10, 2014

Closures fundamentally broken jeremyletang/rgtk#28

Open

pnkfelix reviewed Jul 17, 2014
View reviewed changes

librustc: Implement unboxed closures with mutable receivers

02adaca

bors added a commit that referenced this pull request Jul 18, 2014

auto merge of #14539 : pcwalton/rust/unboxed-closures, r=pnkfelix

9c9bdfd

@nikomatsakis r?

bors closed this Jul 18, 2014

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unboxed closures #14539

Unboxed closures #14539

pcwalton commented May 30, 2014

bstrie May 30, 2014

bstrie May 30, 2014

thestinger May 30, 2014

pcwalton commented May 31, 2014

pcwalton commented May 31, 2014

alexcrichton commented May 31, 2014

pcwalton commented May 31, 2014

huonw Jun 3, 2014

pcwalton Jun 4, 2014

nikomatsakis commented Jun 7, 2014

nikomatsakis Jun 10, 2014

pnkfelix Jul 17, 2014

nikomatsakis commented Jun 10, 2014

alexcrichton commented Jun 30, 2014

pcwalton commented Jun 30, 2014

pcwalton commented Jul 9, 2014

nikomatsakis commented Jul 9, 2014

pcwalton commented Jul 12, 2014

pcwalton commented Jul 12, 2014

pcwalton commented Jul 17, 2014

sfackler commented Jul 17, 2014

pcwalton commented Jul 17, 2014

pnkfelix Jul 17, 2014

pcwalton Jul 17, 2014

pnkfelix commented Jul 17, 2014

pcwalton commented Jul 17, 2014

pcwalton commented Jul 17, 2014

pcwalton commented Jul 18, 2014

		@@ -1 +1 @@
		Subproject commit 0a894645cf120539876e9eb4eb0d7b572dfa9d14
		Subproject commit 4b4d0533b4f76cc3fbba31bd9e7ac02e0c738b1d

Unboxed closures #14539

Unboxed closures #14539

Conversation

pcwalton commented May 30, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pcwalton commented May 31, 2014

pcwalton commented May 31, 2014

alexcrichton commented May 31, 2014

pcwalton commented May 31, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Jun 7, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nikomatsakis commented Jun 10, 2014

alexcrichton commented Jun 30, 2014

pcwalton commented Jun 30, 2014

pcwalton commented Jul 9, 2014

nikomatsakis commented Jul 9, 2014

pcwalton commented Jul 12, 2014

pcwalton commented Jul 12, 2014

pcwalton commented Jul 17, 2014

sfackler commented Jul 17, 2014

pcwalton commented Jul 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnkfelix commented Jul 17, 2014

pcwalton commented Jul 17, 2014

pcwalton commented Jul 17, 2014

pcwalton commented Jul 18, 2014