Skip to content

/rust

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

Tracking issue for RFC 2229, "Closures Capture Disjoint Fields" #53488

Open
Centril opened this Issue Aug 19, 2018 · 2 comments

Comments

Assignees

@blitzerr blitzerr

Labels
B-RFC-approved C-tracking-issue T-lang
Projects
None yet
Milestone
No milestone
4 participants
@Centril @eddyb @samsartor @blitzerr
@Centril
Copy link
Contributor

Centril commented Aug 19, 2018 •
edited by blitzerr

This is a tracking issue for the RFC "Closures Capture Disjoint Fields" (rust-lang/rfcs#2229).

Steps:

Implementation Plan:

  • #56905 refactor the compiler to be less reliant on freevars, which are tied to the variables, and instead to use a map produced by the type-checker
  • #57482 Always make the "upvar types" a tuple of the actual upvar types

Unresolved questions:

  • How to optimize pointers. Can borrows that all reference parts of the same object be stored as a single pointer? How should this optimization be implemented (e.g. a special repr, refinement typing)?
  • How to signal that a function is pure. Is this even needed/wanted? Any other places where the language could benefit?
  • Should Box be special?
  • Drop order can change as a result of this RFC, is this a real stability problem? How should this be resolved?

@Centril Centril added B-RFC-approved T-lang C-tracking-issue labels Aug 19, 2018

@Centril Centril referenced this issue Aug 19, 2018

Merged

Closures Capture Disjoint Fields #2229

@eddyb

This comment has been minimized.

Sign in to view
Copy link
Member

eddyb commented Aug 19, 2018

IIRC, this needs to wait for the new MIR (NLL) borrow-checker to replace the old one.
cc @rust-lang/wg-compiler-nll
@samsartor

This comment has been minimized.

Sign in to view
Copy link

samsartor commented Aug 21, 2018

I would love to follow this through and give the code a shot, but it's probably too big of a first contribution to the compiler. Maybe I'll take a swing at some starter issues and then come back if no one else has gotten it done.

@eddyb Yes, this definitely should be built on NLL.

@blitzerr blitzerr self-assigned this Oct 29, 2018

blitzerr added a commit to blitzerr/rust that referenced this issue Nov 12, 2018

@blitzerr
capture_disjoint_fields(rust-lang#53488) 
1. Running RustFmt on collect.rs and upvar.rs
e535cd9

blitzerr added a commit to blitzerr/rust that referenced this issue Nov 14, 2018

@blitzerr
capture_disjoint_fields(rust-lang#53488) 
Just running RustFmt on upvar.rs
1ca505a

blitzerr added a commit to blitzerr/rust that referenced this issue Nov 15, 2018

@blitzerr
capture_disjoint_fields(rust-lang#53488) 
Refactoring out the HirId of the UpvarId in another struct.
6779bb4

pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 15, 2018

@pietroalbini
Rollup merge of rust-lang#55953 - blitzerr:master, r=nikomatsakis 
rust-lang#53488 Refactoring UpvarId
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
cacfc14

@pietroalbini pietroalbini referenced this issue Nov 15, 2018

Closed

Rollup of 15 pull requests #55990

pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 15, 2018

@pietroalbini
Rollup merge of rust-lang#55953 - blitzerr:master, r=nikomatsakis 
rust-lang#53488 Refactoring UpvarId
Verified
This commit was signed with a verified signature.
@pietroalbini pietroalbini Pietro Albini
GPG key ID: 3E06ABE80BAAF19C Learn about signing commits
3825dbf

bors added a commit that referenced this issue Nov 15, 2018

@bors
Auto merge of #55990 - pietroalbini:rollup, r=pietroalbini 
Rollup of 16 pull requests

Successful merges:

 - #55527 (Implement checked_add_duration for SystemTime)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55672 (miri: accept extern types in structs if they are the only field)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55867 (do not panic just because cargo failed)
 - #55871 (codegen_llvm_back: improve allocations)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor usfeful for mutation)
 - #55921 (Add placeholder types)
 - #55936 (save-analysis: be even more aggressive about ignorning macro-generated defs)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)

Failed merges:

r? @ghost
Loading status checks…
5b22e46

bors added a commit that referenced this issue Nov 16, 2018

@bors
Auto merge of #55990 - pietroalbini:rollup, r=pietroalbini 
Rollup of 16 pull requests

Successful merges:

 - #55527 (Implement checked_add_duration for SystemTime)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55672 (miri: accept extern types in structs if they are the only field)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55867 (do not panic just because cargo failed)
 - #55871 (codegen_llvm_back: improve allocations)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor usfeful for mutation)
 - #55921 (Add placeholder types)
 - #55936 (save-analysis: be even more aggressive about ignorning macro-generated defs)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)

Failed merges:

r? @ghost
Loading status checks…
a95f670

pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 16, 2018

@pietroalbini
Rollup merge of rust-lang#55953 - blitzerr:master, r=nikomatsakis 
rust-lang#53488 Refactoring UpvarId
Verified
This commit was signed with a verified signature.
@pietroalbini pietroalbini Pietro Albini
GPG key ID: 3E06ABE80BAAF19C Learn about signing commits
357031a

bors added a commit that referenced this issue Nov 16, 2018

@bors
Auto merge of #55990 - pietroalbini:rollup, r=pietroalbini 
Rollup of 15 pull requests

Successful merges:

 - #55527 (Implement checked_add_duration for SystemTime)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55672 (miri: accept extern types in structs if they are the only field)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55867 (do not panic just because cargo failed)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor usfeful for mutation)
 - #55921 (Add placeholder types)
 - #55936 (save-analysis: be even more aggressive about ignorning macro-generated defs)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)

Failed merges:

r? @ghost
Loading status checks…
ae83055

kennytm added a commit to kennytm/rust that referenced this issue Nov 17, 2018

@kennytm
Rollup merge of rust-lang#55953 - blitzerr:master, r=nikomatsakis 
rust-lang#53488 Refactoring UpvarId
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
1a662d2

@kennytm kennytm referenced this issue Nov 17, 2018

Closed

Rollup of 17 pull requests #56026

bors added a commit that referenced this issue Nov 17, 2018

@bors
Auto merge of #56026 - kennytm:rollup, r=kennytm 
Rollup of 17 pull requests

Successful merges:

 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55672 (miri: accept extern types in structs if they are the only field)
 - #55767 (Disable some pretty-printers when gdb is rust-enabled)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55857 (remove unused dependency)
 - #55867 (do not panic just because cargo failed)
 - #55916 (Make miri value visitor usfeful for mutation)
 - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)
 - #55999 (Update Cargo submodule)
 - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.)
 - #56012 (avoid shared ref in UnsafeCell::get)
 - #56017 (std: Add debugging for a failing test on appveyor)

Failed merges:

 - #56010 (fix intra-link resolution spans in block comments)

r? @ghost
Loading status checks…
ce79ba5

pietroalbini added a commit to pietroalbini/rust that referenced this issue Nov 18, 2018

@pietroalbini
Rollup merge of rust-lang#55953 - blitzerr:master, r=nikomatsakis 
rust-lang#53488 Refactoring UpvarId
Verified
This commit was created on GitHub.com and signed with a verified signature using GitHub’s key.
GPG key ID: 4AEE18F83AFDEB23 Learn about signing commits
7869aeb

@pietroalbini pietroalbini referenced this issue Nov 18, 2018

Merged

Rollup of 25 pull requests #56051

bors added a commit that referenced this issue Nov 19, 2018

@bors
Auto merge of #56051 - pietroalbini:rollup, r=pietroalbini 
Rollup of 25 pull requests

Successful merges:

 - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55857 (remove unused dependency)
 - #55862 (in which the E0618 "expected function" diagnostic gets a makeover)
 - #55867 (do not panic just because cargo failed)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor usfeful for mutation)
 - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code)
 - #55923 (reword #[test] attribute error on fn items)
 - #55935 (appveyor: Use VS2017 for all our images)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)
 - #55968 (Clean up some non-mod-rs stuff.)
 - #55970 (Miri backtrace improvements)
 - #56007 (CTFE: dynamically make sure we do not call non-const-fn)
 - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.)
 - #56012 (avoid shared ref in UnsafeCell::get)
 - #56016 (Add VecDeque::resize_with)
 - #56027 (docs: Add missing backtick in object_safety.rs docs)
 - #56043 (remove "approx env bounds" if we already know from trait)

Failed merges:

r? @ghost
Loading status checks…
fba315e

bors added a commit that referenced this issue Nov 19, 2018

@bors
Auto merge of #56051 - pietroalbini:rollup, r=pietroalbini 
Rollup of 25 pull requests

Successful merges:

 - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55857 (remove unused dependency)
 - #55862 (in which the E0618 "expected function" diagnostic gets a makeover)
 - #55867 (do not panic just because cargo failed)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor usfeful for mutation)
 - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code)
 - #55923 (reword #[test] attribute error on fn items)
 - #55935 (appveyor: Use VS2017 for all our images)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)
 - #55968 (Clean up some non-mod-rs stuff.)
 - #55970 (Miri backtrace improvements)
 - #56007 (CTFE: dynamically make sure we do not call non-const-fn)
 - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.)
 - #56012 (avoid shared ref in UnsafeCell::get)
 - #56016 (Add VecDeque::resize_with)
 - #56027 (docs: Add missing backtick in object_safety.rs docs)
 - #56043 (remove "approx env bounds" if we already know from trait)

Failed merges:

r? @ghost
Loading status checks…
d87625b

bors added a commit that referenced this issue Nov 19, 2018

@bors
Auto merge of #56051 - pietroalbini:rollup, r=pietroalbini 
Rollup of 26 pull requests

Successful merges:

 - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55857 (remove unused dependency)
 - #55862 (in which the E0618 "expected function" diagnostic gets a makeover)
 - #55867 (do not panic just because cargo failed)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor useful for mutation)
 - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code)
 - #55923 (reword #[test] attribute error on fn items)
 - #55935 (appveyor: Use VS2017 for all our images)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)
 - #55968 (Clean up some non-mod-rs stuff.)
 - #55970 (Miri backtrace improvements)
 - #56007 (CTFE: dynamically make sure we do not call non-const-fn)
 - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.)
 - #56012 (avoid shared ref in UnsafeCell::get)
 - #56016 (Add VecDeque::resize_with)
 - #56027 (docs: Add missing backtick in object_safety.rs docs)
 - #56043 (remove "approx env bounds" if we already know from trait)
 - #56059 (Increase `Duration` approximate equal threshold to 1us)
Loading status checks…
9f0370d

bors added a commit that referenced this issue Nov 19, 2018

@bors
Auto merge of #56051 - pietroalbini:rollup, r=pietroalbini 
Rollup of 26 pull requests

Successful merges:

 - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55857 (remove unused dependency)
 - #55862 (in which the E0618 "expected function" diagnostic gets a makeover)
 - #55867 (do not panic just because cargo failed)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor useful for mutation)
 - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code)
 - #55923 (reword #[test] attribute error on fn items)
 - #55935 (appveyor: Use VS2017 for all our images)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)
 - #55968 (Clean up some non-mod-rs stuff.)
 - #55970 (Miri backtrace improvements)
 - #56007 (CTFE: dynamically make sure we do not call non-const-fn)
 - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.)
 - #56012 (avoid shared ref in UnsafeCell::get)
 - #56016 (Add VecDeque::resize_with)
 - #56027 (docs: Add missing backtick in object_safety.rs docs)
 - #56043 (remove "approx env bounds" if we already know from trait)
 - #56059 (Increase `Duration` approximate equal threshold to 1us)
Loading status checks…
f172731

kennytm added a commit to pietroalbini/rust that referenced this issue Nov 19, 2018

@pietroalbini @kennytm
Rollup merge of rust-lang#55953 - blitzerr:master, r=nikomatsakis 
rust-lang#53488 Refactoring UpvarId
Verified
This commit was signed with a verified signature.
@kennytm kennytm
GPG key ID: FEF6C8051D0E013C Learn about signing commits
989d06a

bors added a commit that referenced this issue Nov 19, 2018

@bors
Auto merge of #56051 - pietroalbini:rollup, r=pietroalbini 
Rollup of 25 pull requests

Successful merges:

 - #55562 (Add powerpc- and powerpc64-unknown-linux-musl targets)
 - #55564 (test/linkage-visibility: Ignore on musl targets)
 - #55827 (A few tweaks to iterations/collecting)
 - #55834 (Forward the ABI of the non-zero sized fields of an union if they have the same ABI)
 - #55857 (remove unused dependency)
 - #55862 (in which the E0618 "expected function" diagnostic gets a makeover)
 - #55867 (do not panic just because cargo failed)
 - #55894 (miri enum discriminant handling: Fix treatment of pointers, better error when it is undef)
 - #55916 (Make miri value visitor useful for mutation)
 - #55919 (core/tests/num: Simplify `test_int_from_str_overflow()` test code)
 - #55923 (reword #[test] attribute error on fn items)
 - #55949 (ty: return impl Iterator from Predicate::walk_tys)
 - #55952 (Update to Clang 7 on CI.)
 - #55953 (#53488 Refactoring UpvarId)
 - #55962 (rustdoc: properly calculate spans for intra-doc link resolution errors)
 - #55963 (Stress test for MPSC)
 - #55968 (Clean up some non-mod-rs stuff.)
 - #55970 (Miri backtrace improvements)
 - #56007 (CTFE: dynamically make sure we do not call non-const-fn)
 - #56011 (Replace data.clone() by Arc::clone(&data) in mutex doc.)
 - #56012 (avoid shared ref in UnsafeCell::get)
 - #56016 (Add VecDeque::resize_with)
 - #56027 (docs: Add missing backtick in object_safety.rs docs)
 - #56043 (remove "approx env bounds" if we already know from trait)
 - #56059 (Increase `Duration` approximate equal threshold to 1us)
Loading status checks…
9e8a982

blitzerr added a commit to blitzerr/rust that referenced this issue Dec 29, 2018

@blitzerr
[Cleanup] This is the first in the series of removals of with_freevar… 
…s usage.

Currently, there are many places in rustc, where we use
with_freevars to iterate over freevars of a closure. The
problem with this is the argument to with_freevars is a
NodeId and this will get in the way of our eventual goal
of solving for issue (rust-lang#53488).
2dd6910

blitzerr added a commit to blitzerr/rust that referenced this issue Jan 3, 2019

@blitzerr
[Cleanup] This is the first in the series of removals of with_freevar… 
…s usage.

Currently, there are many places in rustc, where we use
with_freevars to iterate over freevars of a closure. The
problem with this is the argument to with_freevars is a
NodeId and this will get in the way of our eventual goal
of solving for issue (rust-lang#53488), sub-issue (rust-lang#56905)
8bb6411

blitzerr added a commit to blitzerr/rust that referenced this issue Jan 3, 2019

@blitzerr
[Cleanup] This is the first in the series of removals of with_freevar… 
…s usage.

Currently, there are many places in rustc, where we use
with_freevars to iterate over freevars of a closure. The
problem with this is the argument to with_freevars is a
NodeId and this will get in the way of our eventual goal
of solving for issue (rust-lang#53488), sub-issue (rust-lang#56905)
923042e

blitzerr added a commit to blitzerr/rust that referenced this issue Jan 9, 2019

@blitzerr
[Cleanup] This is the first in the series of removals of with_freevar… 
…s usage.

Currently, there are many places in rustc, where we use
with_freevars to iterate over freevars of a closure. The
problem with this is the argument to with_freevars is a
NodeId and this will get in the way of our eventual goal
of solving for issue (rust-lang#53488), sub-issue (rust-lang#56905)
15d8e8f

@jonas-schievink jonas-schievink referenced this issue Jan 10, 2019

Closed

Polonius borrow checker regression in recent nightlies #57487

This was referenced Jan 10, 2019

Open
Closed

blitzerr added a commit to blitzerr/rust that referenced this issue Jan 13, 2019

@blitzerr
Replacing the slice of upvar type parameters with tuple for closure s… 
…ubstitutions.

Related to sub issue rust-lang#57482, main issue rust-lang#53488.
e9faf40

blitzerr added a commit to blitzerr/rust that referenced this issue Jan 13, 2019

@blitzerr
Replacing the slice of upvar type parameters with tuple for Generator… 
… substitutions.

Related to sub issue rust-lang#57482, main issue rust-lang#53488.
7fa36f4

blitzerr added a commit to blitzerr/rust that referenced this issue Jan 13, 2019

@blitzerr
Make the ppaux code to print error and move on if the upvar types are… 
… not inferred

It was safe for the ppaux code to call the upvar_tys but not anymore
after the change wraping the upvar types in a tuple as this can panic
if called from parts when the types are yet to be bound. This patch
therefore creates a helper, that returns an Option and ppaux can call
the new helper instead.
Main issue rust-lang#53488, sub-issue rust-lang#57482
8b32d92

@oleganza oleganza referenced this issue Mar 19, 2019

Merged

MuSig: add counterparties #229

3 of 3 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.