Rollup merge of #78272 - lcnr:abstract-const-unused-node, r=oli-obk

const_evaluatable_checked: deal with unused nodes + div

r? @oli-obk

Author: jonas-schievink

compiler/rustc_trait_selection/src/traits/const_evaluatable.rs

@@ -223,11 +223,23 @@ impl AbstractConst<'tcx> {
     }
 }
 
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+struct WorkNode<'tcx> {
+    node: Node<'tcx>,
+    span: Span,
+    used: bool,
+}
+
 struct AbstractConstBuilder<'a, 'tcx> {
     tcx: TyCtxt<'tcx>,
     body: &'a mir::Body<'tcx>,
     /// The current WIP node tree.
-    nodes: IndexVec<NodeId, Node<'tcx>>,
+    ///
+    /// We require all nodes to be used in the final abstract const,
+    /// so we store this here. Note that we also consider nodes as used
+    /// if they are mentioned in an assert, so some used nodes are never
+    /// actually reachable by walking the [`AbstractConst`].
+    nodes: IndexVec<NodeId, WorkNode<'tcx>>,
     locals: IndexVec<mir::Local, NodeId>,
     /// We only allow field accesses if they access
     /// the result of a checked operation.
@@ -274,6 +286,27 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
         Ok(Some(builder))
     }
 
+    fn add_node(&mut self, node: Node<'tcx>, span: Span) -> NodeId {
+        // Mark used nodes.
+        match node {
+            Node::Leaf(_) => (),
+            Node::Binop(_, lhs, rhs) => {
+                self.nodes[lhs].used = true;
+                self.nodes[rhs].used = true;
+            }
+            Node::UnaryOp(_, input) => {
+                self.nodes[input].used = true;
+            }
+            Node::FunctionCall(func, nodes) => {
+                self.nodes[func].used = true;
+                nodes.iter().for_each(|&n| self.nodes[n].used = true);
+            }
+        }
+
+        // Nodes start as unused.
+        self.nodes.push(WorkNode { node, span, used: false })
+    }
+
     fn place_to_local(
         &mut self,
         span: Span,
@@ -311,7 +344,7 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
                 let local = self.place_to_local(span, p)?;
                 Ok(self.locals[local])
             }
-            mir::Operand::Constant(ct) => Ok(self.nodes.push(Node::Leaf(ct.literal))),
+            mir::Operand::Constant(ct) => Ok(self.add_node(Node::Leaf(ct.literal), span)),
         }
     }
 
@@ -336,38 +369,38 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
 
     fn build_statement(&mut self, stmt: &mir::Statement<'tcx>) -> Result<(), ErrorReported> {
         debug!("AbstractConstBuilder: stmt={:?}", stmt);
+        let span = stmt.source_info.span;
         match stmt.kind {
             StatementKind::Assign(box (ref place, ref rvalue)) => {
-                let local = self.place_to_local(stmt.source_info.span, place)?;
+                let local = self.place_to_local(span, place)?;
                 match *rvalue {
                     Rvalue::Use(ref operand) => {
-                        self.locals[local] =
-                            self.operand_to_node(stmt.source_info.span, operand)?;
+                        self.locals[local] = self.operand_to_node(span, operand)?;
                         Ok(())
                     }
                     Rvalue::BinaryOp(op, ref lhs, ref rhs) if Self::check_binop(op) => {
-                        let lhs = self.operand_to_node(stmt.source_info.span, lhs)?;
-                        let rhs = self.operand_to_node(stmt.source_info.span, rhs)?;
-                        self.locals[local] = self.nodes.push(Node::Binop(op, lhs, rhs));
+                        let lhs = self.operand_to_node(span, lhs)?;
+                        let rhs = self.operand_to_node(span, rhs)?;
+                        self.locals[local] = self.add_node(Node::Binop(op, lhs, rhs), span);
                         if op.is_checkable() {
                             bug!("unexpected unchecked checkable binary operation");
                         } else {
                             Ok(())
                         }
                     }
                     Rvalue::CheckedBinaryOp(op, ref lhs, ref rhs) if Self::check_binop(op) => {
-                        let lhs = self.operand_to_node(stmt.source_info.span, lhs)?;
-                        let rhs = self.operand_to_node(stmt.source_info.span, rhs)?;
-                        self.locals[local] = self.nodes.push(Node::Binop(op, lhs, rhs));
+                        let lhs = self.operand_to_node(span, lhs)?;
+                        let rhs = self.operand_to_node(span, rhs)?;
+                        self.locals[local] = self.add_node(Node::Binop(op, lhs, rhs), span);
                         self.checked_op_locals.insert(local);
                         Ok(())
                     }
                     Rvalue::UnaryOp(op, ref operand) if Self::check_unop(op) => {
-                        let operand = self.operand_to_node(stmt.source_info.span, operand)?;
-                        self.locals[local] = self.nodes.push(Node::UnaryOp(op, operand));
+                        let operand = self.operand_to_node(span, operand)?;
+                        self.locals[local] = self.add_node(Node::UnaryOp(op, operand), span);
                         Ok(())
                     }
-                    _ => self.error(Some(stmt.source_info.span), "unsupported rvalue")?,
+                    _ => self.error(Some(span), "unsupported rvalue")?,
                 }
             }
             // These are not actually relevant for us here, so we can ignore them.
@@ -415,13 +448,9 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
                         .map(|arg| self.operand_to_node(terminator.source_info.span, arg))
                         .collect::<Result<Vec<NodeId>, _>>()?,
                 );
-                self.locals[local] = self.nodes.push(Node::FunctionCall(func, args));
+                self.locals[local] = self.add_node(Node::FunctionCall(func, args), fn_span);
                 Ok(Some(target))
             }
-            // We only allow asserts for checked operations.
-            //
-            // These asserts seem to all have the form `!_local.0` so
-            // we only allow exactly that.
             TerminatorKind::Assert { ref cond, expected: false, target, .. } => {
                 let p = match cond {
                     mir::Operand::Copy(p) | mir::Operand::Move(p) => p,
@@ -430,7 +459,15 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
 
                 const ONE_FIELD: mir::Field = mir::Field::from_usize(1);
                 debug!("proj: {:?}", p.projection);
-                if let &[mir::ProjectionElem::Field(ONE_FIELD, _)] = p.projection.as_ref() {
+                if let Some(p) = p.as_local() {
+                    debug_assert!(!self.checked_op_locals.contains(p));
+                    // Mark locals directly used in asserts as used.
+                    //
+                    // This is needed because division does not use `CheckedBinop` but instead
+                    // adds an explicit assert for `divisor != 0`.
+                    self.nodes[self.locals[p]].used = true;
+                    return Ok(Some(target));
+                } else if let &[mir::ProjectionElem::Field(ONE_FIELD, _)] = p.projection.as_ref() {
                     // Only allow asserts checking the result of a checked operation.
                     if self.checked_op_locals.contains(p.local) {
                         return Ok(Some(target));
@@ -457,7 +494,13 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
             if let Some(next) = self.build_terminator(block.terminator())? {
                 block = &self.body.basic_blocks()[next];
             } else {
-                return Ok(self.tcx.arena.alloc_from_iter(self.nodes));
+                assert_eq!(self.locals[mir::RETURN_PLACE], self.nodes.last().unwrap());
+                self.nodes[self.locals[mir::RETURN_PLACE]].used = true;
+                if let Some(&unused) = self.nodes.iter().find(|n| !n.used) {
+                    self.error(Some(unused.span), "dead code")?;
+                }
+
+                return Ok(self.tcx.arena.alloc_from_iter(self.nodes.into_iter().map(|n| n.node)));
             }
         }
     }

src/test/ui/const-generics/const_evaluatable_checked/division.rs

@@ -0,0 +1,11 @@
+// run-pass
+#![feature(const_generics, const_evaluatable_checked)]
+#![allow(incomplete_features)]
+
+fn with_bound<const N: usize>() where [u8; N / 2]: Sized {
+    let _: [u8; N / 2] = [0; N / 2];
+}
+
+fn main() {
+    with_bound::<4>();
+}

src/test/ui/const-generics/const_evaluatable_checked/unused_expr.rs

@@ -0,0 +1,25 @@
+#![feature(const_generics, const_evaluatable_checked)]
+#![allow(incomplete_features)]
+
+fn add<const N: usize>() -> [u8; { N + 1; 5 }] {
+    //~^ ERROR overly complex generic constant
+    todo!()
+}
+
+fn div<const N: usize>() -> [u8; { N / 1; 5 }] {
+    //~^ ERROR overly complex generic constant
+    todo!()
+}
+
+const fn foo(n: usize) {}
+
+fn fn_call<const N: usize>() -> [u8; { foo(N); 5 }] {
+    //~^ ERROR overly complex generic constant
+    todo!()
+}
+
+fn main() {
+    add::<12>();
+    div::<9>();
+    fn_call::<14>();
+}

src/test/ui/const-generics/const_evaluatable_checked/unused_expr.stderr

@@ -0,0 +1,32 @@
+error: overly complex generic constant
+  --> $DIR/unused_expr.rs:4:34
+   |
+LL | fn add<const N: usize>() -> [u8; { N + 1; 5 }] {
+   |                                  ^^-----^^^^^
+   |                                    |
+   |                                    dead code
+   |
+   = help: consider moving this anonymous constant into a `const` function
+
+error: overly complex generic constant
+  --> $DIR/unused_expr.rs:9:34
+   |
+LL | fn div<const N: usize>() -> [u8; { N / 1; 5 }] {
+   |                                  ^^-----^^^^^
+   |                                    |
+   |                                    dead code
+   |
+   = help: consider moving this anonymous constant into a `const` function
+
+error: overly complex generic constant
+  --> $DIR/unused_expr.rs:16:38
+   |
+LL | fn fn_call<const N: usize>() -> [u8; { foo(N); 5 }] {
+   |                                      ^^------^^^^^
+   |                                        |
+   |                                        dead code
+   |
+   = help: consider moving this anonymous constant into a `const` function
+
+error: aborting due to 3 previous errors
+