ICE: Adding `-C save-temps` to incremental compile causes rustc_codegen_ssa::back::write panic

[pnkfelix]

Steps to reproduce:

% mkdir temp
% cd temp
% touch lib.rs
% rustc +nightly lib.rs --crate-type=lib -C incremental=incr
% rustc +nightly lib.rs --crate-type=lib -C incremental=incr -C save-temps
thread '<unnamed>' panicked at 'assertion failed: `(left == right)`
  left: `false`,
 right: `true`', src/librustc_codegen_ssa/back/write.rs:872:5
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/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.40.0-nightly (50f8aadd7 2019-11-07) running on x86_64-unknown-linux-gnu

note: compiler flags: -C incremental -C save-temps --crate-type lib

thread '<unnamed>' panicked at 'src/librustc_codegen_ssa/back/write.rs:1488: worker thread panicked', src/librustc/util/bug.rs:37:26

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/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.40.0-nightly (50f8aadd7 2019-11-07) running on x86_64-unknown-linux-gnu

note: compiler flags: -C incremental -C save-temps --crate-type lib

thread 'rustc' panicked at 'src/librustc_codegen_ssa/back/write.rs:1758: panic during codegen/LLVM phase', src/librustc/util/bug.rs:\
37:26

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/blob/master/CONTRIBUTING.md#bug-reports

note: rustc 1.40.0-nightly (50f8aadd7 2019-11-07) running on x86_64-unknown-linux-gnu

note: compiler flags: -C incremental -C save-temps --crate-type lib

[pnkfelix]

(the workaround is to make sure you start with -C save-temps in your initial command line. I assume you need to subsequently keep it around to avoid another ICE, but I have not verified that.)

[pnkfelix]

(also, the reported "compiler flags" in the diagnostic says -C incremental when the actual option was -C incremental=incr. A subtle difference but I bet it would mean the world to some people trying to debug something...)

[pnkfelix]

The panic is due to this assert failure in execute_copy_from_cache_work_item:

rust/src/librustc_codegen_ssa/back/write.rs

Line 868 in a916ac2

assert_eq!(bytecode.is_some(), module_config.emit_bc);

My first instinct was to try to revise the logic so that we would force the code generation of the bytecode when -C save-temps is added to the command line.

But after thinking about it more, I do not think that is the appropriate fix. Someone adding -C save-temps probably is trying to understand the current compiler invocation. Having -C save-temps cause it to go through significantly new code paths (i.e. generating new code when previously it would have just reused existing object files) does not sound right to me.

So the question is, what is the correct fix here?

I can imagine:

1. turn the assert into a normal error, with a diagnostic like: "emit LLVM bytecode requested, but no bytecode file found in incremental cache",
2. or get rid of the assert entirely; maybe nothing further down in the control flow depends on the property being asserted there? Wishful thinking, I know.

@michaelwoerister , any thoughts here? And also cc @alexcrichton, whom I believe is the source for these assertions way back in 8197a0b

[alexcrichton]

I would agree that the intention of save-temps is "just don't delete temporary files you would otherwise delete" and is really only used for debugging. In that sense I think it's fine to basically get rid of this assert personally.

[michaelwoerister]

Getting rid of the assertion seems fine.

[JohnTitor]

The assertion was removed by #71754.

[Enselic]

Triage: Seems quite easy to add a regression test for this ICE so it can be closed. See ./tests/incremental for examples of incremental tests.

@rustbot label E-needs-test