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

Regression in diesel with 2020-01-15 nightly: overflow evaluating requirement #68264

Closed
jebrosen opened this issue Jan 16, 2020 · 8 comments · Fixed by #68297
Closed

Regression in diesel with 2020-01-15 nightly: overflow evaluating requirement #68264

jebrosen opened this issue Jan 16, 2020 · 8 comments · Fixed by #68297
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.

Comments

@jebrosen
Copy link
Contributor

jebrosen commented Jan 16, 2020

rustc version: rustc 1.42.0-nightly (3291ae339 2020-01-15)

.../code/diesel/diesel (master=) ✖ RUSTFLAGS="--cap-lints=warn" cargo +nightly check
   Compiling proc-macro2 v1.0.7
   Compiling unicode-xid v0.2.0
   Compiling syn v1.0.13
   Compiling byteorder v1.3.2
   Compiling quote v1.0.2
   Compiling diesel_derives v1.4.1 (/home/jeb/code/diesel/diesel_derives)
    Checking diesel v1.4.3 (/home/jeb/code/diesel/diesel)
warning: use of deprecated item 'std::error::Error::description': use the Display impl or to_string()
(... trimmed many instances of this warning...)
warning: use of deprecated item 'std::error::Error::description': use the Display impl or to_string()
   --> diesel/src/migration/errors.rs:108:43
    |
108 |         write!(f, "Failed with: {}", self.description())
    |                                           ^^^^^^^^^^^

error[E0275]: overflow evaluating the requirement `<Self as query_dsl::limit_dsl::LimitDsl>::Output`

error: aborting due to previous error

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

To learn more, run the command again with --verbose.

Notably, there is no information about where the error actually is. rustc 1.42.0-nightly (31dd4f4ac 2020-01-13) was fine. I also see a passing CI run on diesel for nightly on commit 8a87b945b27.

EDIT: I did run this test on the master branch of diesel, but I first discovered the problem in diesel 1.4.3.

@csmoe csmoe added the E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc label Jan 16, 2020
@Centril Centril added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. T-lang Relevant to the language team, which will review and decide on the PR/issue. labels Jan 16, 2020
@RalfJung RalfJung added the regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. label Jan 16, 2020
@RalfJung
Copy link
Member

The same code (diesel 1.4.3) builds fine in stable, so marking as regression.

@weiznich
Copy link
Contributor

weiznich commented Jan 16, 2020

There is a bisec in the linked diesel issue.

Bisect was able to find 6d0bb91 as the culprit, but it's a rollup.

@oli-obk
Copy link
Contributor

oli-obk commented Jan 16, 2020

Likely #67914 cc @Aaron1011

@Centril Centril removed the T-lang Relevant to the language team, which will review and decide on the PR/issue. label Jan 16, 2020
@Aaron1011
Copy link
Member

Aaron1011 commented Jan 16, 2020

It looks like we need to run normalization, not just fulfillment, in canonical mode.

I'll open a PR later today

@Aaron1011
Copy link
Member

Aaron1011 commented Jan 16, 2020

Unfortunately, normalization and projection don't have the equivalent of SelectionContext's "canonical mode" - they always report overflow errors. I'm somewhat hesitant to add such a mode, as I think this would be the only use-case.

The root issue here is that const-prop is very much "best effort" - it's okay to bail out if we're uncertain about something. However, trait selection and projection are very much not "best effort" - if we can't prove something, we tend to need to raise an error to avoid unsoundness. A lot of code is designed around this principle (e.g. normalize does not return a Result).

@jonas-schievink jonas-schievink added E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example C-bug Category: This is a bug. and removed E-needs-bisection Call for participation: This issue needs bisection: https://github.com/rust-lang/cargo-bisect-rustc labels Jan 17, 2020
@weiznich
Copy link
Contributor

A minimal example to reproduce is this:

pub trait Query {}

pub trait AsQuery {
    type Query: Query;
}
pub trait Table: AsQuery + Sized {}

pub trait LimitDsl {
    type Output;
}

pub(crate) trait LoadQuery<Conn, U>: RunQueryDsl<Conn> {}

impl<T: Query> AsQuery for T {
    type Query = Self;
}

impl<T> LimitDsl for T
where
    T: Table,
    T::Query: LimitDsl,
{
    type Output = <T::Query as LimitDsl>::Output;
}

pub(crate) trait RunQueryDsl<Conn>: Sized {
    fn first<U>(self, _conn: &Conn) -> U
    where
        Self: LimitDsl,
        Self::Output: LoadQuery<Conn, U>,
    {
        // Overflow is caused by this function body
        unimplemented!()
    }
}

Please note that this is only reproducible in a library crate, but not in a binary crate (So put it in lib.rs, not in main.rs).

@jonas-schievink jonas-schievink removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Jan 17, 2020
agersant added a commit to agersant/polaris that referenced this issue Jan 18, 2020
JohnTitor added a commit to JohnTitor/rust that referenced this issue Jan 20, 2020
…, r=oli-obk

 Filter and test predicates using `normalize_and_test_predicates` for const-prop

Fixes rust-lang#68264

Previously, I attempted to use
`substitute_normalize_and_test_predicates` to detect unsatisfiable
bounds. Unfortunately, since const-prop runs in a generic environment
(we don't have any of the function's generic parameters substituted),
this could lead to cycle errors when attempting to normalize predicates.

This check is replaced with a more precise check. We now only call
`normalize_and_test_predicates` on predicates that have the possibility
of being proved unsatisfiable - that is, predicates that don't depend
on anything local to the function (e.g. generic parameters). This
ensures that we don't hit cycle errors when we normalize said
predicates, while still ensuring that we detect unsatisfiable
predicates.

I haven't been able to come up with a minimization of the Diesel issue - however, I've verified that it compiles successfully.
@bors bors closed this as completed in 171fe82 Jan 21, 2020
@jh0l
Copy link

jh0l commented Jan 25, 2020

I got this error on diesel 1.4.3 but I guess the difference was I was on rust nightly. Updating rust to nightly 2020-1-25 fixed the issue. spooky

@AshtonKem
Copy link

I'm getting this issue while installing the diesel crate on nightly 2020-01-19. Updating to 2020-1-26 fixed the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-bug Category: This is a bug. regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. T-compiler Relevant to the compiler team, which will review and decide on the PR/issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants