Simplify unstable methods list #128549
Simplify unstable methods list #128549
Conversation
This PR is not intended to make any functional difference, but it replaces a reference with an owned Vec for a slight code simplification. This will become more useful in subsequent work towards arbitrary self types.
|
r? @davidtwco rustbot has assigned @davidtwco. Use |
rustbot
added
S-waiting-on-review
|
The commit in this PR is also the first commit in #128548. r? @wesleywiser |
| @@ -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)>>, | |||
There was a problem hiding this comment.
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![]).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
This PR is not intended to make any functional difference, but it replaces a reference with an owned
Vecfor a slight code simplification. This will become more useful in subsequent work towards arbitrary self types, RFC 3519 (#44874).