Skip to content

Commit 6ad140c

Browse files
lcnrlcnr
committed
const_eval_checked: deal with unused nodes + div
1 parent a9cd294 commit 6ad140c

File tree

4 files changed

+113
-12
lines changed

4 files changed

+113
-12
lines changed

‎compiler/rustc_trait_selection/src/traits/const_evaluatable.rs‎

Lines changed: 51 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -227,7 +227,12 @@ struct AbstractConstBuilder<'a, 'tcx> {
227227
tcx: TyCtxt<'tcx>,
228228
body: &'a mir::Body<'tcx>,
229229
/// The current WIP node tree.
230-
nodes: IndexVec<NodeId, Node<'tcx>>,
230+
///
231+
/// We require all nodes to be used in the final abstract const,
232+
/// so we store this here. Note that we also consider nodes as used
233+
/// if they are mentioned in an assert, so some used nodes are never
234+
/// actually reachable by walking the [`AbstractConst`].
235+
nodes: IndexVec<NodeId, (Node<'tcx>, bool)>,
231236
locals: IndexVec<mir::Local, NodeId>,
232237
/// We only allow field accesses if they access
233238
/// the result of a checked operation.
@@ -274,6 +279,27 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
274279
Ok(Some(builder))
275280
}
276281

282+
fn add_node(&mut self, n: Node<'tcx>) -> NodeId {
283+
// Mark used nodes.
284+
match n {
285+
Node::Leaf(_) => (),
286+
Node::Binop(_, lhs, rhs) => {
287+
self.nodes[lhs].1 = true;
288+
self.nodes[rhs].1 = true;
289+
}
290+
Node::UnaryOp(_, input) => {
291+
self.nodes[input].1 = true;
292+
}
293+
Node::FunctionCall(func, nodes) => {
294+
self.nodes[func].1 = true;
295+
nodes.iter().for_each(|&n| self.nodes[n].1 = true);
296+
}
297+
}
298+
299+
// Nodes start as unused.
300+
self.nodes.push((n, false))
301+
}
302+
277303
fn place_to_local(
278304
&mut self,
279305
span: Span,
@@ -311,7 +337,7 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
311337
let local = self.place_to_local(span, p)?;
312338
Ok(self.locals[local])
313339
}
314-
mir::Operand::Constant(ct) => Ok(self.nodes.push(Node::Leaf(ct.literal))),
340+
mir::Operand::Constant(ct) => Ok(self.add_node(Node::Leaf(ct.literal))),
315341
}
316342
}
317343

@@ -348,7 +374,7 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
348374
Rvalue::BinaryOp(op, ref lhs, ref rhs) if Self::check_binop(op) => {
349375
let lhs = self.operand_to_node(stmt.source_info.span, lhs)?;
350376
let rhs = self.operand_to_node(stmt.source_info.span, rhs)?;
351-
self.locals[local] = self.nodes.push(Node::Binop(op, lhs, rhs));
377+
self.locals[local] = self.add_node(Node::Binop(op, lhs, rhs));
352378
if op.is_checkable() {
353379
bug!("unexpected unchecked checkable binary operation");
354380
} else {
@@ -358,13 +384,13 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
358384
Rvalue::CheckedBinaryOp(op, ref lhs, ref rhs) if Self::check_binop(op) => {
359385
let lhs = self.operand_to_node(stmt.source_info.span, lhs)?;
360386
let rhs = self.operand_to_node(stmt.source_info.span, rhs)?;
361-
self.locals[local] = self.nodes.push(Node::Binop(op, lhs, rhs));
387+
self.locals[local] = self.add_node(Node::Binop(op, lhs, rhs));
362388
self.checked_op_locals.insert(local);
363389
Ok(())
364390
}
365391
Rvalue::UnaryOp(op, ref operand) if Self::check_unop(op) => {
366392
let operand = self.operand_to_node(stmt.source_info.span, operand)?;
367-
self.locals[local] = self.nodes.push(Node::UnaryOp(op, operand));
393+
self.locals[local] = self.add_node(Node::UnaryOp(op, operand));
368394
Ok(())
369395
}
370396
_ => self.error(Some(stmt.source_info.span), "unsupported rvalue")?,
@@ -415,13 +441,9 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
415441
.map(|arg| self.operand_to_node(terminator.source_info.span, arg))
416442
.collect::<Result<Vec<NodeId>, _>>()?,
417443
);
418-
self.locals[local] = self.nodes.push(Node::FunctionCall(func, args));
444+
self.locals[local] = self.add_node(Node::FunctionCall(func, args));
419445
Ok(Some(target))
420446
}
421-
// We only allow asserts for checked operations.
422-
//
423-
// These asserts seem to all have the form `!_local.0` so
424-
// we only allow exactly that.
425447
TerminatorKind::Assert { ref cond, expected: false, target, .. } => {
426448
let p = match cond {
427449
mir::Operand::Copy(p) | mir::Operand::Move(p) => p,
@@ -430,7 +452,15 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
430452

431453
const ONE_FIELD: mir::Field = mir::Field::from_usize(1);
432454
debug!("proj: {:?}", p.projection);
433-
if let &[mir::ProjectionElem::Field(ONE_FIELD, _)] = p.projection.as_ref() {
455+
if let Some(p) = p.as_local() {
456+
debug_assert!(!self.checked_op_locals.contains(p));
457+
// Mark locals directly used in asserts as used.
458+
//
459+
// This is needed because division does not use `CheckedBinop` but instead
460+
// adds an explicit assert for `divisor != 0`.
461+
self.nodes[self.locals[p]].1 = true;
462+
return Ok(Some(target));
463+
} else if let &[mir::ProjectionElem::Field(ONE_FIELD, _)] = p.projection.as_ref() {
434464
// Only allow asserts checking the result of a checked operation.
435465
if self.checked_op_locals.contains(p.local) {
436466
return Ok(Some(target));
@@ -457,7 +487,16 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
457487
if let Some(next) = self.build_terminator(block.terminator())? {
458488
block = &self.body.basic_blocks()[next];
459489
} else {
460-
return Ok(self.tcx.arena.alloc_from_iter(self.nodes));
490+
assert_eq!(self.locals[mir::Local::from_usize(0)], self.nodes.last().unwrap());
491+
self.nodes[self.locals[mir::Local::from_usize(0)]].1 = true;
492+
if !self.nodes.iter().all(|n| n.1) {
493+
self.error(None, "unused node")?;
494+
}
495+
496+
return Ok(self
497+
.tcx
498+
.arena
499+
.alloc_from_iter(self.nodes.into_iter().map(|(n, _used)| n)));
461500
}
462501
}
463502
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
// run-pass
2+
#![feature(const_generics, const_evaluatable_checked)]
3+
#![allow(incomplete_features)]
4+
5+
fn with_bound<const N: usize>() where [u8; N / 2]: Sized {
6+
let _: [u8; N / 2] = [0; N / 2];
7+
}
8+
9+
fn main() {
10+
with_bound::<4>();
11+
}

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

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
#![feature(const_generics, const_evaluatable_checked)]
2+
#![allow(incomplete_features)]
3+
4+
fn add<const N: usize>() -> [u8; { N + 1; 5 }] {
5+
//~^ ERROR overly complex generic constant
6+
todo!()
7+
}
8+
9+
fn div<const N: usize>() -> [u8; { N / 1; 5 }] {
10+
//~^ ERROR overly complex generic constant
11+
todo!()
12+
}
13+
14+
const fn foo(n: usize) {}
15+
16+
fn fn_call<const N: usize>() -> [u8; { foo(N); 5 }] {
17+
//~^ ERROR overly complex generic constant
18+
todo!()
19+
}
20+
21+
fn main() {
22+
add::<12>();
23+
div::<9>();
24+
fn_call::<14>();
25+
}

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: overly complex generic constant
2+
--> $DIR/unused_expr.rs:4:34
3+
|
4+
LL | fn add<const N: usize>() -> [u8; { N + 1; 5 }] {
5+
| ^^^^^^^^^^^^ unused node
6+
|
7+
= help: consider moving this anonymous constant into a `const` function
8+
9+
error: overly complex generic constant
10+
--> $DIR/unused_expr.rs:9:34
11+
|
12+
LL | fn div<const N: usize>() -> [u8; { N / 1; 5 }] {
13+
| ^^^^^^^^^^^^ unused node
14+
|
15+
= help: consider moving this anonymous constant into a `const` function
16+
17+
error: overly complex generic constant
18+
--> $DIR/unused_expr.rs:16:38
19+
|
20+
LL | fn fn_call<const N: usize>() -> [u8; { foo(N); 5 }] {
21+
| ^^^^^^^^^^^^^ unused node
22+
|
23+
= help: consider moving this anonymous constant into a `const` function
24+
25+
error: aborting due to 3 previous errors
26+

0 commit comments

Comments
 (0)