Skip to content

Commit

Permalink
Arbitrary self types v2: probe for more methods.
Browse files Browse the repository at this point in the history
Rust prioritizes method candidates in this order:
1. By value;
2. By reference;
3. By mutable reference;
4. By const ptr.

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.

We're adding the extra searches in a preliminary commit because they are
the risky bit. We want to find any functional or performance problems
from these extra searches before we proceed with the actual work on
arbitrary self types and specifically on extra deshadowing errors.
  • Loading branch information
adetaylor committed Jul 16, 2024
1 parent b286722 commit edc2276
Showing 1 changed file with 108 additions and 17 deletions.
125 changes: 108 additions & 17 deletions compiler/rustc_hir_typeck/src/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1056,33 +1056,124 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
.unwrap_or_else(|_| {
span_bug!(self.span, "{:?} was applicable but now isn't?", step.self_ty)
});
self.pick_by_value_method(step, self_ty, unstable_candidates.as_deref_mut())
.or_else(|| {
self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Not,
unstable_candidates.as_deref_mut(),
)
.or_else(|| {
self.pick_autorefd_method(

let by_value_pick =
self.pick_by_value_method(step, self_ty, unstable_candidates.as_deref_mut());

// Check for shadowing of a by-reference method by a by-value method (see comments on check_for_shadowing)
if let Some(by_value_pick) = by_value_pick {
if let Ok(by_value_pick) = by_value_pick.as_ref() {
if by_value_pick.kind == PickKind::InherentImplPick {
let autoref_pick = self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Not,
unstable_candidates.as_deref_mut(),
);
if let Err(e) = self.check_for_shadowing(by_value_pick, autoref_pick) {
return Some(Err(e));
}

let autoref_mut_pick = self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Mut,
unstable_candidates.as_deref_mut(),
)
})
.or_else(|| {
self.pick_const_ptr_method(
);
if let Err(e) =
self.check_for_shadowing(by_value_pick, autoref_mut_pick)
{
return Some(Err(e));
}
}
}
return Some(by_value_pick);
}

let autoref_pick = self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Not,
unstable_candidates.as_deref_mut(),
);
// Check for shadowing of a by-mut-ref method by a by-reference method (see comments on check_for_shadowing)
if let Some(autoref_pick) = autoref_pick {
if let Ok(autoref_pick) = autoref_pick.as_ref() {
// Check we're not shadowing others
if autoref_pick.kind == PickKind::InherentImplPick {
let autoref_mut_pick = self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Mut,
unstable_candidates.as_deref_mut(),
)
})
})
);
if let Err(e) = self.check_for_shadowing(autoref_pick, autoref_mut_pick)
{
return Some(Err(e));
}
}
}
return Some(autoref_pick);
}

// Note that no shadowing errors are produced from here on,
// as we consider const ptr methods.
// We allow new methods that take *mut T to shadow
// methods which took *const T, so there is no entry in
// this list for the results of `pick_const_ptr_method`.
// The reason is that the standard pointer cast method
// (on a mutable pointer) always already shadows the
// cast method (on a const pointer). So, if we added
// `pick_const_ptr_method` to this method, the anti-
// shadowing algorithm would always complain about
// the conflict between *const::cast and *mut::cast.
// In practice therefore this does constrain us:
// we cannot add new
// self: *mut Self
// methods to types such as NonNull or anything else
// which implements Receiver, because this might in future
// shadow existing methods taking
// self: *const NonNull<Self>
// in the pointee. In practice, methods taking raw pointers
// are rare, and it seems that it should be easily possible
// to avoid such compatibility breaks.
self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Mut,
unstable_candidates.as_deref_mut(),
)
.or_else(|| {
self.pick_const_ptr_method(step, self_ty, unstable_candidates.as_deref_mut())
})
})
}

/// Check for cases where arbitrary self types allows shadowing
/// of methods that might be a compatibility break. Specifically,
/// we have something like:
/// ```
/// struct A;
/// impl A {
/// fn foo(self: &NonNull<A>) {}

Check failure on line 1158 in compiler/rustc_hir_typeck/src/method/probe.rs

View workflow job for this annotation

GitHub Actions / PR - x86_64-gnu-llvm-17

cannot find type `NonNull` in this scope
/// // note this is by reference
/// }
/// ```
/// then we've come along and added this method to `NonNull`:
/// ```
/// fn foo(self) // note this is by value

Check failure on line 1164 in compiler/rustc_hir_typeck/src/method/probe.rs

View workflow job for this annotation

GitHub Actions / PR - x86_64-gnu-llvm-17

free function without a body

Check failure on line 1164 in compiler/rustc_hir_typeck/src/method/probe.rs

View workflow job for this annotation

GitHub Actions / PR - x86_64-gnu-llvm-17

`self` parameter is only allowed in associated functions
/// ```

Check failure on line 1165 in compiler/rustc_hir_typeck/src/method/probe.rs

View workflow job for this annotation

GitHub Actions / PR - x86_64-gnu-llvm-17

expected one of `->`, `where`, or `{`, found `}`
/// Report an error in this case.
fn check_for_shadowing(
&self,
_possible_shadower: &Pick<'tcx>,
_possible_shadowed: Option<Result<Pick<'tcx>, MethodError<'tcx>>>,
) -> Result<(), MethodError<'tcx>> {
// At the moment, this function does nothing. A future
// commit will fill out the body here.
Ok(())
}

/// For each type `T` in the step list, this attempts to find a method where
/// the (transformed) self type is exactly `T`. We do however do one
/// transformation on the adjustment: if we are passing a region pointer in,
Expand Down

0 comments on commit edc2276

Please sign in to comment.