From 3857d9c2da1250546ccb87ab7ddcc68904f45dc5 Mon Sep 17 00:00:00 2001 From: David Wood Date: Mon, 24 Jul 2023 15:47:03 +0100 Subject: [PATCH 1/2] borrowck/errors: fix i18n error in delayed bug During borrowck, the `MultiSpan` from a buffered diagnostic is cloned and used to emit a delayed bug indicating a diagnostic was buffered - when the buffered diagnostic is translated, then the cloned `MultiSpan` may contain labels which can only render with the diagnostic's arguments, but the delayed bug being emitted won't have those arguments. Adds a function which clones `MultiSpan` without also cloning the contained labels, and use this function when creating the buffered diagnostic delayed bug. Signed-off-by: David Wood --- compiler/rustc_borrowck/src/lib.rs | 9 ++++----- compiler/rustc_error_messages/src/lib.rs | 8 ++++++++ 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/compiler/rustc_borrowck/src/lib.rs b/compiler/rustc_borrowck/src/lib.rs index ea32506fc8980..2fd42e2f56a63 100644 --- a/compiler/rustc_borrowck/src/lib.rs +++ b/compiler/rustc_borrowck/src/lib.rs @@ -2306,11 +2306,10 @@ mod error { pub fn buffer_error(&mut self, t: DiagnosticBuilder<'_, ErrorGuaranteed>) { if let None = self.tainted_by_errors { - self.tainted_by_errors = Some( - self.tcx - .sess - .delay_span_bug(t.span.clone(), "diagnostic buffered but not emitted"), - ) + self.tainted_by_errors = Some(self.tcx.sess.delay_span_bug( + t.span.clone_ignoring_labels(), + "diagnostic buffered but not emitted", + )) } t.buffer(&mut self.buffered); } diff --git a/compiler/rustc_error_messages/src/lib.rs b/compiler/rustc_error_messages/src/lib.rs index 1879ece59e329..a49b842d9ffea 100644 --- a/compiler/rustc_error_messages/src/lib.rs +++ b/compiler/rustc_error_messages/src/lib.rs @@ -533,6 +533,14 @@ impl MultiSpan { pub fn has_span_labels(&self) -> bool { self.span_labels.iter().any(|(sp, _)| !sp.is_dummy()) } + + /// Clone this `MultiSpan` without keeping any of the span labels - sometimes a `MultiSpan` is + /// to be re-used in another diagnostic, but includes `span_labels` which have translated + /// messages. These translated messages would fail to translate without their diagnostic + /// arguments which are unlikely to be cloned alongside the `Span`. + pub fn clone_ignoring_labels(&self) -> Self { + Self { primary_spans: self.primary_spans.clone(), ..MultiSpan::new() } + } } impl From for MultiSpan { From 037b27430bc338ca5722aeb87a4614e70a5b3df4 Mon Sep 17 00:00:00 2001 From: David Wood Date: Tue, 25 Jul 2023 15:24:58 +0100 Subject: [PATCH 2/2] abi: unsized field in union - assert to delay bug Unions cannot have unsized fields, and as such, layout computation for unions asserts that each union field is sized (as this would normally have halted compilation earlier). However, if a generator ends up with an unsized local - a circumstance in which an error will always have been emitted earlier, for example, if attempting to dereference a `&str` - then the generator transform will produce a union with an unsized field. Since #110107, later passes will be run, such as constant propagation, and can attempt layout computation on the generator, which will result in layout computation of `str` in the context of it being a field of a union - and so the aforementioned assertion would cause an ICE. It didn't seem appropriate to try and detect this case in the MIR body and skip this specific pass; tainting the MIR body or delaying a bug from the generator transform (or elsewhere) wouldn't prevent this either (as neither would prevent the later pass from running); and tainting when the deref of `&str` is reported, if that's possible, would unnecessarily prevent potential other errors from being reported later in compilation, and is very tailored to this specific case of getting a unsized type in a generator. Given that this circumstance can only happen when an error should have already been reported, the correct fix appears to be just changing the assert to a delayed bug. This will still assert if there is some circumstance where this occurs and no error has been reported, but it won't crash the compiler in this instance. Signed-off-by: David Wood --- compiler/rustc_abi/src/layout.rs | 4 +++- compiler/rustc_abi/src/lib.rs | 1 - tests/ui/generator/issue-113279.rs | 27 ++++++++++++++++++++++++++ tests/ui/generator/issue-113279.stderr | 16 +++++++++++++++ 4 files changed, 46 insertions(+), 2 deletions(-) create mode 100644 tests/ui/generator/issue-113279.rs create mode 100644 tests/ui/generator/issue-113279.stderr diff --git a/compiler/rustc_abi/src/layout.rs b/compiler/rustc_abi/src/layout.rs index aea88641f82b1..489a8403c3b1f 100644 --- a/compiler/rustc_abi/src/layout.rs +++ b/compiler/rustc_abi/src/layout.rs @@ -763,7 +763,9 @@ pub trait LayoutCalculator { let mut size = Size::ZERO; let only_variant = &variants[FIRST_VARIANT]; for field in only_variant { - assert!(field.0.is_sized()); + if field.0.is_unsized() { + self.delay_bug("unsized field in union".to_string()); + } align = align.max(field.align()); max_repr_align = max_repr_align.max(field.max_repr_align()); diff --git a/compiler/rustc_abi/src/lib.rs b/compiler/rustc_abi/src/lib.rs index ef0c763ac2038..93b29d037d67e 100644 --- a/compiler/rustc_abi/src/lib.rs +++ b/compiler/rustc_abi/src/lib.rs @@ -1345,7 +1345,6 @@ impl Abi { /// Discard validity range information and allow undef. pub fn to_union(&self) -> Self { - assert!(self.is_sized()); match *self { Abi::Scalar(s) => Abi::Scalar(s.to_union()), Abi::ScalarPair(s1, s2) => Abi::ScalarPair(s1.to_union(), s2.to_union()), diff --git a/tests/ui/generator/issue-113279.rs b/tests/ui/generator/issue-113279.rs new file mode 100644 index 0000000000000..f69f804b716d5 --- /dev/null +++ b/tests/ui/generator/issue-113279.rs @@ -0,0 +1,27 @@ +#![feature(generators)] + +// `foo` attempts to dereference `""`, which results in an error being reported. Later, the +// generator transform for `foo` then produces a union which contains a `str` type - unions should +// not contain unsized types, but this is okay because an error has been reported already. +// When const propagation happens later in compilation, it attempts to compute the layout of the +// generator (as part of checking whether something can be const propagated) and in turn attempts +// to compute the layout of `str` in the context of a union - where this caused an ICE. This test +// makes sure that doesn't happen again. + +fn foo() { + let _y = static || { + let x = &mut 0; + *{ + yield; + x + } += match { *"" }.len() { + //~^ ERROR cannot move a value of type `str` [E0161] + //~^^ ERROR cannot move out of a shared reference [E0507] + _ => 0, + }; + }; +} + +fn main() { + foo() +} diff --git a/tests/ui/generator/issue-113279.stderr b/tests/ui/generator/issue-113279.stderr new file mode 100644 index 0000000000000..cc9b64ef9ac2f --- /dev/null +++ b/tests/ui/generator/issue-113279.stderr @@ -0,0 +1,16 @@ +error[E0161]: cannot move a value of type `str` + --> $DIR/issue-113279.rs:17:20 + | +LL | } += match { *"" }.len() { + | ^^^^^^^ the size of `str` cannot be statically determined + +error[E0507]: cannot move out of a shared reference + --> $DIR/issue-113279.rs:17:22 + | +LL | } += match { *"" }.len() { + | ^^^ move occurs because value has type `str`, which does not implement the `Copy` trait + +error: aborting due to 2 previous errors + +Some errors have detailed explanations: E0161, E0507. +For more information about an error, try `rustc --explain E0161`.