Skip to content

Commit 4154e8a

Browse files
nikomatsakisnikomatsakis
committed
apply suggestions from code review
1 parent fc8113d commit 4154e8a

File tree

10 files changed

+63
-61
lines changed

10 files changed

+63
-61
lines changed

‎compiler/rustc_typeck/src/check/upvar.rs

+21-19
Original file line numberDiff line numberDiff line change
@@ -87,9 +87,9 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
8787
/// corresponding place being captured and a String which contains the captured value's
8888
/// name (i.e: a.b.c)
8989
#[derive(Clone, Debug, PartialEq, Eq, PartialOrd, Ord, Hash)]
90-
enum CapturesInfo {
90+
enum UpvarMigrationInfo {
9191
/// We previously captured all of `x`, but now we capture some sub-path.
92-
CapturingLess { source_expr: Option<hir::HirId>, var_name: String },
92+
CapturingPrecise { source_expr: Option<hir::HirId>, var_name: String },
9393
CapturingNothing {
9494
// where the variable appears in the closure (but is not captured)
9595
use_span: Span,
@@ -123,7 +123,7 @@ impl MigrationWarningReason {
123123

124124
/// Intermediate format to store information needed to generate a note in the migration lint.
125125
struct MigrationLintNote {
126-
captures_info: CapturesInfo,
126+
captures_info: UpvarMigrationInfo,
127127

128128
/// reasons why migration is needed for this capture
129129
reason: MigrationWarningReason,
@@ -751,14 +751,14 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
751751
// for migration being needed
752752
for lint_note in diagnostics_info.iter() {
753753
match &lint_note.captures_info {
754-
CapturesInfo::CapturingLess { source_expr: Some(capture_expr_id), var_name: captured_name } => {
754+
UpvarMigrationInfo::CapturingPrecise { source_expr: Some(capture_expr_id), var_name: captured_name } => {
755755
let cause_span = self.tcx.hir().span(*capture_expr_id);
756756
diagnostics_builder.span_label(cause_span, format!("in Rust 2018, this closure captures all of `{}`, but in Rust 2021, it will only capture `{}`",
757757
self.tcx.hir().name(*var_hir_id),
758758
captured_name,
759759
));
760760
}
761-
CapturesInfo::CapturingNothing { use_span } => {
761+
UpvarMigrationInfo::CapturingNothing { use_span } => {
762762
diagnostics_builder.span_label(*use_span, format!("in Rust 2018, this causes the closure to capture `{}`, but in Rust 2021, it has no effect",
763763
self.tcx.hir().name(*var_hir_id),
764764
));
@@ -773,13 +773,13 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
773773
let drop_location_span = drop_location_span(self.tcx, &closure_hir_id);
774774

775775
match &lint_note.captures_info {
776-
CapturesInfo::CapturingLess { var_name: captured_name, .. } => {
776+
UpvarMigrationInfo::CapturingPrecise { var_name: captured_name, .. } => {
777777
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",
778778
self.tcx.hir().name(*var_hir_id),
779779
captured_name,
780780
));
781781
}
782-
CapturesInfo::CapturingNothing { use_span: _ } => {
782+
UpvarMigrationInfo::CapturingNothing { use_span: _ } => {
783783
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",
784784
v = self.tcx.hir().name(*var_hir_id),
785785
));
@@ -791,16 +791,18 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
791791
for &missing_trait in &lint_note.reason.auto_traits {
792792
// not capturing something anymore cannot cause a trait to fail to be implemented:
793793
match &lint_note.captures_info {
794-
CapturesInfo::CapturingLess { var_name: captured_name, .. } => {
795-
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}",
796-
missing_trait = missing_trait,
797-
x = self.tcx.hir().name(*var_hir_id),
798-
p = captured_name,
799-
));
794+
UpvarMigrationInfo::CapturingPrecise { var_name: captured_name, .. } => {
795+
let var_name = self.tcx.hir().name(*var_hir_id);
796+
diagnostics_builder.span_label(closure_head_span, format!("\
797+
in Rust 2018, this closure implements {missing_trait} \
798+
as `{var_name}` implements {missing_trait}, but in Rust 2021, \
799+
this closure will no longer implement {missing_trait} \
800+
because `{var_name}` is not fully captured \
801+
and `{captured_name}` does not implement {missing_trait}"));
800802
}
801803

802804
// Cannot happen: if we don't capture a variable, we impl strictly more traits
803-
CapturesInfo::CapturingNothing { use_span } => span_bug!(*use_span, "missing trait from not capturing something"),
805+
UpvarMigrationInfo::CapturingNothing { use_span } => span_bug!(*use_span, "missing trait from not capturing something"),
804806
}
805807
}
806808
}
@@ -919,7 +921,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
919921
min_captures: Option<&ty::RootVariableMinCaptureList<'tcx>>,
920922
var_hir_id: hir::HirId,
921923
closure_clause: hir::CaptureBy,
922-
) -> Option<FxHashMap<CapturesInfo, FxHashSet<&'static str>>> {
924+
) -> Option<FxHashMap<UpvarMigrationInfo, FxHashSet<&'static str>>> {
923925
let auto_traits_def_id = vec![
924926
self.tcx.lang_items().clone_trait(),
925927
self.tcx.lang_items().sync_trait(),
@@ -1008,7 +1010,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10081010

10091011
if !capture_problems.is_empty() {
10101012
problematic_captures.insert(
1011-
CapturesInfo::CapturingLess {
1013+
UpvarMigrationInfo::CapturingPrecise {
10121014
source_expr: capture.info.path_expr_id,
10131015
var_name: capture.to_string(self.tcx),
10141016
},
@@ -1042,7 +1044,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10421044
min_captures: Option<&ty::RootVariableMinCaptureList<'tcx>>,
10431045
closure_clause: hir::CaptureBy,
10441046
var_hir_id: hir::HirId,
1045-
) -> Option<FxHashSet<CapturesInfo>> {
1047+
) -> Option<FxHashSet<UpvarMigrationInfo>> {
10461048
let ty = self.infcx.resolve_vars_if_possible(self.node_ty(var_hir_id));
10471049

10481050
if !ty.has_significant_drop(self.tcx, self.tcx.param_env(closure_def_id.expect_local())) {
@@ -1068,7 +1070,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10681070
let mut diagnostics_info = FxHashSet::default();
10691071
let upvars = self.tcx.upvars_mentioned(closure_def_id).expect("must be an upvar");
10701072
let upvar = upvars[&var_hir_id];
1071-
diagnostics_info.insert(CapturesInfo::CapturingNothing { use_span: upvar.span });
1073+
diagnostics_info.insert(UpvarMigrationInfo::CapturingNothing { use_span: upvar.span });
10721074
return Some(diagnostics_info);
10731075
}
10741076
hir::CaptureBy::Ref => {}
@@ -1086,7 +1088,7 @@ impl<'a, 'tcx> FnCtxt<'a, 'tcx> {
10861088
// Only care about captures that are moved into the closure
10871089
ty::UpvarCapture::ByValue(..) => {
10881090
projections_list.push(captured_place.place.projections.as_slice());
1089-
diagnostics_info.insert(CapturesInfo::CapturingLess {
1091+
diagnostics_info.insert(UpvarMigrationInfo::CapturingPrecise {
10901092
source_expr: captured_place.info.path_expr_id,
10911093
var_name: captured_place.to_string(self.tcx),
10921094
});

‎src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.fixed

+4-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ fn test_send_trait() {
2121
let fptr = SendPointer(&mut f as *mut i32);
2222
thread::spawn(move || { let _ = &fptr; unsafe {
2323
//~^ ERROR: changes to closure capture
24-
//~| 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`
24+
//~| NOTE: in Rust 2018, this closure implements `Send`
2525
//~| NOTE: for more information, see
2626
//~| HELP: add a dummy let to cause `fptr` to be fully captured
2727
*fptr.0 = 20;
@@ -41,8 +41,8 @@ fn test_sync_trait() {
4141
let fptr = SyncPointer(f);
4242
thread::spawn(move || { let _ = &fptr; unsafe {
4343
//~^ ERROR: changes to closure capture
44-
//~| 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`
45-
//~| 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`
44+
//~| NOTE: in Rust 2018, this closure implements `Sync`
45+
//~| NOTE: in Rust 2018, this closure implements `Send`
4646
//~| NOTE: for more information, see
4747
//~| HELP: add a dummy let to cause `fptr` to be fully captured
4848
*fptr.0.0 = 20;
@@ -67,7 +67,7 @@ fn test_clone_trait() {
6767
let c = || {
6868
let _ = &f;
6969
//~^ ERROR: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements
70-
//~| 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`
70+
//~| NOTE: in Rust 2018, this closure implements `Clone`
7171
//~| NOTE: for more information, see
7272
//~| HELP: add a dummy let to cause `f` to be fully captured
7373
let f_1 = f.1;

‎src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ fn test_send_trait() {
2121
let fptr = SendPointer(&mut f as *mut i32);
2222
thread::spawn(move || unsafe {
2323
//~^ ERROR: changes to closure capture
24-
//~| 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`
24+
//~| NOTE: in Rust 2018, this closure implements `Send`
2525
//~| NOTE: for more information, see
2626
//~| HELP: add a dummy let to cause `fptr` to be fully captured
2727
*fptr.0 = 20;
@@ -41,8 +41,8 @@ fn test_sync_trait() {
4141
let fptr = SyncPointer(f);
4242
thread::spawn(move || unsafe {
4343
//~^ ERROR: changes to closure capture
44-
//~| 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`
45-
//~| 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`
44+
//~| NOTE: in Rust 2018, this closure implements `Sync`
45+
//~| NOTE: in Rust 2018, this closure implements `Send`
4646
//~| NOTE: for more information, see
4747
//~| HELP: add a dummy let to cause `fptr` to be fully captured
4848
*fptr.0.0 = 20;
@@ -66,7 +66,7 @@ fn test_clone_trait() {
6666
let f = U(S(Foo(0)), T(0));
6767
let c = || {
6868
//~^ ERROR: changes to closure capture in Rust 2021 will affect drop order and which traits the closure implements
69-
//~| 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`
69+
//~| NOTE: in Rust 2018, this closure implements `Clone`
7070
//~| NOTE: for more information, see
7171
//~| HELP: add a dummy let to cause `f` to be fully captured
7272
let f_1 = f.1;

‎src/test/ui/closures/2229_closure_analysis/migrations/auto_traits.stderr

+4-4
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ error: changes to closure capture in Rust 2021 will affect which traits the clos
22
--> $DIR/auto_traits.rs:22:19
33
|
44
LL | thread::spawn(move || unsafe {
5-
| ^^^^^^^^^^^^^^ 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`
5+
| ^^^^^^^^^^^^^^ 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`
66
...
77
LL | *fptr.0 = 20;
88
| ------- in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0`
@@ -29,8 +29,8 @@ error: changes to closure capture in Rust 2021 will affect which traits the clos
2929
LL | thread::spawn(move || unsafe {
3030
| ^^^^^^^^^^^^^^
3131
| |
32-
| 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`
33-
| 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`
32+
| 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`
33+
| 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`
3434
...
3535
LL | *fptr.0.0 = 20;
3636
| --------- in Rust 2018, this closure captures all of `fptr`, but in Rust 2021, it will only capture `fptr.0.0`
@@ -50,7 +50,7 @@ error: changes to closure capture in Rust 2021 will affect drop order and which
5050
--> $DIR/auto_traits.rs:67:13
5151
|
5252
LL | let c = || {
53-
| ^^ 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`
53+
| ^^ 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`
5454
...
5555
LL | let f_1 = f.1;
5656
| --- in Rust 2018, this closure captures all of `f`, but in Rust 2021, it will only capture `f.1`

‎src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.fixed

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,8 @@ where
2020
let result = panic::catch_unwind(move || {
2121
let _ = &f;
2222
//~^ ERROR: changes to closure capture in Rust 2021 will affect which traits the closure implements [rust_2021_incompatible_closure_captures]
23-
//~| 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`
24-
//~| 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`
23+
//~| NOTE: in Rust 2018, this closure implements `UnwindSafe`
24+
//~| NOTE: in Rust 2018, this closure implements `RefUnwindSafe`
2525
//~| NOTE: for more information, see
2626
//~| HELP: add a dummy let to cause `f` to be fully captured
2727
f.0()

‎src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ where
1919
let f = panic::AssertUnwindSafe(f);
2020
let result = panic::catch_unwind(move || {
2121
//~^ ERROR: changes to closure capture in Rust 2021 will affect which traits the closure implements [rust_2021_incompatible_closure_captures]
22-
//~| 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`
23-
//~| 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`
22+
//~| NOTE: in Rust 2018, this closure implements `UnwindSafe`
23+
//~| NOTE: in Rust 2018, this closure implements `RefUnwindSafe`
2424
//~| NOTE: for more information, see
2525
//~| HELP: add a dummy let to cause `f` to be fully captured
2626
f.0()

‎src/test/ui/closures/2229_closure_analysis/migrations/mir_calls_to_shims.stderr

+2-2
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@ error: changes to closure capture in Rust 2021 will affect which traits the clos
44
LL | let result = panic::catch_unwind(move || {
55
| ^^^^^^^
66
| |
7-
| 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`
8-
| 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`
7+
| 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`
8+
| 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`
99
...
1010
LL | f.0()
1111
| --- in Rust 2018, this closure captures all of `f`, but in Rust 2021, it will only capture `f.0`

0 commit comments

Comments
 (0)