Skip to content

rustdoc's resugaring of projection predicates to type bindings is flawed #113015

Open
Open
rustdoc's resugaring of projection predicates to type bindings is flawed#113015
Assignees
fmease
Labels
A-cross-crate-reexportsArea: Documentation that has been re-exported from a different crateC-bugCategory: This is a bug.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

Description

@fmease
fmease
opened on Jun 25, 2023

First discovered thanks to bevyengine/bevy#8898: There, the Output type of the cross-crate opaque impl FnMut(…) -> bool gets dropped by rustdoc resulting in impl FnMut(…). This specific bug is caused by rustdoc's opaque bound cleaner not realizing that it can & should utilize the Output assoc ty defined on FnMut's supertrait FnOnce. During further investigations I've discovered more bugs in this area.

In rustdoc there are two three major and distinct places where we resugar bounds. Both All three are flawed but each one exhibits different bugs.

  • (1) clean_middle_opaque_bounds for bounds of opaques (existential impl-Trait)
    • we only check for TraitRef equality while we should also check the super predicates (incl. their substs tho!)
      • (once we do, ensure that we don't intro name clashes into the list of type bindings (this can be triggered by relatively simple user code involving supertraits))
  • (2) clean_ty_generics for bounds in where-clauses and APIT (universal impl-Trait)
    • we do check super predicates but we only ever check DefIds (of traits) and never the relevant substs which is super incorrect leading rustdoc to move some projection predicates to unrelated trait bounds
    • the process mutates state by yanking those predicates rendering it order dependent (consider permutations of super and subtraits each one with own type bindings)
  • (3) param_env_to_generics in auto_trait.rs for where-clauses of synthetic impls
    • update: replaced by simplify::{where_clauses,sized_bounds} in #123340
    • doesn't use simplify::merge_bounds at all unlike the other two places
    • doesn't merge two partially resugared bounds like Tr<A = …> and Tr<B = …>
    • doesn't render bound vars / higher-ranked parameters at all
    • (I haven't thoroughly checked yet how supertraits are handled but it seems like Fn-family traits are special-cased)

Probably there are some more but less important bugs to be found (e.g. related to bound vars I can imagine).

I'm already deep in the weeds working on a correct & most general fix for this.
Opening this issue to keep the description of the future PR more slender.
@rustbot claim

@rustbot label C-bug T-rustdoc A-cross-crate-reexports

Activity

  1. rustbot
    assigned
    fmease
    on Jun 25, 2023
  2. rustbot
    added
    A-cross-crate-reexportsArea: Documentation that has been re-exported from a different crate
    C-bugCategory: This is a bug.
    T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.
    on Jun 25, 2023
  3. fmease
    changed the title [-]rustdoc's resugaring of projections predicates to type bindings is flawed when supertraits are involved[/-] [+]rustdoc's resugaring of projection predicates to type bindings is flawed when supertraits are involved[/+] on Jun 25, 2023
  4. matthiaskrgr
    added a commit that references this issue on Oct 4, 2023

    Rollup merge of rust-lang#116388 - fmease:rustdoc-fix-n-clean-up-x-cr…

    3e29363
  5. rust-timer
    added a commit that references this issue on Oct 4, 2023

    Unrolled build for rust-lang#116388

    67caf0e

  6. RalfJung commented on Mar 23, 2024

    @RalfJung
    RalfJung
    on Mar 23, 2024
    Member

    The duplicate Target = [u8] here is another instance of this bug.

  7. bors
    added a commit that references this issue on Mar 24, 2024

    Auto merge of rust-lang#122974 - fmease:rustdoc-auto-trait-experiment…

    87fbd78
  8. bors
    added 2 commits that reference this issue on Apr 1, 2024

    Auto merge of rust-lang#123340 - fmease:rustdoc-simplify-auto-trait-i…

    ce8cfc5

    Auto merge of rust-lang#123340 - fmease:rustdoc-simplify-auto-trait-i…

    5dbaafd

  9. fmease commented on Apr 16, 2024

    @fmease
    fmease
    on Apr 16, 2024
    MemberAuthor

    Major: OPAQ: We don't take supertrait bounds into account when reconstructing assoc item constraints (“type bindings”) from projections (#116735 would've fixed that but I've closed it in favor of a more general PR).

    Source

    pub fn rpit_fn() -> impl Fn() -> bool {
    || false
}

    Rendered inlined cross-crate re-export

    pub fn rpit_fn() -> impl Fn()

    Source

    pub fn f() -> impl TrA<X = ()> {}

pub trait TrA: TrB {}
pub trait TrB { type X; }

impl TrA for () {}
impl TrB for () { type X = (); }

    Rendered inlined cross-crate re-export

    pub fn f() -> impl TrA

  10. fmease commented on Apr 16, 2024

    @fmease
    fmease
    on Apr 16, 2024
    MemberAuthor

    Major: WHERE & APIT: We don't take generic arguments of super trait bounds into account when reconstructing assoc item constraints (“type bindings”) from projections, like at all!

    Source

    pub struct Ty<T>(T)
where
    T: TrA + TrB<i32, X = ()>;

pub trait TrA: TrB<()> {}
pub trait TrB<D> { type X; }

    Rendered inlined cross-crate re-export

    pub struct Ty<T>(/* private fields */)
where
    T: TrA<X = ()> + TrB<i32>;

    Source

    pub struct Ty<T>(T)
where
    T: TrA<X = bool> + TrB<i32, X = ()>;

pub trait TrA: TrB<()> {}
pub trait TrB<D> { type X; }

    Rendered inline cross-crate re-export

    pub struct Ty<T>(/* private fields */)
where
    T: TrA<X = bool, X = ()> + TrB<i32>;

  11. fmease commented on Apr 16, 2024

    @fmease
    fmease
    on Apr 16, 2024
    MemberAuthor

    Minor: WHERE: We don't try to reconstruct overarching / outer binders of clauses.

    Source

    pub struct Ty<T>(T)
where
    for<'o> T: TrA<'o> + TrB<'o>;

pub trait TrA<'a> {}
pub trait TrB<'a> {}

    Rendered inlined cross-crate re-export

    pub struct Ty<T>(/* private fields */)
where
    T: for<'o> TrA<'o> + for<'o> TrB<'o>;

    Edit: Medium: WHERE: We drop the "outer" binder of type-outlives predicates:

    Source

    pub fn demo<T>() where for<'a> T: 'a {}

    Rendered inlined cross-crate re-export

    pub fn demo<T>()
where
    T: 'a,

  12. fmease commented on Apr 16, 2024

    @fmease
    fmease
    on Apr 16, 2024
    MemberAuthor

    Minor: WHERE: We don't necessarily preserve the order bounds that differ in LHS (bounded ty, lt). However, the HIR cleaner doesn't seem to do that either (requires a different reproducer though).

    Source

    pub fn f<'a, T>() where T: 'static, 'a: 'static {}

    Rendered inlined cross-crate re-export

    pub fn f<'a, T>()
where
    'a: 'static,
    T: 'static,

    Order is lifetime bounds, then type bounds(, then equality bounds). This happens in simplify::where_clauses.

  13. fmease commented on Apr 16, 2024

    @fmease
    fmease
    on Apr 16, 2024
    MemberAuthor

    Minor: OPAQ: We don't necessarily preserve the order of bounds, we move traits in front of lifetime bounds.

  14. fmease commented on Apr 16, 2024

    @fmease
    fmease
    on Apr 16, 2024
    MemberAuthor

    Medium: WHERE & APIT: The cleaning process is order-dependent.

    pub trait TrA: TrB {}
pub trait TrB { type X; }

pub fn f<T: TrA + TrB<X = ()>>() {} // rendered IXCRX of the bound: `T: TrA<X = ()> + TrB`
pub fn g<T: TrB<X = ()> + TrA>() {} // rendered IXCRX of the bound: `T: TrB<X = ()> + TrA`
  15. fmease
    changed the title [-]rustdoc's resugaring of projection predicates to type bindings is flawed when supertraits are involved[/-] [+]rustdoc's resugaring of projection predicates to type bindings is flawed[/+] on Mar 12, 2025

  16. jethrogb commented on Mar 12, 2026

    @jethrogb
    jethrogb
    on Mar 12, 2026
    Contributor

    Is this the same bug? https://docs.rs/kube/latest/kube/trait.Resource.html

    This renders as:

    impl<K, S> Resource for K
where
    K: Metadata<Ty = ObjectMeta, Scope = S> + Resource,

    instead of the (IMO, much more logical):

    impl<K, S> Resource for K
where
    K: Metadata<Ty = ObjectMeta> + Resource<Scope = S>,

  17. fmease commented on Mar 18, 2026

    @fmease
    fmease
    on Mar 18, 2026
    MemberAuthor

    Yes it is. We should add the binding to the most specific trait bound. At the moment, it's order dependent, see also #113015 (comment). It can be boiled down to:

    pub trait Sub: Super { type X; }
pub trait Super { type Y; }

// IXCRE = inlined cross-crate re-export

// bound of IXCRE gets rendered as `Sub<X = (), Y = ()> + Super`
pub fn demo0<T: Sub<X = ()> + Super<Y = ()>>() {} 
// bound of IXCRE gets rendered as `Super<Y = ()> + Sub<X = ()>`
pub fn demo1<T: Super<Y = ()> + Sub<X = ()>>() {}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

A-cross-crate-reexportsArea: Documentation that has been re-exported from a different crateC-bugCategory: This is a bug.T-rustdocRelevant to the rustdoc team, which will review and decide on the PR/issue.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions