Skip to content

Commit

Permalink
review
Browse files Browse the repository at this point in the history
  • Loading branch information
@lcnr
lcnr committed Oct 23, 2020
1 parent 6ad140c commit 47cb871
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 35 deletions.
68 changes: 36 additions & 32 deletions compiler/rustc_trait_selection/src/traits/const_evaluatable.rs
View file
Expand Up @@ -223,6 +223,13 @@ 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>,
Expand All @@ -232,7 +239,7 @@ struct AbstractConstBuilder<'a, 'tcx> {
/// 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, (Node<'tcx>, bool)>,
nodes: IndexVec<NodeId, WorkNode<'tcx>>,
locals: IndexVec<mir::Local, NodeId>,
/// We only allow field accesses if they access
/// the result of a checked operation.
Expand Down Expand Up @@ -279,25 +286,25 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
Ok(Some(builder))
}

fn add_node(&mut self, n: Node<'tcx>) -> NodeId {
fn add_node(&mut self, node: Node<'tcx>, span: Span) -> NodeId {
// Mark used nodes.
match n {
match node {
Node::Leaf(_) => (),
Node::Binop(_, lhs, rhs) => {
self.nodes[lhs].1 = true;
self.nodes[rhs].1 = true;
self.nodes[lhs].used = true;
self.nodes[rhs].used = true;
}
Node::UnaryOp(_, input) => {
self.nodes[input].1 = true;
self.nodes[input].used = true;
}
Node::FunctionCall(func, nodes) => {
self.nodes[func].1 = true;
nodes.iter().for_each(|&n| self.nodes[n].1 = true);
self.nodes[func].used = true;
nodes.iter().for_each(|&n| self.nodes[n].used = true);
}
}

// Nodes start as unused.
self.nodes.push((n, false))
self.nodes.push(WorkNode { node, span, used: false })
}

fn place_to_local(
Expand Down Expand Up @@ -337,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.add_node(Node::Leaf(ct.literal))),
mir::Operand::Constant(ct) => Ok(self.add_node(Node::Leaf(ct.literal), span)),
}
}

Expand All @@ -362,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.add_node(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.add_node(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.add_node(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.
Expand Down Expand Up @@ -441,7 +448,7 @@ 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.add_node(Node::FunctionCall(func, args));
self.locals[local] = self.add_node(Node::FunctionCall(func, args), fn_span);
Ok(Some(target))
}
TerminatorKind::Assert { ref cond, expected: false, target, .. } => {
Expand All @@ -458,7 +465,7 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
//
// This is needed because division does not use `CheckedBinop` but instead
// adds an explicit assert for `divisor != 0`.
self.nodes[self.locals[p]].1 = true;
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.
Expand Down Expand Up @@ -487,16 +494,13 @@ impl<'a, 'tcx> AbstractConstBuilder<'a, 'tcx> {
if let Some(next) = self.build_terminator(block.terminator())? {
block = &self.body.basic_blocks()[next];
} else {
assert_eq!(self.locals[mir::Local::from_usize(0)], self.nodes.last().unwrap());
self.nodes[self.locals[mir::Local::from_usize(0)]].1 = true;
if !self.nodes.iter().all(|n| n.1) {
self.error(None, "unused node")?;
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, _used)| n)));
return Ok(self.tcx.arena.alloc_from_iter(self.nodes.into_iter().map(|n| n.node)));
}
}
}
Expand Down
12 changes: 9 additions & 3 deletions src/test/ui/const-generics/const_evaluatable_checked/unused_expr.stderr
View file
Expand Up @@ -2,23 +2,29 @@ error: overly complex generic constant
--> $DIR/unused_expr.rs:4:34
|
LL | fn add<const N: usize>() -> [u8; { N + 1; 5 }] {
| ^^^^^^^^^^^^ unused node
| ^^-----^^^^^
| |
| 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 }] {
| ^^^^^^^^^^^^ unused node
| ^^-----^^^^^
| |
| 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 }] {
| ^^^^^^^^^^^^^ unused node
| ^^------^^^^^
| |
| dead code
|
= help: consider moving this anonymous constant into a `const` function

Expand Down

0 comments on commit 47cb871

Please sign in to comment.