Rollup merge of rust-lang#95953 - JakobDegen:repeat-leak, r=oli-obk

Modify MIR building to drop repeat expressions with length zero

Closes rust-lang#74836 .

Previously, when a user wrote `[foo; 0]` we used to simply leak `foo`. The goal is to fix that. This PR changes MIR building to make `[foo; 0]` equivalent to `{ drop(foo); [] }` in all cases. Of course, this is a breaking change (see below). A crater run did not indicate any regressions though, and given that the previous behavior was almost definitely not what any user wanted, it seems unlikely that anyone was relying on this.

Note that const generics are in general unaffected by this. Inserting the extra `drop` is only meaningful/necessary when `foo` is of a non-`Copy` type, and array repeat expressions with const generic repetition count must always be `Copy`.

Besides the obvious change to behavior associated with the additional drop, there are three categories of examples where this also changes observable behavior. In all of these cases, the new behavior is consistent with what you would get by replacing `[foo; 0]` with `{ drop(foo); [] }`. As such, none of these give the user new powers to express more things.

**No longer allowed in const (breaking)**:

```rust
const _: [String; 0] = [String::new(); 0];
```

This compiles on stable today. Because we now introduce the drop of `String`, this no longer compiles as `String` may not be dropped in a const context.

**Reduced dataflow (non-breaking)**:

```rust
let mut x: i32 = 0;
let r = &x;
let a = [r; 0];
x = 5;
let _b = a;
```

Borrowck rejects this code on stable because it believes there is dataflow between `a` and `r`, and so the lifetime of `r` has to extend to the last statement. This change removes the dataflow and the above code is allowed to compile.

**More const promotion (non-breaking)**:

```rust
let _v: &'static [String; 0] = &[String::new(); 0];
```

This does not compile today because `String` having drop glue keeps it from being const promoted (despite that drop glue never being executed). After this change, this is allowed to compile.

### Alternatives

A previous attempt at this tried to reduce breakage by various tricks. This is still a possibility, but given that crater showed no regressions it seems unclear why we would want to introduce this complexity.

Disallowing `[foo; 0]` completely is also an option, but obviously this is more of a breaking change. I do not know how often this is actually used though.

r? `@oli-obk`

Author: Dylan-DPC

compiler/rustc_mir_build/src/build/expr/as_rvalue.rs

@@ -52,11 +52,20 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 })
             }
             ExprKind::Repeat { value, count } => {
-                let value_operand = unpack!(
-                    block =
-                        this.as_operand(block, scope, &this.thir[value], None, NeedsTemporary::No)
-                );
-                block.and(Rvalue::Repeat(value_operand, count))
+                if Some(0) == count.try_eval_usize(this.tcx, this.param_env) {
+                    this.build_zero_repeat(block, value, scope, source_info)
+                } else {
+                    let value_operand = unpack!(
+                        block = this.as_operand(
+                            block,
+                            scope,
+                            &this.thir[value],
+                            None,
+                            NeedsTemporary::No
+                        )
+                    );
+                    block.and(Rvalue::Repeat(value_operand, count))
+                }
             }
             ExprKind::Binary { op, lhs, rhs } => {
                 let lhs = unpack!(
@@ -515,6 +524,37 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
         }
     }
 
+    fn build_zero_repeat(
+        &mut self,
+        mut block: BasicBlock,
+        value: ExprId,
+        scope: Option<region::Scope>,
+        outer_source_info: SourceInfo,
+    ) -> BlockAnd<Rvalue<'tcx>> {
+        let this = self;
+        let value = &this.thir[value];
+        let elem_ty = value.ty;
+        if let Some(Category::Constant) = Category::of(&value.kind) {
+            // Repeating a const does nothing
+        } else {
+            // For a non-const, we may need to generate an appropriate `Drop`
+            let value_operand =
+                unpack!(block = this.as_operand(block, scope, value, None, NeedsTemporary::No));
+            if let Operand::Move(to_drop) = value_operand {
+                let success = this.cfg.start_new_block();
+                this.cfg.terminate(
+                    block,
+                    outer_source_info,
+                    TerminatorKind::Drop { place: to_drop, target: success, unwind: None },
+                );
+                this.diverge_from(block);
+                block = success;
+            }
+            this.record_operands_moved(&[value_operand]);
+        }
+        block.and(Rvalue::Aggregate(Box::new(AggregateKind::Array(elem_ty)), Vec::new()))
+    }
+
     fn limit_capture_mutability(
         &mut self,
         upvar_span: Span,

compiler/rustc_mir_build/src/build/scope.rs

@@ -1033,6 +1033,7 @@ impl<'a, 'tcx> Builder<'a, 'tcx> {
                 self.cfg.block_data(start).terminator().kind,
                 TerminatorKind::Assert { .. }
                     | TerminatorKind::Call { .. }
+                    | TerminatorKind::Drop { .. }
                     | TerminatorKind::DropAndReplace { .. }
                     | TerminatorKind::FalseUnwind { .. }
                     | TerminatorKind::InlineAsm { .. }

src/test/ui/drop/repeat-drop-2.rs

@@ -0,0 +1,15 @@
+fn borrowck_catch() {
+    let foo = String::new();
+    let _bar = foo;
+    let _baz = [foo; 0]; //~ ERROR use of moved value: `foo` [E0382]
+}
+
+const _: [String; 0] = [String::new(); 0];
+//~^ ERROR destructors cannot be evaluated at compile-time [E0493]
+
+fn must_be_init() {
+    let x: u8;
+    let _ = [x; 0]; //~ ERROR: use of possibly-uninitialized variable: `x`
+}
+
+fn main() {}

src/test/ui/drop/repeat-drop-2.stderr

@@ -0,0 +1,29 @@
+error[E0382]: use of moved value: `foo`
+  --> $DIR/repeat-drop-2.rs:4:17
+   |
+LL |     let foo = String::new();
+   |         --- move occurs because `foo` has type `String`, which does not implement the `Copy` trait
+LL |     let _bar = foo;
+   |                --- value moved here
+LL |     let _baz = [foo; 0];
+   |                 ^^^ value used here after move
+
+error[E0493]: destructors cannot be evaluated at compile-time
+  --> $DIR/repeat-drop-2.rs:7:25
+   |
+LL | const _: [String; 0] = [String::new(); 0];
+   |                        -^^^^^^^^^^^^^----
+   |                        ||
+   |                        |constants cannot evaluate destructors
+   |                        value is dropped here
+
+error[E0381]: use of possibly-uninitialized variable: `x`
+  --> $DIR/repeat-drop-2.rs:12:14
+   |
+LL |     let _ = [x; 0];
+   |              ^ use of possibly-uninitialized `x`
+
+error: aborting due to 3 previous errors
+
+Some errors have detailed explanations: E0381, E0382, E0493.
+For more information about an error, try `rustc --explain E0381`.

src/test/ui/drop/repeat-drop.rs

@@ -0,0 +1,117 @@
+// run-pass
+
+static mut CHECK: usize = 0;
+
+struct DropChecker(usize);
+
+impl Drop for DropChecker {
+    fn drop(&mut self) {
+        unsafe {
+            if CHECK != self.0 - 1 {
+                panic!("Found {}, should have found {}", CHECK, self.0 - 1);
+            }
+            CHECK = self.0;
+        }
+    }
+}
+
+macro_rules! check_drops {
+    ($l:literal) => {
+        unsafe { assert_eq!(CHECK, $l) }
+    };
+}
+
+struct DropPanic;
+
+impl Drop for DropPanic {
+    fn drop(&mut self) {
+        panic!()
+    }
+}
+
+fn value_zero() {
+    unsafe { CHECK = 0 };
+    let foo = DropChecker(1);
+    let v: [DropChecker; 0] = [foo; 0];
+    check_drops!(1);
+    std::mem::drop(v);
+    check_drops!(1);
+}
+
+fn value_one() {
+    unsafe { CHECK = 0 };
+    let foo = DropChecker(1);
+    let v: [DropChecker; 1] = [foo; 1];
+    check_drops!(0);
+    std::mem::drop(v);
+    check_drops!(1);
+}
+
+const DROP_CHECKER: DropChecker = DropChecker(1);
+
+fn const_zero() {
+    unsafe { CHECK = 0 };
+    let v: [DropChecker; 0] = [DROP_CHECKER; 0];
+    check_drops!(0);
+    std::mem::drop(v);
+    check_drops!(0);
+}
+
+fn const_one() {
+    unsafe { CHECK = 0 };
+    let v: [DropChecker; 1] = [DROP_CHECKER; 1];
+    check_drops!(0);
+    std::mem::drop(v);
+    check_drops!(1);
+}
+
+fn const_generic_zero<const N: usize>() {
+    unsafe { CHECK = 0 };
+    let v: [DropChecker; N] = [DROP_CHECKER; N];
+    check_drops!(0);
+    std::mem::drop(v);
+    check_drops!(0);
+}
+
+fn const_generic_one<const N: usize>() {
+    unsafe { CHECK = 0 };
+    let v: [DropChecker; N] = [DROP_CHECKER; N];
+    check_drops!(0);
+    std::mem::drop(v);
+    check_drops!(1);
+}
+
+// Make sure that things are allowed to promote as expected
+
+fn allow_promote() {
+    unsafe { CHECK = 0 };
+    let foo = DropChecker(1);
+    let v: &'static [DropChecker; 0] = &[foo; 0];
+    check_drops!(1);
+    std::mem::drop(v);
+    check_drops!(1);
+}
+
+// Verify that unwinding in the drop causes the right things to drop in the right order
+fn on_unwind() {
+    unsafe { CHECK = 0 };
+    std::panic::catch_unwind(|| {
+        let panic = DropPanic;
+        let _local = DropChecker(2);
+        let _v = (DropChecker(1), [panic; 0]);
+        std::process::abort();
+    })
+    .unwrap_err();
+    check_drops!(2);
+}
+
+fn main() {
+    value_zero();
+    value_one();
+    const_zero();
+    const_one();
+    const_generic_zero::<0>();
+    const_generic_one::<1>();
+    allow_promote();
+    on_unwind();
+}