Skip to content

Merge generics and where predicates and prevent duplicates in where predicates #105183

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.

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

Merged
merged 3 commits into from
Dec 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
124 changes: 91 additions & 33 deletions src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub(crate) mod utils;

use rustc_ast as ast;
use rustc_attr as attr;
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexSet};
use rustc_data_structures::fx::{FxHashMap, FxHashSet, FxIndexMap, FxIndexSet, IndexEntry};
use rustc_hir as hir;
use rustc_hir::def::{CtorKind, DefKind, Res};
use rustc_hir::def_id::{DefId, LOCAL_CRATE};
Expand Down Expand Up @@ -598,47 +598,105 @@ pub(crate) fn clean_generics<'tcx>(
})
.collect::<Vec<_>>();

let mut bound_predicates = FxIndexMap::default();
Copy link
Member Author

Choose a reason for hiding this comment

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

I used FxIndexMap to keep the order of predicates.

let mut region_predicates = FxIndexMap::default();
let mut eq_predicates = ThinVec::default();
for pred in gens.predicates.iter().filter_map(|x| clean_where_predicate(x, cx)) {
match pred {
WherePredicate::BoundPredicate { ty, bounds, bound_params } => {
match bound_predicates.entry(ty) {
IndexEntry::Vacant(v) => {
v.insert((bounds, bound_params));
}
IndexEntry::Occupied(mut o) => {
// we merge both bounds.
for bound in bounds {
if !o.get().0.contains(&bound) {
o.get_mut().0.push(bound);
}
}
for bound_param in bound_params {
if !o.get().1.contains(&bound_param) {
o.get_mut().1.push(bound_param);
}
}
}
}
}
WherePredicate::RegionPredicate { lifetime, bounds } => {
match region_predicates.entry(lifetime) {
IndexEntry::Vacant(v) => {
v.insert(bounds);
}
IndexEntry::Occupied(mut o) => {
// we merge both bounds.
for bound in bounds {
if !o.get().contains(&bound) {
o.get_mut().push(bound);
}
}
}
}
}
WherePredicate::EqPredicate { lhs, rhs, bound_params } => {
eq_predicates.push(WherePredicate::EqPredicate { lhs, rhs, bound_params });
}
}
}

let mut params = ThinVec::with_capacity(gens.params.len());
// In this loop, we gather the generic parameters (`<'a, B: 'a>`) and check if they have
// bounds in the where predicates. If so, we move their bounds into the where predicates
// while also preventing duplicates.
for p in gens.params.iter().filter(|p| !is_impl_trait(p) && !is_elided_lifetime(p)) {
let p = clean_generic_param(cx, Some(gens), p);
let mut p = clean_generic_param(cx, Some(gens), p);
match &mut p.kind {
GenericParamDefKind::Lifetime { ref mut outlives } => {
if let Some(region_pred) = region_predicates.get_mut(&Lifetime(p.name)) {
// We merge bounds in the `where` clause.
for outlive in outlives.drain(..) {
let outlive = GenericBound::Outlives(outlive);
if !region_pred.contains(&outlive) {
region_pred.push(outlive);
}
}
}
}
GenericParamDefKind::Type { bounds, synthetic: false, .. } => {
if let Some(bound_pred) = bound_predicates.get_mut(&Type::Generic(p.name)) {
// We merge bounds in the `where` clause.
for bound in bounds.drain(..) {
if !bound_pred.0.contains(&bound) {
bound_pred.0.push(bound);
}
}
}
}
GenericParamDefKind::Type { .. } | GenericParamDefKind::Const { .. } => {
// nothing to do here.
}
}
params.push(p);
}
params.extend(impl_trait_params);

let mut generics = Generics {
Generics {
params,
where_predicates: gens
.predicates
.iter()
.filter_map(|x| clean_where_predicate(x, cx))
where_predicates: bound_predicates
.into_iter()
.map(|(ty, (bounds, bound_params))| WherePredicate::BoundPredicate {
ty,
bounds,
bound_params,
})
.chain(
region_predicates
.into_iter()
.map(|(lifetime, bounds)| WherePredicate::RegionPredicate { lifetime, bounds }),
)
.chain(eq_predicates.into_iter())
.collect(),
};

// Some duplicates are generated for ?Sized bounds between type params and where
// predicates. The point in here is to move the bounds definitions from type params
// to where predicates when such cases occur.
for where_pred in &mut generics.where_predicates {
Copy link
Member Author

@GuillaumeGomez GuillaumeGomez Dec 2, 2022

Choose a reason for hiding this comment

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

It seems like this whole block was not necessary anymore with my dedup changes above.

match *where_pred {
WherePredicate::BoundPredicate { ty: Generic(ref name), ref mut bounds, .. } => {
if bounds.is_empty() {
for param in &mut generics.params {
match param.kind {
GenericParamDefKind::Lifetime { .. } => {}
GenericParamDefKind::Type { bounds: ref mut ty_bounds, .. } => {
if &param.name == name {
mem::swap(bounds, ty_bounds);
break;
}
}
GenericParamDefKind::Const { .. } => {}
}
}
}
}
_ => continue,
}
}
generics
}

fn clean_ty_generics<'tcx>(
Expand Down
20 changes: 20 additions & 0 deletions src/test/rustdoc/bounds-in-multiple-parts.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#![crate_name = "foo"]

pub trait Eq {}
pub trait Eq2 {}

// Checking that "where predicates" and "generics params" are merged.
// @has 'foo/trait.T.html'
// @has - "//*[@id='tymethod.f']/h4" "fn f<'a, 'b, 'c, T>()where Self: Eq, T: Eq + 'a, 'c: 'b + 'a,"
pub trait T {
fn f<'a, 'b, 'c: 'a, T: Eq + 'a>()
where Self: Eq, Self: Eq, T: Eq, 'c: 'b;
}

// Checking that a duplicated "where predicate" is removed.
// @has 'foo/trait.T2.html'
// @has - "//*[@id='tymethod.f']/h4" "fn f<T>()where Self: Eq + Eq2, T: Eq2 + Eq,"
pub trait T2 {
fn f<T: Eq>()
where Self: Eq, Self: Eq2, T: Eq2;
}
4 changes: 2 additions & 2 deletions src/test/rustdoc/impl-parts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pub auto trait AnAutoTrait {}
pub struct Foo<T> { field: T }

// @has impl_parts/struct.Foo.html '//*[@class="impl has-srclink"]//h3[@class="code-header"]' \
// "impl<T: Clone> !AnAutoTrait for Foo<T>where T: Sync,"
// "impl<T> !AnAutoTrait for Foo<T>where T: Sync + Clone,"
// @has impl_parts/trait.AnAutoTrait.html '//*[@id="implementors-list"]//h3[@class="code-header"]' \
// "impl<T: Clone> !AnAutoTrait for Foo<T>where T: Sync,"
// "impl<T> !AnAutoTrait for Foo<T>where T: Sync + Clone,"
impl<T: Clone> !AnAutoTrait for Foo<T> where T: Sync {}
2 changes: 1 addition & 1 deletion src/test/rustdoc/rfc-2632-const-trait-impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ impl<T> S<T> {
// @has - '//section[@id="method.foo"]/h4[@class="code-header"]/a[@class="trait"]' 'Clone'
// @!has - '//section[@id="method.foo"]/h4[@class="code-header"]/span[@class="where"]' '~const'
// @has - '//section[@id="method.foo"]/h4[@class="code-header"]/span[@class="where fmt-newline"]' ': Clone'
pub const fn foo<B: ~const Clone + ~const Destruct>()
pub const fn foo<B, C: ~const Clone + ~const Destruct>()
where
B: ~const Clone + ~const Destruct,
{
Expand Down
2 changes: 1 addition & 1 deletion src/test/rustdoc/whitespace-after-where-clause.enum.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="item-decl"><pre class="rust enum"><code>pub enum Cow&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a&gt;<span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt;,</span>{
<div class="item-decl"><pre class="rust enum"><code>pub enum Cow&lt;'a, B&gt;<span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt; + ?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a,</span>{
Borrowed(<a class="primitive" href="{{channel}}/std/primitive.reference.html">&amp;'a </a>B),
Whatever(<a class="primitive" href="{{channel}}/std/primitive.u32.html">u32</a>),
}</code></pre></div>
2 changes: 1 addition & 1 deletion src/test/rustdoc/whitespace-after-where-clause.struct.html
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<div class="item-decl"><pre class="rust struct"><code>pub struct Struct&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a&gt;<span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt;,</span>{
<div class="item-decl"><pre class="rust struct"><code>pub struct Struct&lt;'a, B&gt;<span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt; + ?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a,</span>{
pub a: <a class="primitive" href="{{channel}}/std/primitive.reference.html">&amp;'a </a>B,
pub b: <a class="primitive" href="{{channel}}/std/primitive.u32.html">u32</a>,
}</code></pre></div>
2 changes: 1 addition & 1 deletion src/test/rustdoc/whitespace-after-where-clause.union.html
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
<div class="item-decl"><pre class="rust union"><code>pub union Union&lt;'a, B:&#160;?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a&gt;<span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt;,</span>{
<div class="item-decl"><pre class="rust union"><code>pub union Union&lt;'a, B&gt;<span class="where fmt-newline">where<br />&#160;&#160;&#160;&#160;B: <a class="trait" href="trait.ToOwned.html" title="trait foo::ToOwned">ToOwned</a>&lt;dyn <a class="trait" href="{{channel}}/core/clone/trait.Clone.html" title="trait core::clone::Clone">Clone</a>&gt; + ?<a class="trait" href="{{channel}}/core/marker/trait.Sized.html" title="trait core::marker::Sized">Sized</a> + 'a,</span>{
/* private fields */
}</code></pre></div>