Stream: t-compiler/help

Topic: internal compiler error[E0391]; causes delay_span_bug


Joshua Nelson (Jul 11 2020 at 01:18, on Zulip):

In https://github.com/rust-lang/rust/pull/73566, the following code causes an ICE in rustdoc:

enum E {
    V(E),
}
error: internal compiler error[E0391]: cycle detected when computing `Sized` constraints for `E`
 --> src/lib.rs:1:1
  |
1 | enum E {
  | ^^^^^^
  |
  = note: ...which again requires computing `Sized` constraints for `E`, completing the cycle
  = note: cycle used when evaluating trait selection obligation `E: std::convert::From<E>`

error: internal compiler error: TyKind::Error constructed but no error reported

I have two questions.

  1. Why is this an ICE instead of a normal error? When I use cargo check instead of cargo doc this instead gives error[E0072]: recursive type 'E' has infinite size, without the ICE.
  2. Why does this give a secondary delay_span_bug?

I don't see how this is related to my changes :/ The only thing I can think of that's related is I removed a call to tcx.analysis().

eddyb (Jul 11 2020 at 01:37, on Zulip):

@Joshua Nelson did you get query stacks for each of those?

eddyb (Jul 11 2020 at 01:38, on Zulip):

(i.e. -Z treat-err-as-bug=0 and -Z treat-err-as-bug=1, separately. or maybe 1 and 2, I forget how we count that)

eddyb (Jul 11 2020 at 01:38, on Zulip):

I suspect it is because you're not running some parts of type-checking

eddyb (Jul 11 2020 at 01:39, on Zulip):

the cycle error is weird but may be due to one of the cycle-"catching" situations where it's only allowed through requiring that compilation does not succeed

eddyb (Jul 11 2020 at 01:40, on Zulip):

(since successful cycles can have their results depend on which query in the cycle was the first one queried, and is generally unsound, especially wrt incremental or parallel compilation)

eddyb (Jul 11 2020 at 01:41, on Zulip):

@Joshua Nelson what I'm thinking about is that rustc_typeck::check calls a few things, and you want some, or even all, of them other than the "item bodies" one (the one calling typeck_tables_of on everything)

Joshua Nelson (Jul 11 2020 at 01:42, on Zulip):

-Z treat-err-as-bug doesn't work - passing =1 panics on the call to delay_span bug, and passing =2 panics when the delay span bug is reported

Joshua Nelson (Jul 11 2020 at 01:43, on Zulip):
$ rustdoc +stage1 lib.rs -Z treat-err-as-bug
error: internal compiler error: TyKind::Error constructed but no error reported

thread 'rustc' panicked at 'aborting due to `-Z treat-err-as-bug=1`', src/librustc_errors/lib.rs:942:13
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace
Joshua Nelson (Jul 11 2020 at 01:43, on Zulip):
rustdoc +stage1 lib.rs -Z treat-err-as-bug=2
error: internal compiler error[E0391]: cycle detected when computing `Sized` constraints for `E`
 --> lib.rs:1:1
  |
1 | enum E {
  | ^^^^^^
  |
  = note: ...which again requires computing `Sized` constraints for `E`, completing the cycle
  = note: cycle used when evaluating trait selection obligation `E: std::convert::From<E>`

error: internal compiler error: TyKind::Error constructed but no error reported
  |
  = note: delayed at /home/joshua/src/rust/src/librustc_session/session.rs:436:9

thread 'rustc' panicked at 'aborting after 2 errors due to `-Z treat-err-as-bug=2`', src/librustc_errors/lib.rs:942:13
Joshua Nelson (Jul 11 2020 at 01:44, on Zulip):

what I'm thinking about is that rustc_typeck::check calls a few things, and you want some, or even all, of them other than the "item bodies" one (the one calling typeck_tables_of on everything)

hmm, I can see if I can run everything except item-bodies checking

eddyb (Jul 11 2020 at 01:44, on Zulip):

panics on the call to delay_span bug

that's what you want, yes

Joshua Nelson (Jul 11 2020 at 01:44, on Zulip):

ideally of course it wouldn't matter which order queries are called in but I've spent enough time on this I just want it to work lol

Joshua Nelson (Jul 11 2020 at 01:45, on Zulip):

ok, with =1 the query stack is

query stack during panic:
#0 [adt_sized_constraint] computing `Sized` constraints for `E`
#1 [evaluate_obligation] evaluating trait selection obligation `E: std::convert::From<E>`
end of query stack
eddyb (Jul 11 2020 at 01:45, on Zulip):

that checks out

eddyb (Jul 11 2020 at 01:45, on Zulip):

so anything that could possibly end up checking if E: Sized will hit this

eddyb (Jul 11 2020 at 01:46, on Zulip):

Joshua Nelson said:

what I'm thinking about is that rustc_typeck::check calls a few things, and you want some, or even all, of them other than the "item bodies" one (the one calling typeck_tables_of on everything)

hmm, I can see if I can run everything except item-bodies checking

oh wait I already figured out how to do this nonintrusively

eddyb (Jul 11 2020 at 01:46, on Zulip):

you just ended up removing tcx.analysis() instead

eddyb (Jul 11 2020 at 01:47, on Zulip):

so you didn't need the other thing

eddyb (Jul 11 2020 at 01:47, on Zulip):

@Joshua Nelson okay so you can do to typeck_item_bodies the same thing you did to lint_mod

eddyb (Jul 11 2020 at 01:47, on Zulip):

meaning it becomes a noop

Joshua Nelson (Jul 11 2020 at 01:47, on Zulip):

ok, and all other checks will still run

eddyb (Jul 11 2020 at 01:47, on Zulip):

and then you can call https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/lib.rs#L321

Joshua Nelson (Jul 11 2020 at 01:48, on Zulip):

why am I calling check_crate() instead of analysis()?

eddyb (Jul 11 2020 at 01:48, on Zulip):

doesn't analysis include lints?

eddyb (Jul 11 2020 at 01:49, on Zulip):

I mean you could, but there's a risk you might need another full crater run :P

Joshua Nelson (Jul 11 2020 at 01:49, on Zulip):

ugh

Joshua Nelson (Jul 11 2020 at 01:49, on Zulip):

ok let me rephrase - why does check_crate do less work than analysis?

eddyb (Jul 11 2020 at 01:49, on Zulip):

because it's this line https://github.com/rust-lang/rust/blob/master/src/librustc_interface/passes.rs#L857

Joshua Nelson (Jul 11 2020 at 01:50, on Zulip):

got it

eddyb (Jul 11 2020 at 01:50, on Zulip):

of a 123-line function :P

eddyb (Jul 11 2020 at 01:50, on Zulip):

oh it'd also try to do MIR borrowck and other fun stuff, lol

eddyb (Jul 11 2020 at 01:50, on Zulip):

even generating MIR at all would result in type-checking the body

Joshua Nelson (Jul 11 2020 at 02:02, on Zulip):

yay that worked :tada:

eddyb (Jul 11 2020 at 02:03, on Zulip):

sweet!

Joshua Nelson (Jul 11 2020 at 02:03, on Zulip):

let me make sure it still doesn't look at function bodies

Joshua Nelson (Jul 11 2020 at 02:05, on Zulip):

ugh

error[E0433]: failed to resolve: could not resolve path `content::should::be::irrelevant`
  --> /home/joshua/src/rust/src/test/rustdoc/doc-cfg.rs:23:9
   |
23 |         content::should::be::irrelevant();
   |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ could not resolve path `content::should::be::irrelevant`
   |
   = note: this error was originally ignored because you are running `rustdoc`
   = note: try running again with `rustc` or `cargo check` and you may get a more detailed error
Joshua Nelson (Jul 11 2020 at 02:06, on Zulip):

oh wait I forgot to override typeck_item_bodies lol

Joshua Nelson (Jul 11 2020 at 02:08, on Zulip):

nope, still gives errors

Joshua Nelson (Jul 11 2020 at 02:13, on Zulip):

I guess I can do this the hard way and make the evaluate_obligation query call typeck first?

Joshua Nelson (Jul 11 2020 at 02:13, on Zulip):

not even sure if that would do what I want

eddyb (Jul 11 2020 at 04:06, on Zulip):

uhhhh

eddyb (Jul 11 2020 at 04:06, on Zulip):

@Joshua Nelson what's happening?

Joshua Nelson (Jul 11 2020 at 04:06, on Zulip):

when I ran typeck again it looked at function bodies :(

eddyb (Jul 11 2020 at 04:06, on Zulip):

if you overrode typeck_item_bodies you shouldn't have problems I don't think. but you can always use -Z treat-err-as-bug to see what queried typeck

eddyb (Jul 11 2020 at 04:07, on Zulip):

I don't think you can assume anything before checking that, who knows what it might be

Joshua Nelson (Jul 11 2020 at 04:07, on Zulip):

ok, let me check

Joshua Nelson (Jul 11 2020 at 04:09, on Zulip):
query stack during panic:
#0 [typeck_tables_of] type-checking `unix_only::unix_only_function`
#1 [used_trait_imports] used_trait_imports `unix_only::unix_only_function`
end of query stack
eddyb (Jul 11 2020 at 04:09, on Zulip):

hehehe, nice

Joshua Nelson (Jul 11 2020 at 04:09, on Zulip):

so it looks like it's running lints even though I told it not to?

eddyb (Jul 11 2020 at 04:09, on Zulip):

doubtful

eddyb (Jul 11 2020 at 04:10, on Zulip):

it's https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/check_unused.rs

eddyb (Jul 11 2020 at 04:11, on Zulip):

you have two options

eddyb (Jul 11 2020 at 04:11, on Zulip):

you either override used_trait_imports to return an empty set and hope the lint does nothing

eddyb (Jul 11 2020 at 04:11, on Zulip):

or you querify check_unused

Joshua Nelson (Jul 11 2020 at 04:12, on Zulip):

wait, it's not currently a query?

eddyb (Jul 11 2020 at 04:12, on Zulip):

I mean this https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/lib.rs#L367

eddyb (Jul 11 2020 at 04:12, on Zulip):

I'd call it check_unused_post_typeck

eddyb (Jul 11 2020 at 04:13, on Zulip):

or check_unused_traits_and_crates

eddyb (Jul 11 2020 at 04:13, on Zulip):

(for the lint name)

eddyb (Jul 11 2020 at 04:13, on Zulip):

not actually sure it's worth it tbh

eddyb (Jul 11 2020 at 04:13, on Zulip):

it might be easier to just try overriding used_trait_imports to return Default::default() or w/e

Joshua Nelson (Jul 11 2020 at 04:13, on Zulip):

let me try the empty set thing first, yeah

Joshua Nelson (Jul 11 2020 at 04:14, on Zulip):

I feel like I keep shaving more yaks :/

eddyb (Jul 11 2020 at 04:14, on Zulip):

lol https://github.com/rust-lang/rust/blob/master/src/librustc_typeck/check/mod.rs#L843-L845

eddyb (Jul 11 2020 at 04:16, on Zulip):

uhhhhhh

eddyb (Jul 11 2020 at 04:16, on Zulip):

someone did a clever thing that actually makes this worse

eddyb (Jul 11 2020 at 04:17, on Zulip):

oh noes it's not actually possible to allocate it globally

eddyb (Jul 11 2020 at 04:18, on Zulip):

@Joshua Nelson so you'll uhh need a lazy_static! that uses Box::leak to create a &'static FxHashSet<LocalDefId>

eddyb (Jul 11 2020 at 04:18, on Zulip):

it's not that bad just weird

Joshua Nelson (Jul 11 2020 at 04:19, on Zulip):

why can't I use static map: HashMap?

Joshua Nelson (Jul 11 2020 at 04:19, on Zulip):

oh ugh default() isn't a const fn

Joshua Nelson (Jul 11 2020 at 04:21, on Zulip):

do you know why new() isn't being found?

error[E0599]: no function or associated item named `new` found for struct `std::collections::HashSet<_, std::hash::BuildHasherDefault<rustc_data_structures::fx::FxHasher>>` in the current scope
   --> src/librustdoc/core.rs:367:52
    |
367 |     static map: FxHashSet<LocalDefId> = FxHashSet::new();
    |                                                    ^^^ function or associated item not found in `std::collections::HashSet<_, std::hash::BuildHasherDefault<rustc_data_structures::fx::FxHasher>>`
Joshua Nelson (Jul 11 2020 at 04:22, on Zulip):

oh ugh _every_ method in std is only implemented for RandomState hashers

Joshua Nelson (Jul 11 2020 at 04:22, on Zulip):

whyy

Joshua Nelson (Jul 11 2020 at 04:22, on Zulip):

rustdoc doesn't even have a lazy static dependency yet

Joshua Nelson (Jul 11 2020 at 04:24, on Zulip):

I didn't need the Box::leak though

Joshua Nelson (Jul 11 2020 at 04:26, on Zulip):

ok, that finally did it

eddyb (Jul 11 2020 at 04:26, on Zulip):

it's not new, it's default

Joshua Nelson (Jul 11 2020 at 04:26, on Zulip):

default goes through the trait

eddyb (Jul 11 2020 at 04:26, on Zulip):

what did you write?

Joshua Nelson (Jul 11 2020 at 04:26, on Zulip):

so you can't use it in static

eddyb (Jul 11 2020 at 04:27, on Zulip):

surely you can't allocate a hash table in a static??

Joshua Nelson (Jul 11 2020 at 04:27, on Zulip):

I don't see why you can't allocate an empty one

Joshua Nelson (Jul 11 2020 at 04:27, on Zulip):

but in any case I ended up with

    lazy_static! {
        static ref EMPTY_MAP: FxHashSet<LocalDefId> = FxHashSet::default();
    }
eddyb (Jul 11 2020 at 04:27, on Zulip):

oh did we make them free to do so? but then....

eddyb (Jul 11 2020 at 04:27, on Zulip):

yeah that's what I'd expect

eddyb (Jul 11 2020 at 04:27, on Zulip):

but that's not &'static

Joshua Nelson (Jul 11 2020 at 04:27, on Zulip):

... isn't it?

Joshua Nelson (Jul 11 2020 at 04:28, on Zulip):

this compiled at least

            local_providers.used_trait_imports = |_, _| &EMPTY_MAP;
eddyb (Jul 11 2020 at 04:28, on Zulip):

I mean... hmm

eddyb (Jul 11 2020 at 04:28, on Zulip):

that's kind of weird

eddyb (Jul 11 2020 at 04:28, on Zulip):

(based on how lazy-static works, I mean)

eddyb (Jul 11 2020 at 04:28, on Zulip):

I guess &EMPTY_MAP is &'static TheLazyStaticWrapper before getting deref-coerced huh

eddyb (Jul 11 2020 at 04:29, on Zulip):

that's a new trick for me I guess

Joshua Nelson (Jul 11 2020 at 04:32, on Zulip):

ok I think this should have fixed these issues https://github.com/rust-lang/rust/pull/73566/commits/a0086bee32de2b5c56a4e335a6e77323779fc83b

Joshua Nelson (Jul 11 2020 at 04:33, on Zulip):

oh oops I forgot to commit comments

Joshua Nelson (Jul 11 2020 at 04:33, on Zulip):

https://github.com/rust-lang/rust/pull/73566/commits/8008607fdce30d4cdd96e7871b65cb1c432a5fcb

eddyb (Jul 11 2020 at 04:34, on Zulip):

Some queries are buggy and require that they only run on valid types:

this doesn't sound right?

Joshua Nelson (Jul 11 2020 at 04:35, on Zulip):

well, maybe buggy is the wrong word

eddyb (Jul 11 2020 at 04:35, on Zulip):

there's nothing buggy about the query, you're just not emitting errors

Joshua Nelson (Jul 11 2020 at 04:35, on Zulip):

you're just not emitting errors

I don't think that's related? This happened in code without resolution errors

eddyb (Jul 11 2020 at 04:35, on Zulip):

there's a lot of code in rustc that uses delay_span_bug and expecting something else produced an error

eddyb (Jul 11 2020 at 04:36, on Zulip):

resolution errors are just the case that you're injecting directly by ignoring them in rustc_resolve

eddyb (Jul 11 2020 at 04:36, on Zulip):

there's a lot more stuff using delay_span_bug

Joshua Nelson (Jul 11 2020 at 04:36, on Zulip):

I don't think I touched the other stuff though ...

eddyb (Jul 11 2020 at 04:36, on Zulip):

you did by not running any typeck

eddyb (Jul 11 2020 at 04:37, on Zulip):

like, when you removed tcx.analysis(), that prevented critical checks from running

eddyb (Jul 11 2020 at 04:37, on Zulip):

which other parts of the compiler safeguard against missing errors with delay_span_bug

eddyb (Jul 11 2020 at 04:37, on Zulip):

maybe I should explain this less abstractly

Joshua Nelson (Jul 11 2020 at 04:38, on Zulip):

the reason I'm calling this buggy is because the other query should have called typeck::check_crate if it depends on that being run

eddyb (Jul 11 2020 at 04:38, on Zulip):

it doesn't depend on it in a stateful sense

eddyb (Jul 11 2020 at 04:38, on Zulip):

or in a dataflow sense

Joshua Nelson (Jul 11 2020 at 04:38, on Zulip):

it depends on it in a contracts sense

Joshua Nelson (Jul 11 2020 at 04:38, on Zulip):

"this query can only be called on types that are valid"

eddyb (Jul 11 2020 at 04:38, on Zulip):

there's a lot of stuff like this:

if impossible_state() {
    delay_span_bug("uh oh we didn't catch some error somewhere else")
}
Joshua Nelson (Jul 11 2020 at 04:39, on Zulip):

hmm

eddyb (Jul 11 2020 at 04:39, on Zulip):

it may not even know what would be supposed to emit the error or what error

eddyb (Jul 11 2020 at 04:39, on Zulip):

just that it saw something that should never happen unless there is an error somewhere

eddyb (Jul 11 2020 at 04:39, on Zulip):

and it's not like depending on tcx.analysis() or a subset of it would even make sense: it would trivially lead to cycle errors

eddyb (Jul 11 2020 at 04:40, on Zulip):

and analysis always runs so it doesn't even make sense

Joshua Nelson (Jul 11 2020 at 04:40, on Zulip):

analysis always runs

ah this is the bit I was missing

eddyb (Jul 11 2020 at 04:40, on Zulip):

I mean, you literally had to take it out of rustdoc?

eddyb (Jul 11 2020 at 04:40, on Zulip):

normally it's always there

Joshua Nelson (Jul 11 2020 at 04:40, on Zulip):

right, I didn't realize it was an invariant that it runs

eddyb (Jul 11 2020 at 04:40, on Zulip):

anyway, the "delay" in delay_span_bug refers to the fact that it handles the error being emitted in the future of the detection

Joshua Nelson (Jul 11 2020 at 04:41, on Zulip):

I don't follow, sorry

eddyb (Jul 11 2020 at 04:41, on Zulip):

it "poisons" the current compilation session in a sense

Joshua Nelson (Jul 11 2020 at 04:42, on Zulip):

my understanding was that it's like a drop bomb, if you continue without saying "no I checked this is valid" then it panics

eddyb (Jul 11 2020 at 04:42, on Zulip):

ah no it's not in the same sense

eddyb (Jul 11 2020 at 04:42, on Zulip):

the only requirement for delay_span_bug to not cause an ICE is any error to be emitted by anything

eddyb (Jul 11 2020 at 04:42, on Zulip):

at any point in time

eddyb (Jul 11 2020 at 04:42, on Zulip):

during the same compilation session

Joshua Nelson (Jul 11 2020 at 04:42, on Zulip):

ahhh

Joshua Nelson (Jul 11 2020 at 04:43, on Zulip):

this is why it was so important that my error test cases be in separate files

eddyb (Jul 11 2020 at 04:43, on Zulip):

okay yeah sorry for not explaining this properly before

eddyb (Jul 11 2020 at 04:43, on Zulip):

the name could maybe use some workshopping lol

Joshua Nelson (Jul 11 2020 at 04:43, on Zulip):

I thought it was per-error, not per-session

eddyb (Jul 11 2020 at 04:43, on Zulip):

since I doubt it has changed since it was added and it was probably added as a hack

eddyb (Jul 11 2020 at 04:43, on Zulip):

ah no, there doesn't have to be any correlation for it to be silenced, just any error at all

eddyb (Jul 11 2020 at 04:44, on Zulip):

this allows us to use it aggressively as a safeguard against broken invariants

eddyb (Jul 11 2020 at 04:44, on Zulip):

and it's free if any error has already been emitted, since it's eagerly ignored, and not buffered at all (which is arguably not useful to optimize for)

Joshua Nelson (Jul 11 2020 at 04:45, on Zulip):

I see, so any time you call delay_span_bug() you're saying "there's a broken invariant here" and compilation can't continue much further

eddyb (Jul 11 2020 at 04:45, on Zulip):

so for example @mark-i-m put a lot of effort into switching our Ty::Error case to trigger a delay_span_bug, making it a strong indicator of "this compilation will fail"

Joshua Nelson (Jul 11 2020 at 04:45, on Zulip):

ok this makes sense why the query expected the type to be valid

eddyb (Jul 11 2020 at 04:45, on Zulip):

@Joshua Nelson it can continue pretty far, just not exit successfully :P

eddyb (Jul 11 2020 at 04:46, on Zulip):

and presumably not emit link artifacts

eddyb (Jul 11 2020 at 04:46, on Zulip):

(since they might contain UB etc.)

Joshua Nelson (Jul 11 2020 at 04:49, on Zulip):

I'm a little concerned by how fragile this makes rustdoc

Joshua Nelson (Jul 11 2020 at 04:49, on Zulip):

unrelated changes in other parts of the compiler could now cause tests to fail

Joshua Nelson (Jul 11 2020 at 04:49, on Zulip):

e.g. if a new lint adds typechecking

eddyb (Jul 11 2020 at 05:29, on Zulip):

eeeeeh

eddyb (Jul 12 2020 at 02:03, on Zulip):

@Joshua Nelson btw I was able to get the non-cached version to optimize :P

ret %"rustc_middle::ty::TyS"* (i64*, i32, i32)* @_ZN12rustc_typeck7collect7type_of7type_of17h576de94756ddadc8E
eddyb (Jul 12 2020 at 02:03, on Zulip):

it's still calling Providers::default() because I forgot to make that #[inline(always)] as well =))

eddyb (Jul 12 2020 at 02:14, on Zulip):

oh oops I made a topic for that

Last update: Sep 27 2020 at 14:30UTC