ICE index OOB in 1.59.0-beta.8 🚨

[carols10cents]

I know it's late in the release cycle and I know there were bugs similar to this in previous 1.59.0 betas, but I don't see any issues for this bug in beta 8, so this seems pretty high priority to me @rust-lang/compiler

Code

I'm working on some changes to https://github.com/influxdata/influxdb_iox. I ran a workspace level cargo check that succeeded, I made a few small changes, then I ran cargo test --test end_to_end and go tthis error.

Meta

rustc --version --verbose
rustc 1.59.0-beta.8 (1945ce657 2022-02-12)
binary: rustc
commit-hash: 1945ce6579506787e0b18f0a2ea03fdb4dfc81c7
commit-date: 2022-02-12
host: aarch64-apple-darwin
release: 1.59.0-beta.8
LLVM version: 13.0.0

Error output

thread 'rustc' panicked at 'index out of bounds: the len is 741 but the index is 767', compiler/rustc_query_impl/src/on_disk_cache.rs:726:18
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.59.0-beta.8 (1945ce657 2022-02-12) running on aarch64-apple-darwin

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

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

query stack during panic:
#0 [optimized_mir] optimizing MIR for `ingester::from_window_frame`
#1 [collect_and_partition_mono_items] collect_and_partition_mono_items
end of query stack
error: could not compile `predicate`
warning: build failed, waiting for other jobs to finish...
error: build failed

Backtrace

 RUST_BACKTRACE=1 cargo test --test end_to_end
   Compiling predicate v0.1.0 (/Users/carolnichols/rust/influxdb_iox/predicate)
   Compiling router v0.1.0 (/Users/carolnichols/rust/influxdb_iox/router)
thread 'rustc' panicked at 'index out of bounds: the len is 741 but the index is 767', compiler/rustc_query_impl/src/on_disk_cache.rs:726:18
stack backtrace:
   0: _rust_begin_unwind
   1: core::panicking::panic_fmt
   2: core::panicking::panic_bounds_check
   3: <rustc_span::span_encoding::Span as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
   4: <rustc_middle::ty::VariantDef as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
   5: <rustc_query_impl::on_disk_cache::CacheDecoder as rustc_serialize::serialize::Decoder>::read_seq::<alloc::vec::Vec<rustc_middle::ty::VariantDef>, <alloc::vec::Vec<rustc_middle::ty::VariantDef> as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode::{closure#0}>
   6: <rustc_middle::ty::adt::AdtDef as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
   7: <rustc_middle::ty::sty::TyKind as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
   8: <&rustc_middle::ty::TyS as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
   9: <rustc_query_impl::on_disk_cache::CacheDecoder as rustc_middle::ty::codec::TyDecoder>::cached_ty_for_shorthand::<<&rustc_middle::ty::TyS as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode::{closure#0}>
  10: <&rustc_middle::ty::TyS as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  11: <rustc_middle::ty::subst::GenericArg as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  12: <core::result::Result<rustc_middle::ty::subst::GenericArg, alloc::string::String> as rustc_middle::ty::context::InternIteratorElement<rustc_middle::ty::subst::GenericArg, &rustc_middle::ty::list::List<rustc_middle::ty::subst::GenericArg>>>::intern_with::<core::iter::adapters::map::Map<core::ops::range::Range<usize>, <&rustc_middle::ty::list::List<rustc_middle::ty::subst::GenericArg> as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode::{closure#0}>, <rustc_middle::ty::context::TyCtxt>::mk_substs<core::iter::adapters::map::Map<core::ops::range::Range<usize>, <&rustc_middle::ty::list::List<rustc_middle::ty::subst::GenericArg> as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode::{closure#0}>>::{closure#0}>
  13: <&rustc_middle::ty::list::List<rustc_middle::ty::subst::GenericArg> as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  14: <rustc_middle::ty::sty::TyKind as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  15: <&rustc_middle::ty::TyS as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  16: <rustc_middle::mir::Constant as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  17: <rustc_middle::mir::Operand as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  18: <rustc_middle::mir::terminator::Terminator as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  19: <rustc_query_impl::on_disk_cache::CacheDecoder as rustc_serialize::serialize::Decoder>::read_option::<core::option::Option<rustc_middle::mir::terminator::Terminator>, <core::option::Option<rustc_middle::mir::terminator::Terminator> as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode::{closure#0}>
  20: <rustc_middle::mir::BasicBlockData as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode
  21: <rustc_query_impl::on_disk_cache::CacheDecoder as rustc_serialize::serialize::Decoder>::read_seq::<alloc::vec::Vec<rustc_middle::mir::BasicBlockData>, <alloc::vec::Vec<rustc_middle::mir::BasicBlockData> as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode::{closure#0}>
  22: <rustc_query_impl::on_disk_cache::CacheDecoder as rustc_serialize::serialize::Decoder>::read_struct::<rustc_middle::mir::Body, <rustc_middle::mir::Body as rustc_serialize::serialize::Decodable<rustc_query_impl::on_disk_cache::CacheDecoder>>::decode::{closure#0}>
  23: <rustc_query_impl::on_disk_cache::OnDiskCache>::try_load_query_result::<&rustc_middle::mir::Body>
  24: rustc_query_system::query::plumbing::try_load_from_disk_and_cache_in_memory::<rustc_query_impl::plumbing::QueryCtxt, rustc_span::def_id::DefId, &rustc_middle::mir::Body>
  25: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::plumbing::QueryCtxt, rustc_query_system::query::caches::DefaultCache<rustc_span::def_id::DefId, &rustc_middle::mir::Body>>
  26: rustc_query_system::query::plumbing::get_query::<rustc_query_impl::queries::optimized_mir, rustc_query_impl::plumbing::QueryCtxt>
  27: <rustc_middle::ty::context::TyCtxt>::instance_mir
  28: rustc_monomorphize::collector::collect_neighbours
  29: rustc_monomorphize::collector::collect_items_rec
  30: rustc_monomorphize::collector::collect_items_rec
  31: rustc_monomorphize::collector::collect_items_rec
  32: rustc_monomorphize::collector::collect_items_rec
  33: rustc_monomorphize::collector::collect_items_rec
  34: rustc_monomorphize::collector::collect_items_rec
  35: rustc_monomorphize::collector::collect_items_rec
  36: rustc_monomorphize::collector::collect_items_rec
  37: rustc_monomorphize::collector::collect_items_rec
  38: rustc_monomorphize::collector::collect_items_rec
  39: rustc_monomorphize::collector::collect_items_rec
  40: rustc_monomorphize::collector::collect_items_rec
  41: rustc_monomorphize::collector::collect_items_rec
  42: rustc_monomorphize::collector::collect_items_rec
  43: rustc_monomorphize::collector::collect_items_rec
  44: rustc_monomorphize::collector::collect_items_rec
  45: rustc_monomorphize::collector::collect_items_rec
  46: rustc_monomorphize::collector::collect_items_rec
  47: rustc_monomorphize::collector::collect_items_rec
  48: rustc_monomorphize::collector::collect_items_rec
  49: <rustc_session::session::Session>::time::<(), rustc_monomorphize::collector::collect_crate_mono_items::{closure#1}>
  50: rustc_monomorphize::collector::collect_crate_mono_items
  51: rustc_monomorphize::partitioning::collect_and_partition_mono_items
  52: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::with_task::<rustc_middle::ty::context::TyCtxt, (), (&std::collections::hash::set::HashSet<rustc_span::def_id::DefId, core::hash::BuildHasherDefault<rustc_hash::FxHasher>>, &[rustc_middle::mir::mono::CodegenUnit])>
  53: rustc_data_structures::stack::ensure_sufficient_stack::<((&std::collections::hash::set::HashSet<rustc_span::def_id::DefId, core::hash::BuildHasherDefault<rustc_hash::FxHasher>>, &[rustc_middle::mir::mono::CodegenUnit]), rustc_query_system::dep_graph::graph::DepNodeIndex), rustc_query_system::query::plumbing::execute_job<rustc_query_impl::plumbing::QueryCtxt, (), (&std::collections::hash::set::HashSet<rustc_span::def_id::DefId, core::hash::BuildHasherDefault<rustc_hash::FxHasher>>, &[rustc_middle::mir::mono::CodegenUnit])>::{closure#3}>
  54: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::plumbing::QueryCtxt, rustc_query_system::query::caches::DefaultCache<(), (&std::collections::hash::set::HashSet<rustc_span::def_id::DefId, core::hash::BuildHasherDefault<rustc_hash::FxHasher>>, &[rustc_middle::mir::mono::CodegenUnit])>>
  55: rustc_query_system::query::plumbing::force_query::<rustc_query_impl::queries::collect_and_partition_mono_items, rustc_query_impl::plumbing::QueryCtxt>
  56: rustc_query_impl::query_callbacks::collect_and_partition_mono_items::force_from_dep_node
  57: <rustc_middle::ty::context::TyCtxt as rustc_query_system::dep_graph::DepContext>::try_force_from_dep_node
  58: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::try_mark_previous_green::<rustc_query_impl::plumbing::QueryCtxt>
  59: <rustc_query_system::dep_graph::graph::DepGraph<rustc_middle::dep_graph::dep_node::DepKind>>::try_mark_green::<rustc_query_impl::plumbing::QueryCtxt>
  60: rustc_query_system::query::plumbing::try_load_from_disk_and_cache_in_memory::<rustc_query_impl::plumbing::QueryCtxt, rustc_span::def_id::CrateNum, &[(rustc_middle::middle::exported_symbols::ExportedSymbol, rustc_middle::middle::exported_symbols::SymbolExportLevel)]>
  61: rustc_query_system::query::plumbing::try_execute_query::<rustc_query_impl::plumbing::QueryCtxt, rustc_query_system::query::caches::DefaultCache<rustc_span::def_id::CrateNum, &[(rustc_middle::middle::exported_symbols::ExportedSymbol, rustc_middle::middle::exported_symbols::SymbolExportLevel)]>>
  62: rustc_query_system::query::plumbing::get_query::<rustc_query_impl::queries::exported_symbols, rustc_query_impl::plumbing::QueryCtxt>
  63: <rustc_metadata::rmeta::encoder::EncodeContext>::encode_crate_root
  64: rustc_metadata::rmeta::encoder::encode_metadata_impl
  65: rustc_data_structures::sync::join::<rustc_metadata::rmeta::encoder::encode_metadata::{closure#0}, rustc_metadata::rmeta::encoder::encode_metadata::{closure#1}, rustc_metadata::rmeta::encoder::EncodedMetadata, ()>
  66: rustc_metadata::rmeta::encoder::encode_metadata
  67: <rustc_interface::queries::Queries>::ongoing_codegen
  68: <rustc_interface::interface::Compiler>::enter::<rustc_driver::run_compiler::{closure#1}::{closure#2}, core::result::Result<core::option::Option<rustc_interface::queries::Linker>, rustc_errors::ErrorReported>>
  69: rustc_span::with_source_map::<core::result::Result<(), rustc_errors::ErrorReported>, rustc_interface::interface::create_compiler_and_run<core::result::Result<(), rustc_errors::ErrorReported>, rustc_driver::run_compiler::{closure#1}>::{closure#1}>
  70: rustc_interface::interface::create_compiler_and_run::<core::result::Result<(), rustc_errors::ErrorReported>, rustc_driver::run_compiler::{closure#1}>
  71: <scoped_tls::ScopedKey<rustc_span::SessionGlobals>>::set::<rustc_interface::util::setup_callbacks_and_run_in_thread_pool_with_globals<rustc_interface::interface::run_compiler<core::result::Result<(), rustc_errors::ErrorReported>, rustc_driver::run_compiler::{closure#1}>::{closure#0}, core::result::Result<(), rustc_errors::ErrorReported>>::{closure#0}::{closure#0}, core::result::Result<(), rustc_errors::ErrorReported>>
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose 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.59.0-beta.8 (1945ce657 2022-02-12) running on aarch64-apple-darwin

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

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

query stack during panic:
#0 [optimized_mir] optimizing MIR for `ingester::from_window_frame`
#1 [collect_and_partition_mono_items] collect_and_partition_mono_items
#2 [exported_symbols] exported_symbols
end of query stack
error: could not compile `predicate`
warning: build failed, waiting for other jobs to finish...
error: build failed

[Mark-Simulacrum]

I believe this is #93029

(also #93823, #93371, #93131, #93336)

Which is fixed in 1.60 by #92533 which didn't get backported, seems like intentionally.

[Aaron1011]

That's correct. This ICE was exposed in the process of fixing a different incremental compilation ICE. Unfortunately, it seems that this new ICE occurs much more frequently than the old one.

Given the complexity of the incr comp system, I'd like to avoid backporting the fix unless absolutely necessary.

[carols10cents]

How would you define "absolutely necessary" in this case? If I hit this ICE on beta, people are going to hit it on stable, no?

[estebank]

Should we have a beta revert of this change while we work on master to fix it?

[Aaron1011]

How would you define "absolutely necessary" in this case? If I hit this ICE on beta, people are going to hit it on stable, no?

@carols10cents: I'd want the ICE to be very frequent and disruptive if we're going to do a backport. Since the fingerprint check was enabled by default, there's been a large number of incremental bugs uncovered. I believe that the best way to deal with them is to let the fixes ride the release train, so that we have time to discover any new issues that might be revealed or caused by the fix. I don't think trying to rush fixes to the incremental compilation system is a good idea.

Should we have a beta revert of this change while we work on master to fix it?

@estebank This is already fixed in master by #92533. Even if it wasn't already fixed, this ICE was a pre-existing issue that was revealed by another fix to the incremental system. I don't think we should revert a (correct) just to temporarily hide a different ICE.

[carols10cents]

@Aaron1011 The last time we let incremental compilation bugs ride the train to stable, the team discovered that hardly anyone uses beta to develop with, so beta isn't actually representative of frequency on stable. Has anything changed since then?

Also, last time, incremental compilation had to be disabled by default for a number of stable releases. Is that an acceptable possible outcome again?

[Aaron1011]

The last time we let incremental compilation bugs ride the train to stable,

If you're referring to the time when we had to disable incremental compilation, then that was a special case - enabling the fingerprint checks caused a large number of distinct incr comp bugs to start producing ICEs. Since then, we've let fixes for individual incremental comp bugs ride the train to stable. I don't think that the frequency of this ICE is especially high compared to any of those other issues (e.g. #85868)

In general, I think that beta backports for incremental compilation bugs should be a last resort, not something that we do automatically. @carols10cents Have you hit this bug multiple times on beta?

[carols10cents]

In general, I think that beta backports for incremental compilation bugs should be a last resort, not something that we do automatically. @carols10cents Have you hit this bug multiple times on beta?

Not yet. It seems strange to me to actually get early warning of a problem on beta and then deliberately choose to ignore it. Isn't catching problems early the reason beta exists?

I actually switched to 1.59 beta over 1.58 stable because I was hitting incremental compilation bugs a disruptive number of times there. I've only been on beta.8 for a few days. Certainly seems like given so few people use beta, that if anyone experiences and reports an issue with beta, that it could potentially represent a large number of stable users.

[Aaron1011]

Not yet. It seems strange to me to actually get early warning of a problem on beta and then deliberately choose to ignore it. Isn't catching problems early the reason beta exists?

Yes, but the PR we would be backporting is really a distinct, unrelated change. There are really two PRs that we would need to backport to fully fix the ICE that you're seeing: #93095 and #93095. Backporting those PRs would fix a known ICE on beta, but it would mean that we would lose out on some of the normal testing that those two PRs would get via riding the release train.

We've been making steady progress at fixing incremental compilation bugs. If we discover that a particular bugfix PR introduces new issues, then I think it's reasonable to consider backporting (or reverting on beta). However, if the PR exposes an existing bug, then I don't think we should rush through an unrelated change that could potentially introduce new bugs.

[Urgau]

Not sure if that helps but during the last 2/3 days I encountered multiple times (at least 10 times, sometimes in a row) this bug when I was hacking on rustc. The only solution was to do a ./x.py clean and a full rebuild.

[oli-obk]

We could consider making a commit directly on beta that removes all the stable hash project attributes. This is entirely risk free (I don't see how it could possibly cause problems to stop ignoring spans during stable hashing) and only makes incremental perf worse by a few percent, while also fixing the bug.

[michaelwoerister]

I think we should do something here. If this is an ICE that shows up multiple times a day in people's regular workflow, it will give the impression of being a severe bug.

We could consider making a commit directly on beta that removes all the stable hash project attributes. This is entirely risk free [...]

I agree that a performance hit (even a big one) is acceptable if it allows us to prevent the compiler from ICEing frequently. @oli-obk's proposed mitigation should work -- but it would still be good to have a few days of focused testing.

How flexible are we around moving the release by a little bit?

[Mark-Simulacrum]

Release is Thursday, I don't think we should move it at this point. We can backport (or only land) patches into it up until ~Wednesday evening in the worst case.

[estebank]

@pnkfelix, @wesleywiser, @rust-lang/compiler, I agree with @oli-obk's proposed stop-gap solution for this fingerprinting issue that seems to be prevalent enough in beta to be concerning (at least to me). I believe that we should get a patch ready and backported before tuesday (outside of our weekly backport evaluation schedule). I believe that this should be possible if we have someone in the team with the spare cycles to do this.

[michaelwoerister]

I created a commit that applies the mitigation here: https://github.com/michaelwoerister/rust/tree/mitigate-issue-94124. Local testing is still running.

I (or someone else) can turn this into a pull request later today. But I think we need to first OK this with the compiler team leads, as @estebank suggests. It would also be good to get the opinion of at least @Aaron1011 and @cjgillot on whether the mitigation is indeed safe enough.

[wesleywiser]

If this is an ICE that shows up multiple times a day in people's regular workflow, it will give the impression of being a severe bug.

Yeah, I totally agree with @michaelwoerister's point here. I would rather we proactively decide to take this perf hit now than have to decide if we should do something in a .1 patch in a few days.

Thanks for reporting @carols10cents!

[michaelwoerister]

Hm, I'm getting lots of linker errors in incr. comp. tests with that applied. I seem to remember that there was a problem with spans making their way into symbol name hashes...

[michaelwoerister]

It looks like we also need to backport #92278 for the mitigation to work. Otherwise spans become part of the symbol hash due to a bug in the stable hash caching infrastructure.

[Aaron1011]

It looks like we also need to backport #92278 for the mitigation to work. Otherwise spans become part of the symbol hash due to a bug in the stable hash caching infrastructure.

This is exactly the kind of thing that I was worried about. @oli-obk has thought that just hashing more spans would be entirely risk free (and I didn't see any obvious issues when they suggested it). However, this turned out to be incorrect due to the fact that the incremental compilation logic is so tightly couples to other parts of the compiler (symbol hashing, in this case).

I think incremental compilation is already insufficiently tested, and the tests we do have in src/test/incremental don't really cover the large-scale patterns of edits that users make in practice. Performing even less testing by making some changes skip ahead in the release train, and potentially introducing different bugs into beta due to running some unusual subset of the changes in master, seems like a step in the wrong direction.

I recognize the disruption that incremental compilation bugs cause to users workflows. I'm not sure what the right solution is here (disabling incr comp like we did a while back seems pretty extreme), but I don't think we should try to paper over these bugs by backporting a solution that (by definition) will have even less testing than the current implementation

[michaelwoerister]

The "solution" at https://github.com/michaelwoerister/rust/tree/mitigate-issue-94124 now passes tests for me locally. I added the commits from #92278 and had to disable a few incr. comp. tests because fingerprints depend on source locations in more cases -- so that was expected.

However, I also had to disable one test because it now failed consistently with an "unstable fingerprint" assertion -- which of course makes you really wonder if the cure isn't worse than the disease here.

I agree with @Aaron1011's statement that merging almost untested changes to incr. comp. into stable is not a good idea -- and the findings above only seem to confirm that. I had hoped we'd get a lucky break here but it doesn't look like it.

I still think it's not an option to let this bug make it into stable as is. Are there other things we can do? Make the compiler deal with the error more gracefully maybe?

[pnkfelix]

So: my current take is this:

• We should not release with incr-comp on by default with ICE's that have messaging like the one in this bug. Its not an acceptable end-user experience.
• Talking about this with @oli-obk and @estebank , one suggestion is to change all the indexing operations within compiler/rustc_query_impl/src/on_disk_cache.rs to use .get(...).expect(MSG) where MSG gives advice on how to workaround the issue.
• @oli-obk also suggested that we might be able to add catch_unwind's to the incremental operations and catch all panics of this nature.

In any case, my attitude is that we might consider messaging changes alone, if we can get it done in time.

If we cannot get the messaging changes done in time, or if they end up being broader/too risky for whatever reason, then I'm inclined to disable incr-comp by default for the 1.59.0 release.

cc @rust-lang/compiler @rust-lang/release

[Mark-Simulacrum]

Rather than switching indexing, since the ICE message already has CLI options, we can likely use that to give a better error message. That said, I am not personally convinced an error message is of much help here - ultimately, my understanding is the best we can offer is a "cargo clean" suggestion, right?

We should probably schedule some time to discuss incremental testing and/or policy in general here - my impression is we're basically going to flip flop every N release cycles with the current state, though steadily reducing the amount of bugs.

[pnkfelix]

That said, I am not personally convinced an error message is of much help here - ultimately, my understanding is the best we can offer is a "cargo clean" suggestion, right?

We can tell them to cargo clean to work-around the immediate error.

We might also tell them to disable incremental-compilation entirely (though then that setting might stay in their codebase forever, even after the bugs are fixed, so its not the greatest option; this is a case where a version-conditional setting in the Cargo.toml might provide a better long term experience to address that scenario).

The more I talk about this, the more my gut veers towards "disable incr-comp by default for this release."

We should probably schedule some time to discuss incremental testing and/or policy in general here - my impression is we're basically going to flip flop every N release cycles with the current state, though steadily reducing the amount of bugs.

I agree we should schedule some time for this.

I do wonder: if the number of incr-comp bugs goes down over time, we might expect the time N between flip-off-cycles to increase over time, and that might be an acceptable state of affairs? (As long as the time to flip back on is a small constant, that is...)

[michaelwoerister]

I think turning it off by default and adding a message in the release notes is an acceptable option.

[Mark-Simulacrum]

I've posted the initial stable artifacts build (#94224) with incremental soft-disabled; we can of course iterate on the exact language there and likely a section for the blog post also makes sense. cc @tmandry on the latter, or maybe someone here wants to draft something up.

We have time until ~Wednesday to rebuild artifacts with a different patch, of course, but I want a set of artifacts we could release and this seems like the safest set.

[philipcraig]

Hi everyone, this bug in beta is so prevalent that lots of tests in https://github.com/google/autocxx/blob/main/integration-tests/tests/integration_test.rs have had to be disabled for beta builds. Incremental compiles fail crazily often, and it is breaking the integration tests that use beta channel on the CI, always with this same compile panic.

If the beta channel gets promoted to stable in this state, it will look like a major compilation regression for many rustc users, and will have to be urgently patched in a .1 release post-promotion

[pnkfelix]

If the beta channel gets promoted to stable in this state, it will look like a major compilation regression for many rustc users, and will have to be urgently patched in a .1 release post-promotion

Just to assuage concerns of anyone following along here: We are not letting the beta channel get promoted to stable in the state described here.

As noted by @Mark-Simulacrum above, we have now "soft-disabled" incremental on 1.59.0-stable.

This means that people won't run into this bug unless they jump through the necessary hoops to re-enable incremental compilation

[workingjubilee]

I believe that since these mitigations were deployed in the short-term and since these problems were resolved in the long term, we can close this as one more chapter in Rust's intriguing history?

[philipcraig]

I believe that since these mitigations were deployed in the short-term and since these problems were resolved in the long term, we can close this as one more chapter in Rust's intriguing history?

Agreed.