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

overflow evaluating the requirement <Self as FilterDsl<_>>::Output with 1.49.0 #80953

Closed
weiznich opened this issue Jan 12, 2021 · 13 comments
Closed
Labels
P-high High priority 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

@weiznich
Copy link
Contributor

Code

I've tried to compile wundergraph with the current stable release (1.49)

I expected that the code compiles because it worked with 1.48.

Instead, this happened: The code fails to compile with the following error message:

$ git clone https://github.com/weiznich/wundergraph
$ git checkout ed4986f3e24f38531726125783917bf61af0bbd0
$ cd wundergraph
$ cargo +1.49.0 check
    Checking wundergraph v0.1.2 (/home/weiznich/Dokumente/rust/wundergraph/wundergraph)
error[E0275]: overflow evaluating the requirement `<<<R as HasTable>::Table as AsQuery>::Query as FilterDsl<_>>::Output`
  --> wundergraph/src/query_builder/mutations/delete.rs:35:15
   |
35 |     R::Table: HandleDelete<R, D, DB, Ctx> + 'static,
   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
64 | pub trait HandleDelete<L, K, DB, Ctx> {
   | ------------------------------------- required by this bound in `HandleDelete`
   |
   = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`wundergraph`)
   = note: required because of the requirements on the impl of `HandleDelete<R, D, DB, Ctx>` for `<R as HasTable>::Table`

error[E0275]: overflow evaluating the requirement `<Self as FilterDsl<_>>::Output`
  --> wundergraph/src/query_builder/mutations/delete.rs:64:1
   |
64 | pub trait HandleDelete<L, K, DB, Ctx> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `HandleDelete`
   |
   = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`wundergraph`)
   = note: required because of the requirements on the impl of `HandleDelete<L, K, DB, Ctx>` for `Self`

error[E0275]: overflow evaluating the requirement `<<<R as HasTable>::Table as AsQuery>::Query as FilterDsl<_>>::Output`
  --> wundergraph/src/query_builder/mutations/update.rs:30:15
   |
30 |     R::Table: HandleUpdate<R, U, DB, Ctx> + 'static,
   |               ^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
62 | pub trait HandleUpdate<L, U, DB, Ctx> {
   | ------------------------------------- required by this bound in `HandleUpdate`
   |
   = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`wundergraph`)
   = note: required because of the requirements on the impl of `HandleUpdate<R, U, DB, Ctx>` for `<R as HasTable>::Table`

error[E0275]: overflow evaluating the requirement `<Self as FilterDsl<_>>::Output`
  --> wundergraph/src/query_builder/mutations/update.rs:62:1
   |
62 | pub trait HandleUpdate<L, U, DB, Ctx> {
   | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ required by this bound in `HandleUpdate`
   |
   = help: consider adding a `#![recursion_limit="256"]` attribute to your crate (`wundergraph`)
   = note: required because of the requirements on the impl of `HandleUpdate<L, U, DB, Ctx>` for `Self`

error: aborting due to 4 previous errors

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

Version it worked on

It most recently worked on: Rust 1.48

It most recently worked on:

Version with regression

rustc --version --verbose:

rustc 1.49.0 (e1884a8e3 2020-12-29)
binary: rustc
commit-hash: e1884a8e3c3e813aada8254edfa120e85bf5ffca
commit-date: 2020-12-29
host: x86_64-unknown-linux-gnu
release: 1.49.0

As a side note: This is another diesel/trait system related issue that occurred with the latest release. Could we please consider something like #79599 to prevent this in the future? I doesn't feel right for me that I need to spend a considerable amount of my free time for such regression to a language that calls it self stable in such a way that old code should continue to compile.

@rustbot rustbot added regression-from-stable-to-stable Performance or correctness regression from one stable version to another. I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 12, 2021
@SNCPlay42
Copy link
Contributor

A stable-to-stable regression like this should have been caught by crater, but it looks like wundergraph fails to build on crater.

@weiznich
Copy link
Contributor Author

@SNCPlay42 To just cite from the crate log you've linked: Wundergraph did not fail to build, crater did only fail to build the corresponding tests (which is understandable, because they are not included in the release, because that's impossible to do with cargo (as you cannot release a package that tests depends on another crate that depends on the released crate)).

[INFO] running `Command { std: "docker" "create" "-v" "/var/lib/crater-agent-workspace/builds/worker-11/target:/opt/rustwide/target:rw,Z" "-v" "/var/lib/crater-agent-workspace/builds/worker-11/source:/opt/rustwide/workdir:ro,Z" "-v" "/var/lib/crater-agent-workspace/cargo-home:/opt/rustwide/cargo-home:ro,Z" "-v" "/var/lib/crater-agent-workspace/rustup-home:/opt/rustwide/rustup-home:ro,Z" "-e" "SOURCE_DIR=/opt/rustwide/workdir" "-e" "CARGO_TARGET_DIR=/opt/rustwide/target" "-e" "CARGO_INCREMENTAL=0" "-e" "RUST_BACKTRACE=full" "-e" "RUSTFLAGS=--cap-lints=warn" "-e" "CARGO_HOME=/opt/rustwide/cargo-home" "-e" "RUSTUP_HOME=/opt/rustwide/rustup-home" "-w" "/opt/rustwide/workdir" "-m" "1610612736" "--user" "0:0" "--network" "none" "rustops/crates-build-env@sha256:c8ac004eab7d63a0ad09a2dde3d3353ba464f767bee4de425dc8f74c46a1905e" "/opt/rustwide/cargo-home/bin/cargo" "+1.48.0" "build" "--frozen" "--message-format=json", kill_on_drop: false }`
[INFO] [stdout] 33264a8499ee77cf4f9ca16eb4d17a79ee1f74e74a2fd20bd988e465e3f0fa63
[INFO] [stderr] WARNING: Your kernel does not support swap limit capabilities or the cgroup is not mounted. Memory limited without swap.
[INFO] running `Command { std: "docker" "start" "-a" "33264a8499ee77cf4f9ca16eb4d17a79ee1f74e74a2fd20bd988e465e3f0fa63", kill_on_drop: false }`
[INFO] [stderr]    Compiling syn v1.0.53
[INFO] [stderr]    Compiling serde_derive v1.0.117
[INFO] [stderr]    Compiling juniper_codegen v0.14.2
[INFO] [stderr]    Compiling diesel_derives v1.4.1
[INFO] [stderr]    Compiling thiserror-impl v1.0.22
[INFO] [stderr]    Compiling wundergraph_derive v0.1.0
[INFO] [stderr]    Compiling thiserror v1.0.22
[INFO] [stderr]    Compiling serde v1.0.117
[INFO] [stderr]    Compiling diesel v1.4.5
[INFO] [stderr]    Compiling indexmap v1.6.0
[INFO] [stderr]    Compiling juniper v0.14.2
[INFO] [stderr]    Compiling wundergraph v0.1.2 (/opt/rustwide/workdir)
[INFO] [stderr]     Finished dev [unoptimized + debuginfo] target(s) in 1m 03s

Anyway I believe crate should not just dismiss build results if the tests fail to build.

@SNCPlay42
Copy link
Contributor

Oops, looks like crater wouldn't have caught this then because the beta compiler also successfully built (the previous log I posted is the 1.48 compiler acting as a baseline) the crate without tests. (Was this regression introduced in a backport after the crater run, then?)

But it looks like crater really doesn't differentiate errors building the main crate from errors building the tests (it only differentiates errors running the tests). Filed rust-lang/crater#558

@weiznich
Copy link
Contributor Author

weiznich commented Jan 13, 2021

So likely related to #80417, as blind guess I would say it's #80246, so cc @matthewjasper

@SNCPlay42
Copy link
Contributor

Reduction: (AlternateTable, AlternateQuery and their impls aren't necessary to reproduce the issue, but without them Filter might look like it really is recursively defined, so this shows that that's not necessarily the case):

struct AlternateTable {}
struct AlternateQuery {}

pub trait Query {}
pub trait AsQuery {
    type Query;
}
impl<T: Query> AsQuery for T {
    type Query = Self;
}
impl AsQuery for AlternateTable {
    type Query = AlternateQuery;
}

pub trait Table: AsQuery {
    type PrimaryKey;
}
impl Table for AlternateTable {
    type PrimaryKey = ();
}

pub trait FilterDsl<Predicate> {
    type Output;
}
pub type Filter<Source, Predicate> = <Source as FilterDsl<Predicate>>::Output;
impl<T, Predicate> FilterDsl<Predicate> for T
where
    T: Table,
    T::Query: FilterDsl<Predicate>,
{
    type Output = Filter<T::Query, Predicate>;
}
impl<Predicate> FilterDsl<Predicate> for AlternateQuery {
    type Output = &'static str;
}

pub trait HandleDelete {
    type Filter;
}
impl<T> HandleDelete for T
where
    T: Table,
    T::Query: FilterDsl<T::PrimaryKey>,
    Filter<T::Query, T::PrimaryKey>: ,
{
    type Filter = Filter<T::Query, T::PrimaryKey>;
}

fn main() {
    let x: <AlternateTable as HandleDelete>::Filter = "Hello, world";
    println!("{}", x);
}

The code compiles successfully if:

  • The empty predicate Filter<T::Query, T::PrimaryKey>: is removed from the where clause
  • The references to T::PrimaryKey are replaced, e.g. with ()

@SNCPlay42
Copy link
Contributor

searched nightlies: from nightly-2020-10-01 to nightly-2021-01-13
regressed nightly: nightly-2020-12-27
searched commits: from bb17823 to 0b644e4
regressed commit: 931aa27

bisected with cargo-bisect-rustc v0.6.0

Host triple: x86_64-unknown-linux-gnu
Reproduce with:

cargo bisect-rustc --preserve --start=2020-10-01 -- check 

confirmed to be #80246.

@weiznich
Copy link
Contributor Author

So maybe also related to #80807, with the difference that this worked on some older release.

@matthewjasper matthewjasper added the T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. label Jan 14, 2021
@apiraino
Copy link
Contributor

Assigning P-high, Prioritization Working Group procedure Zulip thread and removing I-prioritize.

@apiraino apiraino added P-high High priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Jan 20, 2021
Dylan-DPC-zz pushed a commit to Dylan-DPC-zz/rust that referenced this issue Jan 28, 2021
…=nikomatsakis

Make hitting the recursion limit in projection non-fatal

This change was originally made in rust-lang#80246 to avoid future (effectively) infinite loop bugs in projections,
but wundergraph relies on rustc recovering here.

cc rust-lang#80953

r? `@nikomatsakis`
bors added a commit to rust-lang-ci/rust that referenced this issue Jan 28, 2021
…ikomatsakis

Make hitting the recursion limit in projection non-fatal

This change was originally made in rust-lang#80246 to avoid future (effectively) infinite loop bugs in projections,
but wundergraph relies on rustc recovering here.

cc rust-lang#80953

r? `@nikomatsakis`
ehuss pushed a commit to ehuss/rust that referenced this issue Feb 5, 2021
…ikomatsakis

Make hitting the recursion limit in projection non-fatal

This change was originally made in rust-lang#80246 to avoid future (effectively) infinite loop bugs in projections,
but wundergraph relies on rustc recovering here.

cc rust-lang#80953

r? `@nikomatsakis`
@jackh726
Copy link
Member

jackh726 commented Sep 2, 2021

Okay, so this worked in 1.48.0, broke in 1.49.0, and was fixed in 1.50.0 by #81055. A stable backport was declined, and this issue should have been closed then. Closing now.

@jackh726 jackh726 closed this as completed Sep 2, 2021
@weiznich
Copy link
Contributor Author

weiznich commented Sep 3, 2021

@jackh726 Is the crater issue fixed, so that similar regressions wont be ignored in the future again?

@jackh726
Copy link
Member

jackh726 commented Sep 3, 2021

Yes there's a regression test src/test/ui/associated-types/project-recursion-limit-non-fatal.rs

@weiznich
Copy link
Contributor Author

weiznich commented Sep 3, 2021

@jackh726 I do not mean this concrete rustc bug, but this crater bug that lead to ignoring this regression.

@jackh726
Copy link
Member

jackh726 commented Sep 3, 2021

I'm not sure. Since the issue (rust-lang/crater#558) is still open, I would assume that bug remains. But that is tracked separately from here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P-high High priority 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

6 participants