Skip to content

Type inference regression and ICE in nightly 2020-10-06 #77638

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Closed
mattico opened this issue Oct 7, 2020 · 18 comments
Closed

Type inference regression and ICE in nightly 2020-10-06 #77638

mattico opened this issue Oct 7, 2020 · 18 comments
Assignees
Labels
A-associated-items Area: Associated items (types, constants & functions) A-inference Area: Type inference C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-critical Critical priority 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

@mattico
Copy link
Contributor

mattico commented Oct 7, 2020

I tried this code:

FluenTech/embedded-time@521569d

I expected to see this happen:

Successful compilation. This works with stable, and nightly 2020-10-05.

Instead, this happened:

https://github.com/FluenTech/embedded-time/pull/77/checks?check_run_id=1218050695

error[E0284]: type annotations needed: cannot satisfy `<<Clock as Clock>::T as Div<_>>::Output == <Clock as Clock>::T`
Error:    --> src/instant.rs:183:66
    |
183 |         if add_ticks <= (<Clock::T as num::Bounded>::max_value() / 2.into()) {
    |                                                                  ^ cannot satisfy `<<Clock as Clock>::T as Div<_>>::Output == <Clock as Clock>::T`

Meta

rustc --version --verbose:

rustc 1.49.0-nightly (98edd1fbf 2020-10-06)
binary: rustc
commit-hash: 98edd1fbf8a68977a2a7c1312eb1ebff80515a92
commit-date: 2020-10-06
host: x86_64-pc-windows-msvc
release: 1.49.0-nightly
LLVM version: 11.0
@mattico
Copy link
Contributor Author

mattico commented Oct 7, 2020

#73905 looks suspicious?

@mattico
Copy link
Contributor Author

mattico commented Oct 7, 2020

Minimal repro: https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=55f8795c91867fec5550107d789a60f8

(at time of writing the playground does not have the affected nightly yet)

@tesuji
Copy link
Contributor

tesuji commented Oct 7, 2020

Yeah, bisecting it's indeed 08e2d46 or #73905 .
However, that PR is a significant improvement that we may don't want to revert.
But I will cc @matthewjasper for their information.

@Stupremee Stupremee added A-inference Area: Type inference regression-from-stable-to-nightly Performance or correctness regression from stable to nightly. labels Oct 7, 2020
@rustbot rustbot added the I-prioritize Issue: Indicates that prioritization has been requested for this issue. label Oct 7, 2020
@mstallmo mstallmo added P-critical Critical priority and removed I-prioritize Issue: Indicates that prioritization has been requested for this issue. labels Oct 7, 2020
@mstallmo
Copy link

mstallmo commented Oct 7, 2020

Assigning P-critical as discussed as part of the Prioritization Working Group procedure and removing I-prioritize

@matthewjasper matthewjasper self-assigned this Oct 7, 2020
@gnunicorn
Copy link

(at time of writing the playground does not have the affected nightly yet)

that playground link now fails with the error described above.

@bkchr
Copy link
Contributor

bkchr commented Oct 7, 2020

Hey, so I tried porting our code to compile with latest nightly and I almost got it working but ultimately it ended with an internal compiler error:

error: internal compiler error: compiler/rustc_trait_selection/src/traits/codegen/mod.rs:121:9: Encountered errors `[FulfillmentError(Obligation(predicate=TraitPredicate(<dyn sc_client_api::PrunableStateChangesTrieStorage<sp_runtime::generic::Block<sp_runtime::generic::Header<u32, sp_runtime::traits::BlakeTwo256>, sp_runtime::OpaqueExtrinsic>> as sp_state_machine::changes_trie::Storage<sp_runtime::traits::BlakeTwo256, u32>>), depth=0),Unimplemented)]` resolving bounds after type-checking

thread 'rustc' panicked at 'Box<Any>', compiler/rustc_errors/src/lib.rs:945:9
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

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

note: we would appreciate a bug report: https://github.com/rust-lang/rust/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.49.0-nightly (98edd1fbf 2020-10-06) running on x86_64-unknown-linux-gnu

note: compiler flags: -C embed-bitcode=no -C debuginfo=2 -C incremental --crate-type lib

note: some of the compiler flags provided by cargo are hidden

error: aborting due to previous error

error: could not compile `node-testing`

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

I created a branch that fails with this or a similar ICE when you run cargo test --all. You can find it here: https://github.com/paritytech/substrate/tree/bkchr-fix-for-latest-nightly

@jyn514 jyn514 added T-compiler Relevant to the compiler team, which will review and decide on the PR/issue. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example A-associated-items Area: Associated items (types, constants & functions) labels Oct 7, 2020
@jyn514 jyn514 changed the title Type inference regression in nightly 2020-10-06 Type inference regression and ICE in nightly 2020-10-06 Oct 7, 2020
@tarcieri
Copy link
Contributor

tarcieri commented Oct 7, 2020

I'm curious if this issue is related to a type inference regression we're experiencing on nightly-2020-10-06 which was NOT present on nightly-2020-10-05:

cometbft/tendermint-rs#584 (comment)

Our particular problem can be reproduced with:

$ git clone https://github.com/RustCrypto/signatures.git
$ cd signatures/ecdsa
$ cargo build --all-features

On nightly-2020-10-05, it compiles. On nightly-2020-10-06, we get:

error[E0277]: cannot multiply `&elliptic_curve::scalar::NonZeroScalar<C>` to `<C as ProjectiveArithmetic>::ProjectivePoint`
  --> ecdsa/src/sign.rs:90:58
   |
90 |             public_key: (C::ProjectivePoint::generator() * &self.secret_scalar).to_affine(),
   |                                                          ^ no implementation for `<C as ProjectiveArithmetic>::ProjectivePoint * &elliptic_curve::scalar::NonZeroScalar<C>`
   |
   = help: the trait `Mul<&elliptic_curve::scalar::NonZeroScalar<C>>` is not implemented for `<C as ProjectiveArithmetic>::ProjectivePoint`

Note that NonZeroScalar<C> has a Deref impl with a target type that has the Mul impl. If we explicitly deref the value with *value, the code compiles.

@lqd
Copy link
Member

lqd commented Oct 7, 2020

This might be related: both embedded-time and some Parity crates did appear in the crater run for that PR in the crates with "Ambiguous projection bounds" section

@mattico
Copy link
Contributor Author

mattico commented Oct 7, 2020

@lqd that crater report is great, thanks.

I figured that the type inference changes might be an intentional bugfix, I'll try to fix embedded-time.

@mattico
Copy link
Contributor Author

mattico commented Oct 7, 2020

Can someone help me understand this? This fixed the problem:

    pub fn checked_add<Dur: Duration>(self, duration: Dur) -> Option<Self>
    where
        Dur: FixedPoint,
-        Clock::T: TryFrom<Dur::T>
+        Clock::T: TryFrom<Dur::T> + ops::Div<Output=Clock::T>

https://play.rust-lang.org/?version=nightly&mode=debug&edition=2018&gist=55f8795c91867fec5550107d789a60f8

But that bound seems redundant because Clock::T: TimeInt and TimeInt: ops::Div<Output = Self>. Is the Self just any TimeInt and not necessarily the specific type?

@lqd
Copy link
Member

lqd commented Oct 7, 2020

Reduced the playground above to remove the dependency on num:

use core::ops::Div;

trait CheckedDiv: Sized + Div<Self, Output = Self> {}

trait Bounded {
    fn max_value() -> Self;
}

struct Fraction;

trait TimeInt: Bounded + CheckedDiv + Div<Fraction, Output = Self> {}
trait Clock {
    type T: TimeInt;
}

trait FixedPoint {
    type T: TimeInt;
}
struct Instant<Clock: crate::Clock> {
    ticks: Clock::T,
}
impl<Clock: crate::Clock> Instant<Clock> {
    pub fn checked_add<Dur: FixedPoint>(self, duration: Dur) {
        let _ = <Clock::T as Bounded>::max_value() / 2.into();
    }
}

playground

(It could probably be reduced further to a MCVE mostly involving the two Div bounds, but at least this is closer to the OP's repro)

@camelid camelid removed the E-needs-mcve Call for participation: This issue has a repro, but needs a Minimal Complete and Verifiable Example label Oct 7, 2020
tarcieri added a commit to RustCrypto/signatures that referenced this issue Oct 8, 2020
The `ecdsa` crate no longer builds on `nightly`, possibly due to:

rust-lang/rust#77638

Unfortunately this means rustdoc builds on https://docs.rs fail!

This commit works around the issue.
tarcieri added a commit to RustCrypto/signatures that referenced this issue Oct 8, 2020
The `ecdsa` crate no longer builds on `nightly`, possibly due to:

rust-lang/rust#77638

Unfortunately this means rustdoc builds on https://docs.rs fail!

This commit works around the issue.
tarcieri added a commit to RustCrypto/signatures that referenced this issue Oct 8, 2020
The `ecdsa` crate no longer builds on `nightly`, possibly due to:

rust-lang/rust#77638

Unfortunately this means rustdoc builds on https://docs.rs fail!

This commit works around the issue.
tarcieri added a commit to RustCrypto/signatures that referenced this issue Oct 8, 2020
The `ecdsa` crate no longer builds on `nightly`, possibly due to:

rust-lang/rust#77638

Unfortunately this means rustdoc builds on https://docs.rs fail!

This commit works around the issue.
tarcieri added a commit to RustCrypto/signatures that referenced this issue Oct 8, 2020
The `ecdsa` crate no longer builds on `nightly`, possibly due to:

rust-lang/rust#77638

Unfortunately this means rustdoc builds on https://docs.rs fail!

This commit works around the issue.
tarcieri added a commit to RustCrypto/traits that referenced this issue Oct 8, 2020
The `elliptic-curve` crate no longer builds on `nightly`.

Possible cause:

rust-lang/rust#77638

Unfortunately this means rustdoc builds on https://docs.rs fail!

This commit works around the issue.
tarcieri added a commit to RustCrypto/traits that referenced this issue Oct 8, 2020
The `elliptic-curve` crate no longer builds on `nightly`.

Possible cause:

rust-lang/rust#77638

Unfortunately this means rustdoc builds on https://docs.rs fail!

This commit works around the issue.
@iliekturtles
Copy link
Contributor

I believe uom ran into a similar issue introduced by the same PR noted here (reported at iliekturtles/uom#210, error first occurs in nightly-2020-10-07, rustc 1.49.0-nightly (98edd1fbf 2020-10-06)). See the following playground. The example works on stable and beta but fails on nightly unless line 14 is uncommented.

@matthewjasper
Copy link
Contributor

The uom issue is fallout from fixing #27675

The really minimal example of this is

use std::ops::Add;

trait A {
    type U: Add<i64> + Add<u64>;
}

fn f<T: A>(t: T::U) {
    let x = t + Default::default();
}

on stable the compiler will arbitrarily decide to use the Add<u64> impl for +. Some of the changes from #73905 meant that there wasn't a good way to keep this behaviour, so this is now reported as ambiguous (which it is).

@tavianator
Copy link
Contributor

@matthewjasper Oh awesome, that means you fixed #72582 (and a few of its duplicates). Should I add a regression test?

@matthewjasper
Copy link
Contributor

There should already be some tests in #73905, if you can find anything not covered by the tests added there then please add some.

@tavianator
Copy link
Contributor

@Aaron1011
Copy link
Member

Can this issue be closed?

@mattico
Copy link
Contributor Author

mattico commented Oct 14, 2020

@Aaron1011 Since it seems that these regressions were already known about when #73905 was merged, I suppose they were intentional. Additionally, @matthewjasper saw this and hasn't said anything to the effect of these being issues that need to be fixed so I'm going to close.

@mattico mattico closed this as completed Oct 14, 2020
iliekturtles added a commit to iliekturtles/uom that referenced this issue Oct 16, 2020
rust-lang/rust#73905 introduced changes to typechecking that made
previously accepted trait bounds where a trait was automatically chosen
now ambiguous. Resolves #210.

See rust-lang/rust#77638 (comment) for
additional discussion.
iliekturtles added a commit to iliekturtles/uom that referenced this issue Oct 18, 2020
rust-lang/rust#73905 introduced changes to typechecking that made
previously accepted trait bounds where a trait was automatically chosen
now ambiguous. Resolves #210.

See rust-lang/rust#77638 (comment) for
additional discussion.
str4d added a commit to zkcrypto/bellman that referenced this issue Oct 30, 2020
Fixes the following error:
  cannot multiply-assign `<E as Engine>::Fr` by `&&<E as Engine>::Fr`

I think this is related to:
  rust-lang/rust#77638
str4d added a commit to zcash/halo2 that referenced this issue Oct 30, 2020
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
A-associated-items Area: Associated items (types, constants & functions) A-inference Area: Type inference C-bug Category: This is a bug. I-ICE Issue: The compiler panicked, giving an Internal Compilation Error (ICE) ❄️ P-critical Critical priority 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

No branches or pull requests