Skip to content

Commit 9fb1d82

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: #127812 (comment) Subsequent commits will improve the speed of the search.
1 parent c1a685b commit 9fb1d82

File tree

1 file changed

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

1 file changed

+112
-10
lines changed

compiler/rustc_hir_typeck/src/method/probe.rs

+112-10
Original file line numberDiff line numberDiff line change
@@ -1161,6 +1161,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
11611161
&self,
11621162
pick_diag_hints: &mut PickDiagHints<'b, 'tcx>,
11631163
) -> Option<PickResult<'tcx>> {
1164+
let track_unstable_candidates = pick_diag_hints.unstable_candidates.is_some();
11641165
self.steps
11651166
.iter()
11661167
// At this point we're considering the types to which the receiver can be converted,
@@ -1184,22 +1185,123 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
11841185
.unwrap_or_else(|_| {
11851186
span_bug!(self.span, "{:?} was applicable but now isn't?", step.self_ty)
11861187
});
1187-
self.pick_by_value_method(step, self_ty, pick_diag_hints).or_else(|| {
1188-
self.pick_autorefd_method(step, self_ty, hir::Mutability::Not, pick_diag_hints)
1189-
.or_else(|| {
1190-
self.pick_autorefd_method(
1188+
1189+
let by_value_pick = self.pick_by_value_method(step, self_ty, pick_diag_hints);
1190+
1191+
// Check for shadowing of a by-reference method by a by-value method (see comments on check_for_shadowing)
1192+
if let Some(by_value_pick) = by_value_pick {
1193+
if let Ok(by_value_pick) = by_value_pick.as_ref() {
1194+
if by_value_pick.kind == PickKind::InherentImplPick {
1195+
if let Err(e) = self.check_for_shadowed_autorefd_method(
1196+
by_value_pick,
1197+
step,
1198+
self_ty,
1199+
hir::Mutability::Not,
1200+
track_unstable_candidates,
1201+
) {
1202+
return Some(Err(e));
1203+
}
1204+
if let Err(e) = self.check_for_shadowed_autorefd_method(
1205+
by_value_pick,
11911206
step,
11921207
self_ty,
11931208
hir::Mutability::Mut,
1194-
pick_diag_hints,
1195-
)
1196-
})
1197-
.or_else(|| self.pick_const_ptr_method(step, self_ty, pick_diag_hints))
1198-
.or_else(|| self.pick_reborrow_pin_method(step, self_ty, pick_diag_hints))
1199-
})
1209+
track_unstable_candidates,
1210+
) {
1211+
return Some(Err(e));
1212+
}
1213+
}
1214+
}
1215+
return Some(by_value_pick);
1216+
}
1217+
1218+
let autoref_pick =
1219+
self.pick_autorefd_method(step, self_ty, hir::Mutability::Not, pick_diag_hints);
1220+
// Check for shadowing of a by-mut-ref method by a by-reference method (see comments on check_for_shadowing)
1221+
if let Some(autoref_pick) = autoref_pick {
1222+
if let Ok(autoref_pick) = autoref_pick.as_ref() {
1223+
// Check we're not shadowing others
1224+
if autoref_pick.kind == PickKind::InherentImplPick {
1225+
if let Err(e) = self.check_for_shadowed_autorefd_method(
1226+
autoref_pick,
1227+
step,
1228+
self_ty,
1229+
hir::Mutability::Mut,
1230+
track_unstable_candidates,
1231+
) {
1232+
return Some(Err(e));
1233+
}
1234+
}
1235+
}
1236+
return Some(autoref_pick);
1237+
}
1238+
1239+
// Note that no shadowing errors are produced from here on,
1240+
// as we consider const ptr methods.
1241+
// We allow new methods that take *mut T to shadow
1242+
// methods which took *const T, so there is no entry in
1243+
// this list for the results of `pick_const_ptr_method`.
1244+
// The reason is that the standard pointer cast method
1245+
// (on a mutable pointer) always already shadows the
1246+
// cast method (on a const pointer). So, if we added
1247+
// `pick_const_ptr_method` to this method, the anti-
1248+
// shadowing algorithm would always complain about
1249+
// the conflict between *const::cast and *mut::cast.
1250+
// In practice therefore this does constrain us:
1251+
// we cannot add new
1252+
// self: *mut Self
1253+
// methods to types such as NonNull or anything else
1254+
// which implements Receiver, because this might in future
1255+
// shadow existing methods taking
1256+
// self: *const NonNull<Self>
1257+
// in the pointee. In practice, methods taking raw pointers
1258+
// are rare, and it seems that it should be easily possible
1259+
// to avoid such compatibility breaks.
1260+
self.pick_autorefd_method(step, self_ty, hir::Mutability::Mut, pick_diag_hints)
1261+
.or_else(|| self.pick_const_ptr_method(step, self_ty, pick_diag_hints))
1262+
.or_else(|| self.pick_reborrow_pin_method(step, self_ty, pick_diag_hints))
12001263
})
12011264
}
12021265

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

0 commit comments

Comments
 (0)