Panic when compiling gluon on 1.46

[nukeop]

I created an empty hello world project with Cargo, added the "gluon" library to it, then tried to compile.
I understand this is something more than just an incompatibility between the nightly compiler and the library.
Does not occur on stable.

Code

There's no code, I added gluon = "0.16.1" to Cargo.toml of an empty project.

Meta

rustc --version --verbose:

rustc 1.47.0-nightly (6c8927b0c 2020-07-26)
binary: rustc
commit-hash: 6c8927b0cf80ceee19386026cf9d7fd4fd9d486f
commit-date: 2020-07-26
host: x86_64-unknown-linux-gnu
release: 1.47.0-nightly
LLVM version: 10.0

Error output

error: internal compiler error: broken MIR in Item(WithOptConstParam { did: DefId(0:814 ~ gluon[bf1c]::std_lib[0]::regex[0]::load[0]), const_param_did: None }) (end of phase Optimized) at bb26[20]:            
encountered `Assign` statement with incompatible types:                                                                                                                                                          
left-hand side has type: fn(&std_lib::regex::Regex, &str) -> std::option::Option<vm::api::Collect<impl std::iter::Iterator>>                                                                                     
right-hand side has type: for<'r, 's> fn(&'r std_lib::regex::Regex, &'s str) -> std::option::Option<vm::api::Collect<impl std::iter::Iterator>>                                                                  
  --> /home/jcd/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/gluon-0.16.1/src/std_lib/regex.rs:90:39                                                                                                          
   |                                                                                                                                                                                                             
90 |             captures => primitive!(2, std::regex::prim::captures),                                                                                                                                          
   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^                                                                                                                                            
   |                                                                                                                                                                                                             
   = note: delayed at src/librustc_mir/transform/validate.rs:140:36                                                                                                                                              
                                                                                                                                                                                                                 
error: internal compiler error: broken MIR in Item(WithOptConstParam { did: DefId(0:2715 ~ gluon[bf1c]::std_lib[0]::regex[0]::load[0]::wrapper[3]), const_param_did: None }) (end of phase Optimized) at bb0[0]: 
encountered `Assign` statement with incompatible types:                                                                                                                                                          
left-hand side has type: fn(&std_lib::regex::Regex, &str) -> std::option::Option<vm::api::Collect<impl std::iter::Iterator>>                                                                                     
right-hand side has type: for<'r, 's> fn(&'r std_lib::regex::Regex, &'s str) -> std::option::Option<vm::api::Collect<impl std::iter::Iterator>>                                                                  
  --> /home/jcd/.cargo/registry/src/git.luolix.top-1ecc6299db9ec823/gluon-0.16.1/src/std_lib/regex.rs:90:39                                                                                                          
   |                                                                                                                                                                                                             
90 |             captures => primitive!(2, std::regex::prim::captures),                                                                                                                                          
   |                                       ^^^^^^^^^^^^^^^^^^^^^^^^^^
   |                                                
   = note: delayed at src/librustc_mir/transform/validate.rs:140:36

thread 'rustc' panicked at 'no errors encountered even though `delay_span_bug` issued', src/librustc_errors/lib.rs:366:17
note: run with `RUST_BACKTRACE=1` environment variable to display 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/issues/new?labels=C-bug%2C+I-ICE%2C+T-compiler&template=ice.md

note: rustc 1.47.0-nightly (6c8927b0c 2020-07-26) running on x86_64-unknown-linux-gnu

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

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

error: could not compile `gluon`.

[Aaron1011]

It looks like the bound regions are preventing us from normalizing an opaque type, leading us to conclude that the LHS and RHS types are not actually equal.

[jonas-schievink]

cc @RalfJung

[RalfJung]

Well I thought our subtyping check would catch such things? Cc @eddyb

It would be great to get a minimal testcase out of this.

[RalfJung]

I thought something like this could be used to reproduce the problem, but so far I could not get it to ICE:

struct Regex;

fn nested(x: (for<'r, 's> fn(&'r Regex, &'s str), String)) -> (fn(&Regex, &str), String) {
    x
}

fn main() {
    nested((|_x, _y| (), String::new()));
}

[eddyb]

It looks like the bound regions are preventing us from normalizing an opaque type, leading us to conclude that the LHS and RHS types are not actually equal.

@RalfJung You're missing the impl Iterator from the original. Here's a short repro:

fn iter_slice<'a, T>(xs: &'a [T]) -> impl Iterator<Item = &'a T> {
    xs.iter()
}

fn main() {
    iter_slice::<()> as fn(_) -> _;
}

---

For the record, the thing of note about primitive!(2, std::regex::prim::captures) is that it casts (twice, even, presumably accounting for the two errors), the function to primitive_cast!(2) (which is just fn(_, _) -> _)

The function is gluon's std::regex::prim::captures, with the signature:

fn captures<'a>(
    re: &Regex,
    text: &'a str,
) -> Option<Collect<impl Iterator<Item = Option<Match<'a>>>>>

[RalfJung]

You're missing the impl Iterator from the original. Here's a short repro:

Nice, a repro! So return-position impl-trait is crucial here. (We don't really need a bisect as we know that this got introduced by checking that MIR assignments are properly typed, so removing that label as well.)

Any proposal for how this could be handled? I don't even understand why it doesn't work, I thought we are deleting all the lifetimes, but clearly not enough of them.^^
Would moving back to a "type fold" (instead of this relator thing we are currently doing) help?

[Dylan-DPC-zz]

Marked this as p-critical as per the wg-prioritization discussion

[RalfJung]

I'm afraid I have no idea how to fix this. I tried using a reveal_all ParamEnv, to no avail. I also tried switching the validation type comparison back to a BottomUpFolder, but that did not help either.

The only thing I can offer is disabling the sanity check for assignments entirely.
I wonder if the check that @bjorn3 has in the cranelift backend can handle this case.

[jonas-schievink]

I thought that code was already getting a reveal_all ParamEnv passed into it

[jonas-schievink]

Is the problem really due to impl Trait alone, or do the late-bound lifetimes cause it? Because the ICE message kind of looks like they are the thing that causes issues:

left-hand side has type: fn(&[()]) -> impl std::iter::Iterator
right-hand side has type: for<'r> fn(&'r [()]) -> impl std::iter::Iterator

[RalfJung]

Late-bound lifetimes work elsewhere though and the impl Trait is needed to reproduce the problem.

[eddyb]

Are you normalizing? The reveal_all aspect is not going to be used unless you actually normalize. You might need to do the hacky excessive erasure of lifetimes before even attempting to normalize as the bound lifetimes may block normalization.

(so the bottom-up folder could erase lifetimes then normalize, at every single step)

[bjorn3]

I wonder if the check that @bjorn3 has in the cranelift backend can handle this case.

It does handle this. I just tried to compile the repro with cg_clif. Compilation fails on the same delay_span_bug! as without cg_clif, but not before cg_clif successfully finished codegen.

[RalfJung]

What was the link to that check again?

[bjorn3]

https://github.com/bjorn3/rustc_codegen_cranelift/blob/8c7c091244f8d820c5de59541c15c817d62e18c6/src/value_and_place.rs#L411-L462

[RalfJung]

@eddyb found a fix, I coded it up: #75419

[eddyb]

@eddyb calling normalize_erasing_regions with a reveal-all param_env doesn't help. :/

@RalfJung for the record, I specifically meant after removing all the bound lifetimes. miri already normalizes, so it would have to be stronger than that.

(EDIT: #75419 appears to do what I was suggesting, although it shouldn't be necessary, see below)

---

However, I believe I may have found the bug, and it's actually in normalization itself:

rust/src/librustc_trait_selection/traits/project.rs

Line 327 in cbe7c5c

ty::Opaque(def_id, substs) if !substs.has_escaping_bound_vars() => {

That if !substs.has_escaping_bound_vars() condition shouldn't be there - IIRC I copied it from the ty::Projection arm.

While for associated type projections it makes sense, due to having to look up the impl (which may have different results depending on how the lifetimes are bound, or be unsound, I guess), it does not for ty::Opaque, which behaves more like a type alias, and which should be completely sound to normalize under a binder.

cc @nikomatsakis

[RalfJung]

@eddyb the old code doesn't normalize, so I don't see how changes to normalization could help. Also I don't know any of that code so it likely makes more sense for someone else to take over then.

[spastorino]

This regressed in #73830 which is a rollup, I guess due to #72796

[RalfJung]

This regressed in #72796. I removed the label because we knew that, but it's not helpful -- we don't want to just revert that PR as that would mean not checking MIR assignments any more.

[spastorino]

@RalfJung right, I was going over the prioritization process and noticed that the PR where this regressed was lacking. I wasn't suggesting more than just that :).

[RalfJung]

Reopening as the problem remains on beta.

@lcnr can you do a backport?

[spastorino]

@RalfJung #75443 is already beta-nominated, still this can be left open until we do the actual backport.

[RalfJung]

Oh I thought the beta PR would get nominated?
Clearly I do not know the process. Feel free to close again if that's the process. :)

[Mark-Simulacrum]

Hm, when you say "beta PR" -- is there a dedicated PR intended for beta backport? Or is #75443 slated for beta backport? Whichever PR's commits should be cherry-picked onto beta should get nominated.

[RalfJung]

Oh gosh now I confused everyone.^^
What I thought would happen is that someone cherry-picks #75443 onto the beta branch, opens that PR, and then marks it as nominated for the team to discuss. But best to ignore me as I clearly have no idea how you are doing backports.^^

[spastorino]

@RalfJung the way it works is ... a PR that was made against master is labelled with beta-nominated requesting a backport for beta or stable-nominated requesting a backport for stable. During the following compiler weekly meeting members vote if the request should be accepted or rejected. If it is accepted a label beta-accepted or stable-accepted is added and at that point @Mark-Simulacrum ports that PR to the corresponding beta/stable branch. If it request is declined the beta-nominated or stable-nominated label is removed leaving the backport without effect.

[teor2345]

We experienced a minor regression with the fix in PR #75443, but there are known workarounds.

The fix caused new "reached the type-length limit" compiler errors in our project. In our particular case, we didn't need the tracing::instrument spans that were creating the large types, so we deleted them (see ZcashFoundation/zebra#923).

This is a known issue in tracing tokio-rs/tracing#616 - they increase the type-length limit as a workaround.

It's also a known issue in rustc (#54540). I added the bisect-rustc regression report to that issue as #54540 (comment).

[pietroalbini]

The fix seems to have been backported to beta. Can we close this?

[RalfJung]

What's the link to the beta PR?

Yes I think it can be closed then.

[pietroalbini]

@RalfJung #75722