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

Edition 2021 enables disjoint capture #86536

Merged
merged 1 commit into from
Jun 24, 2021
Merged

Conversation

arora-aman
Copy link
Member

@rust-highfive
Copy link
Collaborator

r? @jackh726

(rust-highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 22, 2021
@arora-aman
Copy link
Member Author

r? @nikomatsakis

@rust-log-analyzer

This comment has been minimized.

/// Precise capture is enabled if the feature gate `capture_disjoint_fields` is enabled or if
/// user is using Rust Edition 2021 or higher
fn enable_precise_capture(tcx: TyCtxt<'_>) -> bool {
tcx.features().capture_disjoint_fields || tcx.sess.rust_2021()
Copy link
Contributor

Choose a reason for hiding this comment

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

Actually we should be using span.rust_2021(), so this function needs to take a Span argument. We really want, I think, the span of the "closure header" (the |...| part), but I think if we just pass in the span of the closure overall, that will do.

The reason for tying the edition to the span is because macros may inject code from other editions. We should probably write some tests for that, too.

Copy link
Member

Choose a reason for hiding this comment

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

The span of |..| body is made with lo.to(body.span), where lo is the span of the first token (move or async or static or || or |). The Span::to function in most cases carries the edition of the left hand side, but the code has multiple FIXMEs and the behaviour when macros and multiple contexts are involved is not very obvious at all:

pub fn to(self, end: Span) -> Span {
let span_data = self.data();
let end_data = end.data();
// FIXME(jseyfried): `self.ctxt` should always equal `end.ctxt` here (cf. issue #23480).
// Return the macro span on its own to avoid weird diagnostic output. It is preferable to
// have an incomplete span than a completely nonsensical one.
if span_data.ctxt != end_data.ctxt {
if span_data.ctxt == SyntaxContext::root() {
return end;
} else if end_data.ctxt == SyntaxContext::root() {
return self;
}
// Both spans fall within a macro.
// FIXME(estebank): check if it is the *same* macro.
}

Copy link
Contributor

Choose a reason for hiding this comment

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

Good enough for now

@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented Jun 23, 2021

📌 Commit f265997 has been approved by nikomatsakis

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jun 23, 2021
bors added a commit to rust-lang-ci/rust that referenced this pull request Jun 24, 2021
Rollup of 8 pull requests

Successful merges:

 - rust-lang#86137 (Error code cleanup and enforce checks)
 - rust-lang#86296 (Add documentation for various THIR structs)
 - rust-lang#86415 (Document associativity of iterator folds.)
 - rust-lang#86533 (Support lowercase error codes in `--explain`)
 - rust-lang#86536 (Edition 2021 enables disjoint capture)
 - rust-lang#86560 (Update cargo)
 - rust-lang#86561 (chore(rustdoc): Remove unused impl block)
 - rust-lang#86566 (Use `use_verbose` for `mir::Constant`)

Failed merges:

r? `@ghost`
`@rustbot` modify labels: rollup
@bors bors merged commit 3998c03 into rust-lang:master Jun 24, 2021
@rustbot rustbot added this to the 1.55.0 milestone Jun 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants