Skip to content

Simplify unstable methods list #128549

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

Simplify unstable methods list #128549

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
57 changes: 25 additions & 32 deletions compiler/rustc_hir_typeck/src/method/probe.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1030,12 +1030,12 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {

fn pick_core(&self) -> Option<PickResult<'tcx>> {
// Pick stable methods only first, and consider unstable candidates if not found.
self.pick_all_method(Some(&mut vec![])).or_else(|| self.pick_all_method(None))
self.pick_all_method(Some(vec![])).or_else(|| self.pick_all_method(None))
}

fn pick_all_method(
&self,
mut unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
mut unstable_candidates: Option<Vec<(Candidate<'tcx>, Symbol)>>,
) -> Option<PickResult<'tcx>> {
self.steps
.iter()
Expand All @@ -1056,30 +1056,23 @@ 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())
self.pick_by_value_method(step, self_ty, &mut unstable_candidates).or_else(|| {
self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Not,
&mut unstable_candidates,
)
.or_else(|| {
self.pick_autorefd_method(
step,
self_ty,
hir::Mutability::Not,
unstable_candidates.as_deref_mut(),
hir::Mutability::Mut,
&mut unstable_candidates,
)
.or_else(|| {
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(),
)
})
})
.or_else(|| self.pick_const_ptr_method(step, self_ty, &mut unstable_candidates))
})
})
}

Expand All @@ -1093,7 +1086,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
&self,
step: &CandidateStep<'tcx>,
self_ty: Ty<'tcx>,
unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
unstable_candidates: &mut Option<Vec<(Candidate<'tcx>, Symbol)>>,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why did you invert this?

I think Option<&mut T> is generally preferred over &mut Option<T>, unless you intend to modify the value of the Option itself. Passing &mut Option<T> implies the wrong intention with this IMO, since passing in unstable_candidates indicates that that the caller wants unstable candidates recorded -- I don't expect the callee to ever, e.g., set *unstable_candiates = Some(vec![]).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough - I'll close this PR.

It happens to simplify some of the subsequent work on arbitrary self types, but I'll find another way to do it. Most likely I'll make a newtype wrapper for this Option.

Copy link
Member

@compiler-errors compiler-errors Aug 2, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean, I don't mind the other half of this PR, i.e. passing Option<Vec<..>> at the top level of pick -- I don't necessarily see why these had to change from Option<&mut Vec<>> to &mut Option<Vec<>> as a consequence though?

) -> Option<PickResult<'tcx>> {
if step.unsize {
return None;
Expand Down Expand Up @@ -1122,7 +1115,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
step: &CandidateStep<'tcx>,
self_ty: Ty<'tcx>,
mutbl: hir::Mutability,
unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
unstable_candidates: &mut Option<Vec<(Candidate<'tcx>, Symbol)>>,
) -> Option<PickResult<'tcx>> {
let tcx = self.tcx;

Expand All @@ -1147,7 +1140,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
&self,
step: &CandidateStep<'tcx>,
self_ty: Ty<'tcx>,
unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
unstable_candidates: &mut Option<Vec<(Candidate<'tcx>, Symbol)>>,
) -> Option<PickResult<'tcx>> {
// Don't convert an unsized reference to ptr
if step.unsize {
Expand All @@ -1171,7 +1164,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
fn pick_method(
&self,
self_ty: Ty<'tcx>,
mut unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
unstable_candidates: &mut Option<Vec<(Candidate<'tcx>, Symbol)>>,
) -> Option<PickResult<'tcx>> {
debug!("pick_method(self_ty={})", self.ty_to_string(self_ty));

Expand All @@ -1185,7 +1178,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
self_ty,
candidates,
&mut possibly_unsatisfied_predicates,
unstable_candidates.as_deref_mut(),
unstable_candidates,
);
if let Some(pick) = res {
return Some(pick);
Expand All @@ -1194,7 +1187,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {

if self.private_candidate.get().is_none() {
if let Some(Ok(pick)) =
self.consider_candidates(self_ty, &self.private_candidates, &mut vec![], None)
self.consider_candidates(self_ty, &self.private_candidates, &mut vec![], &mut None)
{
self.private_candidate.set(Some((pick.item.kind.as_def_kind(), pick.item.def_id)));
}
Expand All @@ -1217,7 +1210,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
Option<ty::Predicate<'tcx>>,
Option<ObligationCause<'tcx>>,
)>,
mut unstable_candidates: Option<&mut Vec<(Candidate<'tcx>, Symbol)>>,
unstable_candidates: &mut Option<Vec<(Candidate<'tcx>, Symbol)>>,
) -> Option<PickResult<'tcx>> {
let mut applicable_candidates: Vec<_> = candidates
.iter()
Expand All @@ -1237,7 +1230,7 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}
}

if let Some(uc) = &mut unstable_candidates {
if let Some(uc) = unstable_candidates {
applicable_candidates.retain(|&(candidate, _)| {
if let stability::EvalResult::Deny { feature, .. } =
self.tcx.eval_stability(candidate.item.def_id, None, self.span, None)
Expand All @@ -1255,10 +1248,10 @@ impl<'a, 'tcx> ProbeContext<'a, 'tcx> {
}

applicable_candidates.pop().map(|(probe, status)| match status {
ProbeResult::Match => {
Ok(probe
.to_unadjusted_pick(self_ty, unstable_candidates.cloned().unwrap_or_default()))
}
ProbeResult::Match => Ok(probe.to_unadjusted_pick(
self_ty,
unstable_candidates.as_ref().cloned().unwrap_or_default(),
)),
ProbeResult::NoMatch | ProbeResult::BadReturnType => Err(MethodError::BadReturnType),
})
}
Expand Down