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

On recursive requirements, elide repeating output #47720

Closed
estebank opened this issue Jan 24, 2018 · 5 comments
Closed

On recursive requirements, elide repeating output #47720

estebank opened this issue Jan 24, 2018 · 5 comments
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@estebank
Copy link
Contributor

When encountering recursive requirements, elide some of the repeating output instead of having the following wall of text detailing every recursive step the compiler takes:

error[E0275]: overflow evaluating the requirement `Foo: std::marker::Sync`
 --> src/main.rs:5:1
  |
5 | / lazy_static! {
6 | |     static ref CHAR: Foo = unimplemented!();
7 | | }
  | |_^
  |
  = help: consider adding a `#![recursion_limit="128"]` attribute to your crate
  = note: required because it appears within the type `std::marker::PhantomData<Foo>`
  = note: required because it appears within the type `Bar`
  = note: required because it appears within the type `std::marker::PhantomData<Bar>`
  = note: required because it appears within the type `Foo`
  = note: required because it appears within the type `std::marker::PhantomData<Foo>`
  = note: required because it appears within the type `Bar`
  = note: required because it appears within the type `std::marker::PhantomData<Bar>`
  = note: required because it appears within the type `Foo`
  = note: required because it appears within the type `std::marker::PhantomData<Foo>`
  = note: required because it appears within the type `Bar`
8<8<8<8<8<8<8<
  = note: required by `lazy_static::lazy::Lazy`
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)

The following code

ObligationCauseCode::BuiltinDerivedObligation(ref data) => {
let parent_trait_ref = self.resolve_type_vars_if_possible(&data.parent_trait_ref);
err.note(&format!("required because it appears within the type `{}`",
parent_trait_ref.0.self_ty()));
let parent_predicate = parent_trait_ref.to_predicate();
self.note_obligation_cause_code(err,
&parent_predicate,
&data.parent_code);
}

needs to be modified to pass down a vector of the Tys note_obligation_cause_code has encountered on previous runs. I feel it is reasonable to delay the notes from being produced until the bottom has been reached, and prune the list then. The output should be something along the lines of

error[E0275]: overflow evaluating the requirement `Foo: std::marker::Sync`
 --> src/main.rs:5:1
  |
5 | / lazy_static! {
6 | |     static ref CHAR: Foo = unimplemented!();
7 | | }
  | |_^
  |
  = help: consider adding a `#![recursion_limit="128"]` attribute to your crate
  = note: required because it appears within the type `std::marker::PhantomData<Foo>`
  = note: required because it appears within the type `Bar`
  = note: required because it appears within the type `std::marker::PhantomData<Bar>`
  = note: required because it appears within the type `Foo`
  = note: required because it appears within the type `std::marker::PhantomData<Foo>`
  = note: ...and so on, these requirements repeat until reaching the recursion limit
  = note: required by `lazy_static::lazy::Lazy`
  = note: this error originates in a macro outside of the current crate (in Nightly builds, run with -Z external-macro-backtrace for more info)
@estebank estebank added C-enhancement Category: An issue proposing an enhancement or a PR with one. A-diagnostics Area: Messages for errors, warnings, and lints E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Jan 24, 2018
@hrvolapeter
Copy link
Member

hrvolapeter commented Jan 24, 2018

Hello @estebank, I would like to take this one but I could use some more clarification. Just to be clear, your solution expects to:

  1. Add new argument of type vector to note_obligation_cause_code
  2. Move err.note from line 1287 to 1292.
  3. Add errors to vector

and when end of recursion is reached:

  1. Print last 4 errors
  2. prune vector

@estebank
Copy link
Contributor Author

I'm ok with any solution you come up with. The one you have mentioned should work fine. In any way you'll probably have to modify the way note_obligation_cause_code works. For instead of pruning the vector I would look for repeated sequences in the vector, and only print the repeating sequence once. It wouldn't be hard coded as the last 4, as if you add a level of indirection in the repro case it would have to be the last 6, for example.

BTW, I haven't come up with a repro that doesn't depend on lazy_static. That is something we should figure out. You can test it locally using rustc lazy_static/src/lib.rs --crate-type=lib and then using that library $DIR/rustc src/main.rs --extern lazy_static=liblib.rlib, but having a test in this repo would be appreciated to have to avoid regressions.

@CAD97
Copy link
Contributor

CAD97 commented Jan 24, 2018

Minimal reproduction:

use std::marker::PhantomData;

struct AssertSync<T: Sync>(PhantomData<T>);

pub struct Foo {
    bar: *const Bar,
    phantom: PhantomData<Bar>,
}

pub struct Bar {
    foo: *const Foo,
    phantom: PhantomData<Foo>,
}

fn main() {
    let _: AssertSync<Foo> = unimplemented!();
}

bors added a commit that referenced this issue Feb 13, 2018
Optimized error reporting for recursive requirements #47720

Fixes #47720
@CAD97
Copy link
Contributor

CAD97 commented Mar 26, 2020

I found a new instance of this. [playground]

use std::ops::Deref;

fn assert_TextSized<T: TextSized>() {}

/// Text-like structures that have a text size.
pub trait TextSized: Copy {
    /// The size of this text-alike.
    fn text_size(self) -> usize;
}

impl TextSized for &'_ str {
    fn text_size(self) -> usize {
        self.len()
    }
}

impl<D: Deref> TextSized for &'_ D
where for<'a> &'a D::Target: TextSized
{
    fn text_size(self) -> usize {
        self.deref().text_size()
    }
}

const _: fn() = || {
    assert_TextSized::<&()>();
};
error[E0275]: overflow evaluating the requirement `for<'a> &'a _: TextSized`
  --> src/lib.rs:28:5
   |
5  | fn assert_TextSized<T: TextSized>() {}
   |    ----------------    --------- required by this bound in `assert_TextSized`
...
28 |     assert_TextSized::<&()>();
   |     ^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`playground`)
   = note: required because of the requirements on the impl of `for<'a> TextSized` for `&'a _`
     [snip about 123 copies of that line]
   = note: required because of the requirements on the impl of `for<'a> TextSized` for `&'a _`
   = note: required because of the requirements on the impl of `TextSized` for `&()`

error: aborting due to previous error

For more information about this error, try `rustc --explain E0275`.
error: could not compile `playground`.

(edited to make the example actually compile for a correct type)

@estebank estebank reopened this Mar 26, 2020
@estebank estebank added D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. and removed E-mentor Call for participation: This issue has a mentor. Use #t-compiler/help on Zulip for discussion. labels Mar 26, 2020
@estebank
Copy link
Contributor Author

Current output

error[E0275]: overflow evaluating the requirement `for<'a> &'a _: TextSized`
  --> src/lib.rs:28:5
   |
28 |     assert_TextSized::<&()>();
   |     ^^^^^^^^^^^^^^^^^^^^^^^
   |
   = help: consider increasing the recursion limit by adding a `#![recursion_limit = "256"]` attribute to your crate (`playground`)
note: required for `&'a _` to implement `for<'a> TextSized`
  --> src/lib.rs:19:16
   |
19 | impl<D: Deref> TextSized for &'_ D
   |                ^^^^^^^^^     ^^^^^
   = note: 126 redundant requirements hidden
   = note: required for `&()` to implement `TextSized`
note: required by a bound in `assert_TextSized`
  --> src/lib.rs:5:24
   |
5  | fn assert_TextSized<T: TextSized>() {}
   |                        ^^^^^^^^^ required by this bound in `assert_TextSized`

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-diagnostics Area: Messages for errors, warnings, and lints C-enhancement Category: An issue proposing an enhancement or a PR with one. D-verbose Diagnostics: Too much output caused by a single piece of incorrect code. 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

3 participants