Dedup bounds with parent impl block #105325
Conversation
rustbot
added
S-waiting-on-review
src/librustdoc/clean/mod.rs
Outdated
| let (check_not_in_parent_bounds, check_not_in_parent_params): ( | ||
| Box<dyn Fn(&GenericBound) -> bool>, | ||
| Box<dyn Fn(&Lifetime) -> bool>, | ||
| ) = if let Some((bounds, bound_params)) = parent_where_predicates | ||
| .iter() | ||
| .filter_map(|pred| match pred { | ||
| WherePredicate::BoundPredicate { ty: parent_ty, bounds, bound_params } | ||
| if *parent_ty == ty => | ||
| { | ||
| Some((bounds, bound_params)) | ||
| } | ||
| _ => None, | ||
| }) | ||
| .next() | ||
| { | ||
| ( | ||
| Box::new(move |bound: &GenericBound| !bounds.contains(&bound)), | ||
| Box::new(move |bound_param: &Lifetime| !bound_params.contains(&bound_param)), | ||
| ) | ||
| } else { | ||
| (Box::new(|_: &GenericBound| true), Box::new(|_: &Lifetime| true)) | ||
| }; | ||
|
|
||
| let (check_not_in_parent_tys, check_not_in_parent_lifetimes): ( | ||
| Box<dyn Fn(&GenericBound) -> bool>, | ||
| Box<dyn Fn(&Lifetime) -> bool>, | ||
| ) = if let Some(parent_bound) = | ||
| parent_params.iter().filter(|gen| Type::Generic(gen.name) == ty).next() | ||
| { | ||
| ( | ||
| Box::new(move |bound: &GenericBound| match &parent_bound.kind { | ||
| GenericParamDefKind::Type { bounds, .. } => !bounds.contains(&bound), | ||
| _ => true, | ||
| }), | ||
| Box::new(move |bound: &Lifetime| match &parent_bound.kind { | ||
| GenericParamDefKind::Lifetime { outlives } => !outlives.contains(&bound), | ||
| _ => true, | ||
| }), | ||
| ) | ||
| } else { | ||
| (Box::new(|_: &GenericBound| true), Box::new(|_: &Lifetime| true)) | ||
| }; |
There was a problem hiding this comment.
This whole code isn't great but I don't know how to make it a bit better. If someone has an idea...
There was a problem hiding this comment.
What if instead of creating closures closing over bounds / bound_params / outlives, you simply return those aforementioned variables from the two if lets (returning Default::default() / None / Vec::new() from the else branch) and later write closures inline: .filter(check_not_in_parent_bounds) → .filter(|bound| !parent_bounds.contains(bound)). Cloning should not be necessary since you should be able to use references. Not sure if that works out well and looks better but might be worth a shot.
There was a problem hiding this comment.
To illustrate:
- let (check_not_in_parent_bounds, check_not_in_parent_params): (
- Box<dyn Fn(&GenericBound) -> bool>,
- Box<dyn Fn(&Lifetime) -> bool>,
- ) = if let Some((bounds, bound_params)) =
- parent_where_predicates.iter().find_map(|pred| match pred {
- WherePredicate::BoundPredicate { ty: parent_ty, bounds, bound_params }
- if *parent_ty == ty =>
- {
- Some((bounds, bound_params))
- }
- _ => None,
- }) {
- (
- Box::new(move |bound: &GenericBound| !bounds.contains(&bound)),
- Box::new(move |bound_param: &Lifetime| !bound_params.contains(&bound_param)),
- )
- } else {
- (Box::new(|_: &GenericBound| true), Box::new(|_: &Lifetime| true))
- };
+ let (parent_bounds, parent_bound_params) = parent_where_predicates
+ .iter()
+ .find_map(|pred| match pred {
+ WherePredicate::BoundPredicate {
+ ty: parent_ty,
+ bounds,
+ bound_params,
+ } if *parent_ty == ty => Some((bounds, bound_params)),
+ _ => None,
+ })
+ .unwrap_or(const { (&Vec::new(), &Vec::new()) });There was a problem hiding this comment.
bounds & bound_params are of type &Vec<_> as far as I can tell, so I guess .unwrap_or_default() does not work (Vec<_> impls Default but &Vec<_> does not). However, you could instead write sth. like .unwrap_or(const { (&Vec::new(), &Vec::new()) }) instead where the const block const { … } creates &'static Vec<_>s. Instead of const blocks, you could also use const items or apart from that Cow::Owned(Vec::new()) (and Cow::Borrowed in the Some case).
Edit 0: I've updated the patch above.
Edit 1: What do you think about that?
src/librustdoc/clean/mod.rs
Outdated
| } | ||
| _ => None, | ||
| }) | ||
| .next() |
There was a problem hiding this comment.
.filter_map(…).next() → .find_map(…)
GuillaumeGomez
force-pushed
the
dedup-bounds-with-parent
branch
from
2829fc0 to
8a78f42
Compare
|
This is a lot of code doing something half way what the type system knows how to do perfectly. Basically you could get the param env of the parent and see if any of the predicates of the child can be proven just with that param env. Any that can, can be safely ignored. Should hopefully be as simple as building an obligation and calling |
|
Closing in favor of #105392 |
This is another part of #104886.
It still doesn't fix it as I need to handle
Selfas the last part.r? @notriddle