Skip to content

Commit

Permalink
apply suggestions from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@nikomatsakis
nikomatsakis committed Nov 5, 2021
1 parent fc8113d commit 4154e8a
Show file tree
Hide file tree
Showing 10 changed files with 63 additions and 61 deletions.
40 changes: 21 additions & 19 deletions compiler/rustc_typeck/src/check/upvar.rs
View file
Expand Up @@ -87,9 +87,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
/// corresponding place being captured and a String which contains the captured value's
/// name (i.e: a.b.c)
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
enum CapturesInfo {
enum UpvarMigrationInfo {
/// We previously captured all of `x`, but now we capture some sub-path.
CapturingLess { source_expr: Option<hir::HirId>, var_name: String },
CapturingPrecise { source_expr: Option<hir::HirId>, var_name: String },
CapturingNothing {
// where the variable appears in the closure (but is not captured)
use_span: Span,
Expand Down Expand Up @@ -123,7 +123,7 @@ impl MigrationWarningReason {

/// Intermediate format to store information needed to generate a note in the migration lint.
struct MigrationLintNote {
captures_info: CapturesInfo,
captures_info: UpvarMigrationInfo,

/// reasons why migration is needed for this capture
reason: MigrationWarningReason,
Expand Down Expand Up @@ -751,14 +751,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// for migration being needed
for lint_note in diagnostics_info.iter() {
match &lint_note.captures_info {
CapturesInfo::CapturingLess { source_expr: Some(capture_expr_id), var_name: captured_name } => {
UpvarMigrationInfo::CapturingPrecise { source_expr: Some(capture_expr_id), var_name: captured_name } => {
let cause_span = self.tcx.hir().span(*capture_expr_id);
diagnostics_builder.span_label(cause_span, format!("in Rust 2018, this closure captures all of `{}`, but in Rust 2021, it will only capture `{}`",
self.tcx.hir().name(*var_hir_id),
captured_name,
));
}
CapturesInfo::CapturingNothing { use_span } => {
UpvarMigrationInfo::CapturingNothing { use_span } => {
diagnostics_builder.span_label(*use_span, format!("in Rust 2018, this causes the closure to capture `{}`, but in Rust 2021, it has no effect",
self.tcx.hir().name(*var_hir_id),
));
Expand All @@ -773,13 +773,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let drop_location_span = drop_location_span(self.tcx, &closure_hir_id);

match &lint_note.captures_info {
CapturesInfo::CapturingLess { var_name: captured_name, .. } => {
UpvarMigrationInfo::CapturingPrecise { var_name: captured_name, .. } => {
diagnostics_builder.span_label(drop_location_span, format!("in Rust 2018, `{}` is dropped here, but in Rust 2021, only `{}` will be dropped here as part of the closure",
self.tcx.hir().name(*var_hir_id),
captured_name,
));
}
CapturesInfo::CapturingNothing { use_span: _ } => {
UpvarMigrationInfo::CapturingNothing { use_span: _ } => {
diagnostics_builder.span_label(drop_location_span, format!("in Rust 2018, `{v}` is dropped here along with the closure, but in Rust 2021 `{v}` is not part of the closure",
v = self.tcx.hir().name(*var_hir_id),
));
Expand All @@ -791,16 +791,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
for &missing_trait in &lint_note.reason.auto_traits {
// not capturing something anymore cannot cause a trait to fail to be implemented:
match &lint_note.captures_info {
CapturesInfo::CapturingLess { var_name: captured_name, .. } => {
diagnostics_builder.span_label(closure_head_span, format!("in Rust 2018, this closure implements {missing_trait} as `{x}` implements {missing_trait}, but in Rust 2021, this closure will no longer implement {missing_trait} as `{p}` does not implement {missing_trait}",
missing_trait = missing_trait,
x = self.tcx.hir().name(*var_hir_id),
p = captured_name,
));
UpvarMigrationInfo::CapturingPrecise { var_name: captured_name, .. } => {
let var_name = self.tcx.hir().name(*var_hir_id);
diagnostics_builder.span_label(closure_head_span, format!("\
in Rust 2018, this closure implements {missing_trait} \
as `{var_name}` implements {missing_trait}, but in Rust 2021, \
this closure will no longer implement {missing_trait} \
because `{var_name}` is not fully captured \
and `{captured_name}` does not implement {missing_trait}"));
}

// Cannot happen: if we don't capture a variable, we impl strictly more traits
CapturesInfo::CapturingNothing { use_span } => span_bug!(*use_span, "missing trait from not capturing something"),
UpvarMigrationInfo::CapturingNothing { use_span } => span_bug!(*use_span, "missing trait from not capturing something"),
}
}
}
Expand Down Expand Up @@ -919,7 +921,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
min_captures: Option<&ty::RootVariableMinCaptureList<'tcx>>,
var_hir_id: hir::HirId,
closure_clause: hir::CaptureBy,
) -> Option<FxHashMap<CapturesInfo, FxHashSet<&'static str>>> {
) -> Option<FxHashMap<UpvarMigrationInfo, FxHashSet<&'static str>>> {
let auto_traits_def_id = vec![
self.tcx.lang_items().clone_trait(),
self.tcx.lang_items().sync_trait(),
Expand Down Expand Up @@ -1008,7 +1010,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {

if !capture_problems.is_empty() {
problematic_captures.insert(
CapturesInfo::CapturingLess {
UpvarMigrationInfo::CapturingPrecise {
source_expr: capture.info.path_expr_id,
var_name: capture.to_string(self.tcx),
},
Expand Down Expand Up @@ -1042,7 +1044,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
min_captures: Option<&ty::RootVariableMinCaptureList<'tcx>>,
closure_clause: hir::CaptureBy,
var_hir_id: hir::HirId,
) -> Option<FxHashSet<CapturesInfo>> {
) -> Option<FxHashSet<UpvarMigrationInfo>> {
let ty = self.infcx.resolve_vars_if_possible(self.node_ty(var_hir_id));

if !ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local())) {
Expand All @@ -1068,7 +1070,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
let mut diagnostics_info = FxHashSet::default();
let upvars = self.tcx.upvars_mentioned(closure_def_id).expect("must be an upvar");
let upvar = upvars[&var_hir_id];
diagnostics_info.insert(CapturesInfo::CapturingNothing { use_span: upvar.span });
diagnostics_info.insert(UpvarMigrationInfo::CapturingNothing { use_span: upvar.span });
return Some(diagnostics_info);
}
hir::CaptureBy::Ref => {}
Expand All @@ -1086,7 +1088,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
// Only care about captures that are moved into the closure
ty::UpvarCapture::ByValue(..) => {
projections_list.push(captured_place.place.projections.as_slice());
diagnostics_info.insert(CapturesInfo::CapturingLess {
diagnostics_info.insert(UpvarMigrationInfo::CapturingPrecise {
source_expr: captured_place.info.path_expr_id,
var_name: captured_place.to_string(self.tcx),
});
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed
View file
Expand Up @@ -21,7 +21,7 @@ fn test_send_trait() {
let fptr = SendPointer(&mut f as *mut i32);
thread::spawn(move || { let _ = &fptr; unsafe {
//~^ ERROR: changes to closure capture
//~| NOTE: in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` as `fptr.0` does not implement `Send`
//~| NOTE: in Rust 2018, this closure implements `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0 = 20;
Expand All @@ -41,8 +41,8 @@ fn test_sync_trait() {
let fptr = SyncPointer(f);
thread::spawn(move || { let _ = &fptr; unsafe {
//~^ ERROR: changes to closure capture
//~| NOTE: in Rust 2018, this closure implements `Sync` as `fptr` implements `Sync`, but in Rust 2021, this closure will no longer implement `Sync` as `fptr.0.0` does not implement `Sync`
//~| NOTE: in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` as `fptr.0.0` does not implement `Send`
//~| NOTE: in Rust 2018, this closure implements `Sync`
//~| NOTE: in Rust 2018, this closure implements `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0.0 = 20;
Expand All @@ -67,7 +67,7 @@ fn test_clone_trait() {
let c = || {
let _ = &f;
//~^ ERROR: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements
//~| NOTE: in Rust 2018, this closure implements `Clone` as `f` implements `Clone`, but in Rust 2021, this closure will no longer implement `Clone` as `f.1` does not implement `Clone`
//~| NOTE: in Rust 2018, this closure implements `Clone`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `f` to be fully captured
let f_1 = f.1;
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.rs
View file
Expand Up @@ -21,7 +21,7 @@ fn test_send_trait() {
let fptr = SendPointer(&mut f as *mut i32);
thread::spawn(move || unsafe {
//~^ ERROR: changes to closure capture
//~| NOTE: in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` as `fptr.0` does not implement `Send`
//~| NOTE: in Rust 2018, this closure implements `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0 = 20;
Expand All @@ -41,8 +41,8 @@ fn test_sync_trait() {
let fptr = SyncPointer(f);
thread::spawn(move || unsafe {
//~^ ERROR: changes to closure capture
//~| NOTE: in Rust 2018, this closure implements `Sync` as `fptr` implements `Sync`, but in Rust 2021, this closure will no longer implement `Sync` as `fptr.0.0` does not implement `Sync`
//~| NOTE: in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` as `fptr.0.0` does not implement `Send`
//~| NOTE: in Rust 2018, this closure implements `Sync`
//~| NOTE: in Rust 2018, this closure implements `Send`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `fptr` to be fully captured
*fptr.0.0 = 20;
Expand All @@ -66,7 +66,7 @@ fn test_clone_trait() {
let f = U(S(Foo(0)), T(0));
let c = || {
//~^ ERROR: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements
//~| NOTE: in Rust 2018, this closure implements `Clone` as `f` implements `Clone`, but in Rust 2021, this closure will no longer implement `Clone` as `f.1` does not implement `Clone`
//~| NOTE: in Rust 2018, this closure implements `Clone`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `f` to be fully captured
let f_1 = f.1;
Expand Down
8 changes: 4 additions & 4 deletions src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr
View file
Expand Up @@ -2,7 +2,7 @@ error: changes to closure capture in Rust 2021 will affect which traits the clos
--> $DIR/auto_traits.rs:22:19
|
LL | thread::spawn(move || unsafe {
| ^^^^^^^^^^^^^^ in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` as `fptr.0` does not implement `Send`
| ^^^^^^^^^^^^^^ in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` because `fptr` is not fully captured and `fptr.0` does not implement `Send`
...
LL | *fptr.0 = 20;
| ------- in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0`
Expand All @@ -29,8 +29,8 @@ error: changes to closure capture in Rust 2021 will affect which traits the clos
LL | thread::spawn(move || unsafe {
| ^^^^^^^^^^^^^^
| |
| in Rust 2018, this closure implements `Sync` as `fptr` implements `Sync`, but in Rust 2021, this closure will no longer implement `Sync` as `fptr.0.0` does not implement `Sync`
| in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` as `fptr.0.0` does not implement `Send`
| in Rust 2018, this closure implements `Sync` as `fptr` implements `Sync`, but in Rust 2021, this closure will no longer implement `Sync` because `fptr` is not fully captured and `fptr.0.0` does not implement `Sync`
| in Rust 2018, this closure implements `Send` as `fptr` implements `Send`, but in Rust 2021, this closure will no longer implement `Send` because `fptr` is not fully captured and `fptr.0.0` does not implement `Send`
...
LL | *fptr.0.0 = 20;
| --------- in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0.0`
Expand All @@ -50,7 +50,7 @@ error: changes to closure capture in Rust 2021 will affect drop order and which
--> $DIR/auto_traits.rs:67:13
|
LL | let c = || {
| ^^ in Rust 2018, this closure implements `Clone` as `f` implements `Clone`, but in Rust 2021, this closure will no longer implement `Clone` as `f.1` does not implement `Clone`
| ^^ in Rust 2018, this closure implements `Clone` as `f` implements `Clone`, but in Rust 2021, this closure will no longer implement `Clone` because `f` is not fully captured and `f.1` does not implement `Clone`
...
LL | let f_1 = f.1;
| --- in Rust 2018, this closure captures all of `f`, but in Rust 2021, it will only capture `f.1`
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.fixed
View file
Expand Up @@ -20,8 +20,8 @@ where
let result = panic::catch_unwind(move || {
let _ = &f;
//~^ ERROR: changes to closure capture in Rust 2021 will affect which traits the closure implements [rust_2021_incompatible_closure_captures]
//~| NOTE: in Rust 2018, this closure implements `UnwindSafe` as `f` implements `UnwindSafe`, but in Rust 2021, this closure will no longer implement `UnwindSafe` as `f.0` does not implement `UnwindSafe`
//~| NOTE: in Rust 2018, this closure implements `RefUnwindSafe` as `f` implements `RefUnwindSafe`, but in Rust 2021, this closure will no longer implement `RefUnwindSafe` as `f.0` does not implement `RefUnwindSafe`
//~| NOTE: in Rust 2018, this closure implements `UnwindSafe`
//~| NOTE: in Rust 2018, this closure implements `RefUnwindSafe`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `f` to be fully captured
f.0()
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.rs
View file
Expand Up @@ -19,8 +19,8 @@ where
let f = panic::AssertUnwindSafe(f);
let result = panic::catch_unwind(move || {
//~^ ERROR: changes to closure capture in Rust 2021 will affect which traits the closure implements [rust_2021_incompatible_closure_captures]
//~| NOTE: in Rust 2018, this closure implements `UnwindSafe` as `f` implements `UnwindSafe`, but in Rust 2021, this closure will no longer implement `UnwindSafe` as `f.0` does not implement `UnwindSafe`
//~| NOTE: in Rust 2018, this closure implements `RefUnwindSafe` as `f` implements `RefUnwindSafe`, but in Rust 2021, this closure will no longer implement `RefUnwindSafe` as `f.0` does not implement `RefUnwindSafe`
//~| NOTE: in Rust 2018, this closure implements `UnwindSafe`
//~| NOTE: in Rust 2018, this closure implements `RefUnwindSafe`
//~| NOTE: for more information, see
//~| HELP: add a dummy let to cause `f` to be fully captured
f.0()
Expand Down
4 changes: 2 additions & 2 deletions src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.stderr
View file
Expand Up @@ -4,8 +4,8 @@ error: changes to closure capture in Rust 2021 will affect which traits the clos
LL | let result = panic::catch_unwind(move || {
| ^^^^^^^
| |
| in Rust 2018, this closure implements `UnwindSafe` as `f` implements `UnwindSafe`, but in Rust 2021, this closure will no longer implement `UnwindSafe` as `f.0` does not implement `UnwindSafe`
| in Rust 2018, this closure implements `RefUnwindSafe` as `f` implements `RefUnwindSafe`, but in Rust 2021, this closure will no longer implement `RefUnwindSafe` as `f.0` does not implement `RefUnwindSafe`
| in Rust 2018, this closure implements `UnwindSafe` as `f` implements `UnwindSafe`, but in Rust 2021, this closure will no longer implement `UnwindSafe` because `f` is not fully captured and `f.0` does not implement `UnwindSafe`
| in Rust 2018, this closure implements `RefUnwindSafe` as `f` implements `RefUnwindSafe`, but in Rust 2021, this closure will no longer implement `RefUnwindSafe` because `f` is not fully captured and `f.0` does not implement `RefUnwindSafe`
...
LL | f.0()
| --- in Rust 2018, this closure captures all of `f`, but in Rust 2021, it will only capture `f.0`
Expand Down

0 comments on commit 4154e8a

Please sign in to comment.