Skip to content

Commit 2707f55

Browse files
committed
Arbitrary self types v2: deshadowing probe
This is the first part of a series of commits which impact the "deshadowing detection" in the arbitrary self types v2 RFC. This commit should not have any functional changes, but may impact performance. Subsequent commits add back the performance, and add error checking to this new code such that it has a functional effect. Rust prioritizes method candidates in this order: 1. By value; 2. By reference; 3. By mutable reference; 4. By const ptr. 5. By reborrowed pin. Previously, if a suitable candidate was found in one of these earlier categories, Rust wouldn't even move onto probing the other categories. As part of the arbitrary self types work, we're going to need to change that - even if we choose a method from one of the earlier categories, we will sometimes need to probe later categories to search for methods that we may be shadowing. This commit adds those extra searches for shadowing, but it does not yet produce an error when such shadowing problems are found. That will come in a subsequent commit, by filling out the 'check_for_shadowing' method. This commit contains a naive approach to detecting these shadowing problems, which shows what we've functionally looking to do. However, it's too slow. The performance of this approach was explored in this PR: rust-lang#127812 (comment) Subsequent commits will improve the speed of the search.
1 parent 7f7c964 commit 2707f55

File tree

1 file changed

+114
-10
lines changed
  • compiler/rustc_hir_typeck/src/method

1 file changed

+114
-10
lines changed

Diff for: compiler/rustc_hir_typeck/src/method/probe.rs

+114-10
Original file line numberDiff line numberDiff line change
@@ -1144,6 +1144,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
11441144
&self,
11451145
pick_diag_hints: &mut PickDiagHints<'b, 'tcx>,
11461146
) -> Option<PickResult<'tcx>> {
1147+
let track_unstable_candidates = pick_diag_hints.unstable_candidates.is_some();
11471148
self.steps
11481149
.iter()
11491150
// At this point we're considering the types to which the receiver can be converted,
@@ -1167,22 +1168,125 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
11671168
.unwrap_or_else(|_| {
11681169
span_bug!(self.span, "{:?} was applicable but now isn't?", step.self_ty)
11691170
});
1170-
self.pick_by_value_method(step, self_ty, pick_diag_hints).or_else(|| {
1171-
self.pick_autorefd_method(step, self_ty, hir::Mutability::Not, pick_diag_hints)
1172-
.or_else(|| {
1173-
self.pick_autorefd_method(
1171+
1172+
let by_value_pick = self.pick_by_value_method(step, self_ty, pick_diag_hints);
1173+
1174+
// Check for shadowing of a by-reference method by a by-value method (see comments on check_for_shadowing)
1175+
if let Some(by_value_pick) = by_value_pick {
1176+
if let Ok(by_value_pick) = by_value_pick.as_ref() {
1177+
if by_value_pick.kind == PickKind::InherentImplPick {
1178+
if let Err(e) = self.check_for_shadowed_autorefd_method(
1179+
by_value_pick,
1180+
step,
1181+
self_ty,
1182+
hir::Mutability::Not,
1183+
track_unstable_candidates,
1184+
) {
1185+
return Some(Err(e));
1186+
}
1187+
if let Err(e) = self.check_for_shadowed_autorefd_method(
1188+
by_value_pick,
11741189
step,
11751190
self_ty,
11761191
hir::Mutability::Mut,
1177-
pick_diag_hints,
1178-
)
1179-
})
1180-
.or_else(|| self.pick_const_ptr_method(step, self_ty, pick_diag_hints))
1181-
.or_else(|| self.pick_reborrow_pin_method(step, self_ty, pick_diag_hints))
1182-
})
1192+
track_unstable_candidates,
1193+
) {
1194+
return Some(Err(e));
1195+
}
1196+
}
1197+
}
1198+
return Some(by_value_pick);
1199+
}
1200+
1201+
let autoref_pick =
1202+
self.pick_autorefd_method(step, self_ty, hir::Mutability::Not, pick_diag_hints);
1203+
// Check for shadowing of a by-mut-ref method by a by-reference method (see comments on check_for_shadowing)
1204+
if let Some(autoref_pick) = autoref_pick {
1205+
if let Ok(autoref_pick) = autoref_pick.as_ref() {
1206+
// Check we're not shadowing others
1207+
if autoref_pick.kind == PickKind::InherentImplPick {
1208+
if let Err(e) = self.check_for_shadowed_autorefd_method(
1209+
autoref_pick,
1210+
step,
1211+
self_ty,
1212+
hir::Mutability::Mut,
1213+
track_unstable_candidates,
1214+
) {
1215+
return Some(Err(e));
1216+
}
1217+
}
1218+
}
1219+
return Some(autoref_pick);
1220+
}
1221+
1222+
// Note that no shadowing errors are produced from here on,
1223+
// as we consider const ptr methods.
1224+
// We allow new methods that take *mut T to shadow
1225+
// methods which took *const T, so there is no entry in
1226+
// this list for the results of `pick_const_ptr_method`.
1227+
// The reason is that the standard pointer cast method
1228+
// (on a mutable pointer) always already shadows the
1229+
// cast method (on a const pointer). So, if we added
1230+
// `pick_const_ptr_method` to this method, the anti-
1231+
// shadowing algorithm would always complain about
1232+
// the conflict between *const::cast and *mut::cast.
1233+
// In practice therefore this does constrain us:
1234+
// we cannot add new
1235+
// self: *mut Self
1236+
// methods to types such as NonNull or anything else
1237+
// which implements Receiver, because this might in future
1238+
// shadow existing methods taking
1239+
// self: *const NonNull<Self>
1240+
// in the pointee. In practice, methods taking raw pointers
1241+
// are rare, and it seems that it should be easily possible
1242+
// to avoid such compatibility breaks.
1243+
// We also don't check for reborrowed pin methods which
1244+
// may be shadowed; these also seem unlikely to occur.
1245+
self.pick_autorefd_method(step, self_ty, hir::Mutability::Mut, pick_diag_hints)
1246+
.or_else(|| self.pick_const_ptr_method(step, self_ty, pick_diag_hints))
1247+
.or_else(|| self.pick_reborrow_pin_method(step, self_ty, pick_diag_hints))
11831248
})
11841249
}
11851250

1251+
/// Check for cases where arbitrary self types allows shadowing
1252+
/// of methods that might be a compatibility break. Specifically,
1253+
/// we have something like:
1254+
/// ```ignore (illustrative)
1255+
/// struct A;
1256+
/// impl A {
1257+
/// fn foo(self: &NonNull<A>) {}
1258+
/// // note this is by reference
1259+
/// }
1260+
/// ```
1261+
/// then we've come along and added this method to `NonNull`:
1262+
/// ```ignore (illustrative)
1263+
/// fn foo(self) // note this is by value
1264+
/// ```
1265+
/// Report an error in this case.
1266+
fn check_for_shadowed_autorefd_method(
1267+
&self,
1268+
_possible_shadower: &Pick<'tcx>,
1269+
step: &CandidateStep<'tcx>,
1270+
self_ty: Ty<'tcx>,
1271+
mutbl: hir::Mutability,
1272+
track_unstable_candidates: bool,
1273+
) -> Result<(), MethodError<'tcx>> {
1274+
// We don't want to remember any of the diagnostic hints from this
1275+
// shadow search, but we do need to provide Some/None for the
1276+
// unstable_candidates in order to reflect the behavior of the
1277+
// main search.
1278+
let mut pick_diag_hints = PickDiagHints {
1279+
unstable_candidates: if track_unstable_candidates { Some(Vec::new()) } else { None },
1280+
unsatisfied_predicates: &mut Vec::new(),
1281+
};
1282+
let _potentially_shadowed_pick =
1283+
self.pick_autorefd_method(step, self_ty, mutbl, &mut pick_diag_hints);
1284+
1285+
// At the moment, this function does no checks. A future
1286+
// commit will fill out the body here.
1287+
Ok(())
1288+
}
1289+
11861290
/// For each type `T` in the step list, this attempts to find a method where
11871291
/// the (transformed) self type is exactly `T`. We do however do one
11881292
/// transformation on the adjustment: if we are passing a region pointer in,

0 commit comments

Comments
 (0)