Rustdoc recursion limit issue

[jdonszelmann]

Today we ran into a weird issue while generating the rustdoc page for our project. As you can see here we run into a recursion limit issue. However increasing the recursion limit (#![recursion_limit = "4096"]) does not resolve it.

It worked on version 1.37.0-nightly-2019-06-21 but breaks on 1.37.0-nightly-2019-06-22

Any help would be appreciated.

Edit:
For reference as to how we got this error, here is our repo.

[nagisa]

Most likely caused by #60293, but my understanding was that with #60444 increasing the recursion limit should help.

[nagisa]

Just to make sure you are adding the recursion limit attribute to the synstructure crate, right?

[jdonszelmann]

We do not directly use the synstructure crate directly ourselves. It is listed as a dependency of PyO3.

Therefore we do not directly add it to the synstructure crate (or know how to).

[nagisa]

Minimal Cargo.toml to reproduce:

[package]
name = "a"
version = "0.1.0"

[dependencies]
synstructure = "0.10"
syn = { version = "0.15", features = ["full"] }

Adding #![recursion_limit="128"] to synstructure helps, however the leaf crates will still fail to document if they do something like this: pub struct A<'a>(pub synstructure::Structure<'a>); as those leaf crates will still have the original limit of 64. This is unlike

fn assert_unpin<T: std::marker::Unpin>() {}

pub fn foo() {
    assert_unpin::<Vec<syn::GenericParam>>();
}

which only needs the attribute in the crate which actually contains this code (i.e. it does not necessarily infect the dependent crates).

This is, sadly a deeper issue and the change in #60293 only happens to expose it. Any ideas what we could do here to remove/propagate/etc this limit when documenting cc @rust-lang/compiler?

cc @dtolnay you may want to be aware about std::vec::Vec<syn::generics::GenericParam> crating deep stacks when trying to prove implementations of autotraits.

[dtolnay]

Thanks @nagisa. Is there anything I can do about this on my end?

The trace in the error looks correct to me; this is a consequence of almost all Rust syntax being allowed inside of almost all other Rust syntax.

• std::vec::Vec<syn::GenericParam>
• contains alloc::raw_vec::RawVec<syn::GenericParam>
• contains std::ptr::Unique<syn::GenericParam>
• contains *const syn::GenericParam
• contains syn::GenericParam
• contains syn::TypeParam
• contains std::vec::Vec<syn::Attribute>
• contains alloc::raw_vec::RawVec<syn::Attribute>
• contains std::ptr::Unique<syn::Attribute>
• contains *const syn::Attribute
• contains syn::Attribute
• contains syn::Path
• contains syn::punctuated::Punctuated<syn::PathSegment, syn::token::Colon2>
• contains std::vec::Vec<(syn::PathSegment, syn::token::Colon2)>
• contains alloc::raw_vec::RawVec<(syn::PathSegment, syn::token::Colon2)>
• contains std::ptr::Unique<(syn::PathSegment, syn::token::Colon2)>
• contains *const (syn::PathSegment, syn::token::Colon2)
• contains (syn::PathSegment, syn::token::Colon2)
• contains syn::PathSegment
• contains syn::PathArguments
• contains syn::AngleBracketedGenericArguments
• contains syn::punctuated::Punctuated<syn::GenericArgument, syn::token::Comma>
• contains std::vec::Vec<(syn::GenericArgument, syn::token::Comma)>
• contains alloc::raw_vec::RawVec<(syn::GenericArgument, syn::token::Comma)>
• contains std::ptr::Unique<(syn::GenericArgument, syn::token::Comma)>
• contains *const (syn::GenericArgument, syn::token::Comma)
• contains (syn::GenericArgument, syn::token::Comma)
• contains syn::GenericArgument
• contains syn::Type
• contains syn::TypeArray
• contains syn::Expr
• contains syn::ExprIf
• contains syn::Block
• contains std::vec::Vec<syn::Stmt>
• contains alloc::raw_vec::RawVec<syn::Stmt>
• contains std::ptr::Unique<syn::Stmt>
• contains *const syn::Stmt
• contains syn::Stmt
• contains syn::Item
• contains syn::ItemType
• contains syn::Generics
• contains std::option::Option<syn::WhereClause>
• contains syn::WhereClause
• contains syn::punctuated::Punctuated<syn::WherePredicate, syn::token::Comma>
• contains std::vec::Vec<(syn::WherePredicate, syn::token::Comma)>
• contains alloc::raw_vec::RawVec<(syn::WherePredicate, syn::token::Comma)>
• contains std::ptr::Unique<(syn::WherePredicate, syn::token::Comma)>
• contains *const (syn::WherePredicate, syn::token::Comma)
• contains (syn::WherePredicate, syn::token::Comma)
• contains syn::WherePredicate
• contains syn::PredicateType
• contains std::option::Option<syn::BoundLifetimes>
• contains syn::BoundLifetimes
• contains syn::punctuated::Punctuated<syn::LifetimeDef, syn::token::Comma>
• contains std::vec::Vec<(syn::LifetimeDef, syn::token::Comma)>
• contains alloc::raw_vec::RawVec<(syn::LifetimeDef, syn::token::Comma)>
• contains std::ptr::Unique<(syn::LifetimeDef, syn::token::Comma)>
• contains *const (syn::LifetimeDef, syn::token::Comma)
• contains (syn::LifetimeDef, syn::token::Comma)
• contains syn::LifetimeDef
• contains syn::punctuated::Punctuated<syn::Lifetime, syn::token::Add>
• contains std::vec::Vec<(syn::Lifetime, syn::token::Add)>

[nagisa]

@dtolnay Other than reducing the depth of types somehow, there’s very little that can be done on your side. You could manually implement all the marker types for certain types, but the same issue will resurface for new/external auto-traits.

[dtolnay]

Does this warrant increasing the default recursion limit? I would expect the default to be selected such that it is higher than what you would need in any "reasonable" real world code. If the data structures in Syn are "reasonable" i.e. neither contrived nor somehow unsolvable regardless of recursion limit, then this suggests that the current limit is too aggressive. What are the implications of changing it?

[shepmaster]

This also affected fuzzy-pickles but there were some fixes that seemed to help there. Wonder why the difference.

[SimonSapin]

Most likely caused by #60293, but my understanding was that with #60444 increasing the recursion limit should help.

@nagisa, are you saying that the expected fix is to go through the ecosystem and add #![recursion_limit = "something"] as needed? Or, what do you think of raising the default limit, as suggested by dtolnay above?

[pnkfelix]

cc #61960, which tracks the instances where #60444 has hit an overflow during recursive trait constraint resolution.

(Also, I am assuming that this issue (#62059) is not resolved by PR #61754, but it would be good to double-check that assumption.)

---

@SimonSapin asked

@nagisa, are you saying that the expected fix is to go through the ecosystem and add #![recursion_limit = "something"] as needed? Or, what do you think of raising the default limit, as suggested by dtolnay above?

my hope is that many of the cases affected by PR #60444 will be addressed by PR #61754, which we recently approved for beta backport so that it lands in time for the release.

[SimonSapin]

Servo hit this issue in rustc 1.37.0-nightly (929b48e 2019-06-21), see #62132. This version does include #61754, so #61754 is not a sufficient fix.

[nagisa]

@SimonSapin I will probably just revert the patch to rustdoc if there is no other option than annotating crates with an attribute. rustdoc’s case is special in that it infects dependent crates much more easily and having such an attribute in a large number of crates is not a tenable solution.

Ideally rustc/rustdoc would work for arbitrarily deep types, but I can see raising the default limit as a good stop-gap as well.

[nagisa]

@pnkfelix at this point I doubt either of #60444 or #61754 are very related as the type-stack here seems to be very linear.

---

Nominating for a short section of time in the meeting to discuss the best way to proceed here. The non-involved solutions I know at the moment are:

• Increase the default recursion limit;
• Revert the rustdoc PR rustdoc: generate implementors for all auto traits #60293 which changes rustdoc to generate documentation for all autotraits, not just Send and Sync.

Will see if raising the limit is something that we could or should do and perhaps whether there are other options that I have missed.

[pnkfelix]

pre-triage: P-high. Leaving nominated in hopes we discuss it today. Leaving unassigned to reflect reality that I don't think I can take this on myself. (But maybe @nagisa can take point on ensuring this gets resolved in short order...?)

[pnkfelix]

discussed at compiler team meeting. @eddyb raised the idea that rustdoc could just silently ignore the cases where we hit the existing limit, and treat them as unimplemented traits.

In any case, based on assumption that this is indeed a stable-to-nightly regression, I think we have a little bit of breathing room to figure out how we want to address this long term.

Assigning to @nagisa to take point on resolving this, potentially via delegation. (@nagisa , let me know if that does not work for you.)

[SimonSapin]

This is blocking the upgrade of rustc in Servo. (And I get nagged about it every day by the corresponding CI job failing.)

@rust-lang/compiler Is anyone working on a revert or other fix, or should I go add recursion_limit to synstructure and other crates as needed?

[nagisa]

I’m looking into this and investigating the feasibility of implementing @eddyb’s idea or some other fix. Alas, I only have sufficiently long uninterrupted chunks of time on the weekends, so the progress is slow and is very plausible that this won’t be fixed within a few days.

[pnkfelix]

triage: assigning to @nagisa and myself for follow-up.

[pnkfelix]

@SimonSapin do you have any estimate of how many crates would require the recursion_limit addition?

[SimonSapin]

There is one that we know of in Servo’s dependency graph (synstructure). I don’t know if there are more.

[nagisa]

For the purposes of rustdoc, you’ll need to add attribute not only for the synstructure crate, but also recursively to any crate which depends on synstructure. I consider that to be unfeasible. So far I’m inclined to just bump the default recursion limit to 128 or something and solve it properly later, but as e.g. stack size increase has showed, proper fix sometimes fizzles out and the resource bump stays in place for unlimited amount of time.

[SimonSapin]

By running cargo doc -j 100 repeatedly, I find two affected crates in Servo’s dependency graph: synstructure and derivative. It looks like none of the dependent crates make types that are deeper still based on those.

[goddessfreya]

I've just bumped into this issue when trying to build glutin's docs.

https://termbin.com/oo0y

rustc 1.38.0-nightly (dfd52ba6a 2019-07-06)

Hoping this gets raised.