Skip to content
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

Internal compiler error when documenting capnp #51236

Closed
juchiast opened this issue May 31, 2018 · 20 comments
Closed

Internal compiler error when documenting capnp #51236

juchiast opened this issue May 31, 2018 · 20 comments
Labels
A-traits Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@juchiast
Copy link
Contributor

Cargo.toml:

[dependencies]
capnp = "0.8.17"

Result when run cargo doc:

thread '<unnamed>' panicked at 'Unable to fulfill trait DefId(2/0:864 ~ core[b3f3]::marker[0]::Send[0]) for 'list_list::Owned<T>': [FulfillmentError(Obligation(predicate=Binder(ProjectionPredicate(ProjectionTy { substs: [T, ReStatic], item_def_id: DefId(0/0:1150 ~ lib[8787]::traits[0]::Owned[0]::Reader[0]) }, _)),depth=2),Ambiguity), FulfillmentError(Obligation(predicate=Binder(TraitPredicate(<_ as std::marker::Send>)),depth=2),Ambiguity)]', librustc/traits/auto_trait.rs:218:17
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.28.0-nightly (5d0631a64 2018-05-30) running on x86_64-unknown-linux-gnu

error: Could not document `capnp`.

Caused by:
  process didn't exit successfully: `rustdoc --crate-name capnp /home/qwe/.cargo/registry/src/github.com-1ecc6299db9ec823/capnp-0.8.17/src/lib.rs -o /home/qwe/Desktop/upw/test_rustdoc/target/doc -L dependency=/home/qwe/Desktop/upw/test_rustdoc/target/debug/deps --extern byteorder=/home/qwe/Desktop/upw/test_rustdoc/target/debug/deps/libbyteorder-3355ea0a902fb6f6.rmeta` (exit code: 101)

rustc version: nightly-x86_64-unknown-linux-gnu (5d0631a 2018-05-30)
Also happen in nightly-2018-05-29 and nightly-2018-05-23.

@juchiast
Copy link
Contributor Author

UPDATE

Also happen on rustc 1.26.1

thread 'rustc' panicked at 'Unable to fulfill trait DefId(2/0:885 ~ core[3db6]::marker[0]::Send[0]) for 'list_list::Owned<T>': [FulfillmentError(Obligation(predicate=Binder(ProjectionPredicate(ProjectionTy { substs: Slice([T, ReStatic]), item_def_id: DefId(0/0:1150 ~ lib[8787]::traits[0]::Owned[0]::Reader[0]) }, _)),depth=2),Ambiguity), FulfillmentError(Obligation(predicate=Binder(TraitPredicate(<_ as std::marker::Send>)),depth=2),Ambiguity)]', librustdoc/clean/auto_trait.rs:401:17
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.26.1 (827013a31 2018-05-25) running on x86_64-unknown-linux-gnu

error: Could not document `capnp`.

Caused by:
  process didn't exit successfully: `rustdoc --crate-name capnp /home/qwe/.cargo/registry/src/github.com-1ecc6299db9ec823/capnp-0.8.17/src/lib.rs -o /home/qwe/Desktop/upw/test_rustdoc/target/doc -L dependency=/home/qwe/Desktop/upw/test_rustdoc/target/debug/deps --extern byteorder=/home/qwe/Desktop/upw/test_rustdoc/target/debug/deps/libbyteorder-52361c0693377ac0.rmeta` (exit code: 101)

@GuillaumeGomez GuillaumeGomez added the T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. label May 31, 2018
@QuietMisdreavus
Copy link
Member

QuietMisdreavus commented Jun 14, 2018

Minimal reproduction:

use std::marker::PhantomData;

pub mod traits {
    pub trait Owned<'a> {
        type Reader;
    }
}

pub struct Owned<T> where T: for<'a> ::traits::Owned<'a> {
    marker: PhantomData<<T as ::traits::Owned<'static>>::Reader>,
}

Maybe some kind of mismatch between the HRTB on the struct definition and the 'static used in the marker?

EDIT: This is happening when figuring out the auto-trait impls on the Owned struct. It looks like @varkor and @ibabushkin were the last ones to touch that? We'll probably need to bring in a compiler person, depending on what the code is asking the compiler for.

@QuietMisdreavus QuietMisdreavus added the I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ label Jun 14, 2018
@coder543
Copy link

coder543 commented Jun 20, 2018

Happens on current stable* and current nightly**, so this is interesting. I guess I can't build local documentation for my dependencies until this is fixed.

*  rustc 1.26.2 (594fb253c 2018-06-01)
** rustc 1.28.0-nightly (f28c7aef7 2018-06-19)

@eddyb eddyb added A-traits Area: Trait system T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed T-rustdoc Relevant to the rustdoc team, which will review and decide on the PR/issue. labels Jun 22, 2018
@eddyb
Copy link
Member

eddyb commented Jun 22, 2018

cc @nikomatsakis @Aaron1011 So this is what's failing

fulfill.select_all_or_error(&infcx).unwrap_or_else(|e| {
panic!(
"Unable to fulfill trait {:?} for '{:?}': {:?}",
trait_did, ty, e
)
});

The huge comment before all of this suggests to me that it's a bit fragile and this is an edge case:
// Due to the way projections are handled by SelectionContext, we need to run
// evaluate_predicates twice: once on the original param env, and once on the result of
// the first evaluate_predicates call.
//
// The problem is this: most of rustc, including SelectionContext and traits::project,
// are designed to work with a concrete usage of a type (e.g. Vec<u8>
// fn<T>() { Vec<T> }. This information will generally never change - given
// the 'T' in fn<T>() { ... }, we'll never know anything else about 'T'.
// If we're unable to prove that 'T' implements a particular trait, we're done -
// there's nothing left to do but error out.
//
// However, synthesizing an auto trait impl works differently. Here, we start out with
// a set of initial conditions - the ParamEnv of the struct/enum/union we're dealing
// with - and progressively discover the conditions we need to fulfill for it to
// implement a certain auto trait. This ends up breaking two assumptions made by trait
// selection and projection:
//
// * We can always cache the result of a particular trait selection for the lifetime of
// an InfCtxt
// * Given a projection bound such as '<T as SomeTrait>::SomeItem = K', if 'T:
// SomeTrait' doesn't hold, then we don't need to care about the 'SomeItem = K'
//
// We fix the first assumption by manually clearing out all of the InferCtxt's caches
// in between calls to SelectionContext.select. This allows us to keep all of the
// intermediate types we create bound to the 'tcx lifetime, rather than needing to lift
// them between calls.
//
// We fix the second assumption by reprocessing the result of our first call to
// evaluate_predicates. Using the example of '<T as SomeTrait>::SomeItem = K', our first
// pass will pick up 'T: SomeTrait', but not 'SomeItem = K'. On our second pass,
// traits::project will see that 'T: SomeTrait' is in our ParamEnv, allowing
// SelectionContext to return it back to us.

@QuietMisdreavus
Copy link
Member

QuietMisdreavus commented Jun 22, 2018

Debug output when run on my sample:

$ RUST_LOG=rustc::traits::auto_trait rustdoc +local -o asdf l.rs
DEBUG 2018-06-22T19:03:59Z: rustc::traits::auto_trait: evaluate_nested_obligations(ty_did=DefId(0/0:7 ~ l[8787]::Owned[0]), trait_did=DefId(2/0:865 ~ core[1c6a]::marker[0]::Send[0])): succeeded with 'ParamEnv { caller_bounds: [Binder(TraitPredicate(<T as traits::Owned<'a>>)), Binder(TraitPredicate(<T
as std::marker::Sized>)), Binder(TraitPredicate(<<T as traits::Owned<'static>>::Reader as std::marker::Send>)), Binder(TraitPredicate(<T as traits::Owned<'static>>))], reveal: UserFacing }' 'ParamEnv { caller_bounds: [Binder(TraitPredicate(<T as traits::Owned<'static>>)), Binder(TraitPredicate(<<T as
traits::Owned<'static>>::Reader as std::marker::Send>)), Binder(TraitPredicate(<T as std::marker::Sized>)), Binder(TraitPredicate(<T as traits::Owned<'a>>))], reveal: UserFacing }'
DEBUG 2018-06-22T19:03:59Z: rustc::traits::auto_trait: evaluate_nested_obligations(ty_did=DefId(0/0:7 ~ l[8787]::Owned[0]), trait_did=DefId(2/0:865 ~ core[1c6a]::marker[0]::Send[0])): succeeded with 'ParamEnv { caller_bounds: [Binder(TraitPredicate(<T as traits::Owned<'a>>)), Binder(TraitPredicate(<T
as std::marker::Sized>)), Binder(TraitPredicate(<<T as traits::Owned<'static>>::Reader as std::marker::Send>)), Binder(TraitPredicate(<T as traits::Owned<'static>>))], reveal: UserFacing }' 'ParamEnv { caller_bounds: [Binder(TraitPredicate(<T as traits::Owned<'static>>)), Binder(TraitPredicate(<<T as
traits::Owned<'static>>::Reader as std::marker::Send>)), Binder(TraitPredicate(<T as std::marker::Sized>)), Binder(TraitPredicate(<T as traits::Owned<'a>>))], reveal: UserFacing }'
DEBUG 2018-06-22T19:03:59Z: rustc::traits::auto_trait: find_auto_trait_generics(did=DefId(0/0:7 ~ l[8787]::Owned[0]), trait_did=DefId(2/0:865 ~ core[1c6a]::marker[0]::Send[0]), generics=Generics { parent: None, parent_count: 0, params: [Type(T, DefId(0/1:10 ~ l[8787]::Owned[0]::T[0]), 0)], param_def_id_to_index: {DefId(0/1:10 ~ l[8787]::Owned[0]::T[0]): 0}, has_self: false, has_late_bound_regions: None }): fulfilling with ParamEnv { caller_bounds: [Binder(TraitPredicate(<T as traits::Owned<'a>>)), Binder(TraitPredicate(<T as std::marker::Sized>)), Binder(TraitPredicate(<<T as traits::Owned<'static>>::Reader as std::marker::Send>)), Binder(TraitPredicate(<T as traits::Owned<'static>>))], reveal: UserFacing }
thread '<unnamed>' panicked at 'Unable to fulfill trait DefId(2/0:865 ~ core[1c6a]::marker[0]::Send[0]) for 'Owned<T>': [FulfillmentError(Obligation(predicate=Binder(ProjectionPredicate(ProjectionTy { substs: [T, ReStatic], item_def_id: DefId(0/0:6 ~ l[8787]::traits[0]::Owned[0]::Reader[0]) }, _)),depth=2),Ambiguity), FulfillmentError(Obligation(predicate=Binder(TraitPredicate(<_ as std::marker::Send>)),depth=2),Ambiguity)]', librustc/traits/auto_trait.rs:218:17
note: Run with `RUST_BACKTRACE=1` for a backtrace.

error: internal compiler error: unexpected panic

note: the compiler unexpectedly panicked. this is a bug.

note: we would appreciate a bug report: https://github.com/rust-lang/rust/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.28.0-dev running on x86_64-unknown-linux-gnu

@Aaron1011
Copy link
Member

Here's a slightly more simplified reproduction:

use std::marker::PhantomData;

pub trait OwnedTrait<'a> {
    type Reader;
}

pub struct Owned<T> where T: for<'a> OwnedTrait<'a> {
    marker: PhantomData<<T as OwnedTrait<'static>>::Reader>,
}

@Aaron1011
Copy link
Member

I'd like to work on this.

@QuietMisdreavus
Copy link
Member

I wonder if #50159 is related? It's not the same failure, but it's another failure when synthesizing impls for rustdoc, and it also deals with associated types as type parameters in struct fields.

@Aaron1011
Copy link
Member

Initial thoughts:

The issue seems to be related to the fact that the auto trait finder picks up the following two bounds;

<T as OwnedTrait<'a>>
<T as OwnedTrait<'static>>

The first bounds is part of the initial param env (where T: for<'a> OwnedTrait<'a>), while the second gets picked up as a result of attempting to normalize <T as OwnedTrait<'static>>::Reader.

Once both of these predicates have been added to the ParamEnv, we run into issues the next time that SelectionContext.select is called, since both will end up as matching candidates.

I'm not entirely certain what the best way to resolve this is. One solution might be to inspect the generated ParamEnv after each call to select, and check for predicates that differ only in their Substs. However, it's not clear to me how to do this in a way that ensures that we still generate the correct bounds - i.e we need to ensure that the bounds we display to the user are actually sufficient to make it implement the auto trait.

@Aaron1011
Copy link
Member

Some further thoughts:

I think I can make the de-duplication approach work. Given two trait predicates that differ only in their lifetime substations, we should always choose the 'more strict' one. This means always choosing a predicate involving a HRTB over one involving 'static. Requiring that we be able to choose any lifetime in the type means that it's fine to pick 'static.

If I'm correct in my understanding of trait selection and fulfillment, this shouldn't lead to any 'unsound' (not actually sufficient) bounds being displayed to user. This de-duplication will take place before we run FulfillmentContext run, which means that we'll get an error if our computed bounds are insufficient.

@gabrfarina
Copy link

Just wondering: is there any way to work around this issue and produce documentation for a crate depending on capnp while this ICE gets addressed?

@coder543
Copy link

coder543 commented Jun 29, 2018 via email

@gabrfarina
Copy link

@coder543 Ah, great, thanks! 😄

@juchiast
Copy link
Contributor Author

Hi, is anyone fixing this issue? My project requires nightly so I cannot use 1.25.0 .

I think I can try to fix this issue, I will need some guides, though (first time editing rustc's code).

@QuietMisdreavus
Copy link
Member

A better workaround, if you're not exporting the Owned struct in your own crate, is to use cargo doc --no-deps or cargo rustdoc, which will only run documentation for your own crate.

@juchiast
Copy link
Contributor Author

@QuietMisdreavus Thanks! That works!

@coder543
Copy link

coder543 commented Aug 1, 2018

@QuietMisdreavus that's not really a solution, since I need to be able to see the docs for my dependencies. Relying on docs.rs is way more annoying than just having all of my documentation in one place. I can't build with 1.25.0 anymore, since some of my deps require minimum versions greater than that.

It would be super duper appreciated if this could at least get fixed on nightly, somehow. This issue is a very serious workflow impediment.

@QuietMisdreavus QuietMisdreavus added the regression-from-stable-to-stable Performance or correctness regression from one stable version to another. label Aug 1, 2018
@QuietMisdreavus
Copy link
Member

Tagging as regression-from-stable-to-stable as 1.27.0 exhibits this error.

@rust-lang/compiler @Aaron1011 Can we get some renewed attention on this?

@Aaron1011
Copy link
Member

I'm taking a look at this now.

bors added a commit that referenced this issue Aug 6, 2018
Fix ICE when rustdoc encounters certain usages of HRTBs

Fixes #51236

Under certain circumstances, `AutoTraitFinder` could end up computing a `ParamEnv` involving two trait predicates that differed only in the region parameters involved. One of these parameters would be a HRTB, while the other would be a normal region parameter.

When this `ParamEnv` was later passed to `SelectionContext`, an `Ambiguity` error would occur, since the erased versions of these predicates would be identical. To solve the issue, we de-duplicate our list of predicates as we build it up. Whenever we encounter two predicates that differ only in their assignment of region parameters (a HRTB vs a normal lifetime parameter), we pick the HRTB. This corresponds to selecting a 'stricter' bound to display in the generated documentation: we're requiring that a particular type works for all possible lifetime parameters if it's going to implement a particular auto trait.
@coder543
Copy link

coder543 commented Aug 7, 2018

@Aaron1011 Thanks! it looks like there wasn't a new nightly last night, so I can't test it yet, but I look forward to it!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-traits Area: Trait system I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ regression-from-stable-to-stable Performance or correctness regression from one stable version to another. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

No branches or pull requests

7 participants