Skip to content

Commit

Permalink
chore: remove some empty unreachable!s (#3982)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

This PR adds some strings to unreachable panics so that if they manage
to get run then we get a usable error message.

This should be a noop.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored Jan 8, 2024
1 parent 35f18ef commit 38c732c
Show file tree
Hide file tree
Showing 9 changed files with 55 additions and 28 deletions.
2 changes: 1 addition & 1 deletion acvm-repo/acir/src/circuit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ impl FromStr for OpcodeLocation {
let brillig_index = parts[1].parse()?;
Ok(OpcodeLocation::Brillig { acir_index, brillig_index })
}
_ => unreachable!(),
_ => unreachable!("`OpcodeLocation` has too many components"),
}
}

Expand Down
23 changes: 17 additions & 6 deletions acvm-repo/brillig_vm/src/black_box.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,11 +85,7 @@ pub(crate) fn evaluate_black_box<Solver: BlackBoxFunctionSolver>(
signature,
result: result_register,
} => {
let bb_func = match op {
BlackBoxOp::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1,
BlackBoxOp::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1,
_ => unreachable!(),
};
let bb_func = black_box_function_from_op(op);

let public_key_x: [u8; 32] = to_u8_vec(read_heap_array(
memory,
Expand Down Expand Up @@ -124,7 +120,7 @@ pub(crate) fn evaluate_black_box<Solver: BlackBoxFunctionSolver>(
BlackBoxOp::EcdsaSecp256r1 { .. } => {
ecdsa_secp256r1_verify(&hashed_msg, &public_key_x, &public_key_y, &signature)?
}
_ => unreachable!(),
_ => unreachable!("`BlackBoxOp` is guarded against being a non-ecdsa operation"),
};

registers.set(*result_register, result.into());
Expand Down Expand Up @@ -178,6 +174,21 @@ pub(crate) fn evaluate_black_box<Solver: BlackBoxFunctionSolver>(
}
}

fn black_box_function_from_op(op: &BlackBoxOp) -> BlackBoxFunc {
match op {
BlackBoxOp::Sha256 { .. } => BlackBoxFunc::SHA256,
BlackBoxOp::Blake2s { .. } => BlackBoxFunc::Blake2s,
BlackBoxOp::Keccak256 { .. } => BlackBoxFunc::Keccak256,
BlackBoxOp::HashToField128Security { .. } => BlackBoxFunc::HashToField128Security,
BlackBoxOp::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1,
BlackBoxOp::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1,
BlackBoxOp::SchnorrVerify { .. } => BlackBoxFunc::SchnorrVerify,
BlackBoxOp::PedersenCommitment { .. } => BlackBoxFunc::PedersenCommitment,
BlackBoxOp::PedersenHash { .. } => BlackBoxFunc::PedersenHash,
BlackBoxOp::FixedBaseScalarMul { .. } => BlackBoxFunc::FixedBaseScalarMul,
}
}

#[cfg(test)]
mod test {
use acir::brillig::BlackBoxOp;
Expand Down
8 changes: 5 additions & 3 deletions compiler/noirc_evaluator/src/ssa/function_builder/data_bus.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,6 @@ impl FunctionBuilder {
databus.values.push_back(value);
databus.index += 1;
}
Type::Reference(_) => unreachable!(),
Type::Array(typ, len) => {
assert!(typ.len() == 1, "unsupported composite type");
databus.map.insert(value, databus.index);
Expand All @@ -98,8 +97,11 @@ impl FunctionBuilder {
self.add_to_data_bus(element, databus);
}
}
Type::Slice(_) => unreachable!(),
Type::Function => unreachable!(),
Type::Reference(_) => {
unreachable!("Attempted to add invalid type (reference) to databus")
}
Type::Slice(_) => unreachable!("Attempted to add invalid type (slice) to databus"),
Type::Function => unreachable!("Attempted to add invalid type (function) to databus"),
}
}

Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -280,11 +280,11 @@ mod test {

let return_value_id = match entry_block.unwrap_terminator() {
TerminatorInstruction::Return { return_values, .. } => return_values[0],
_ => unreachable!(),
_ => unreachable!("Should have terminator instruction"),
};
let return_element = match &main.dfg[return_value_id] {
Value::Array { array, .. } => array[0],
_ => unreachable!(),
_ => unreachable!("Return type should be array"),
};
// The return element is expected to refer to the new add instruction result.
assert_eq!(main.dfg.resolve(new_add_instr_result), main.dfg.resolve(return_element));
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1102,7 +1102,7 @@ mod test {
let main = ssa.main();
let ret = match main.dfg[main.entry_block()].terminator() {
Some(TerminatorInstruction::Return { return_values, .. }) => return_values[0],
_ => unreachable!(),
_ => unreachable!("Should have terminator instruction"),
};

let merged_values = get_all_constants_reachable_from_instruction(&main.dfg, ret);
Expand Down Expand Up @@ -1473,7 +1473,7 @@ mod test {
None => unreachable!("Expected constant 200 for return value"),
}
}
_ => unreachable!(),
_ => unreachable!("Should have terminator instruction"),
}
}
}
15 changes: 14 additions & 1 deletion compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -257,7 +257,20 @@ impl UnresolvedTypeExpression {
BinaryOpKind::Multiply => BinaryTypeOperator::Multiplication,
BinaryOpKind::Divide => BinaryTypeOperator::Division,
BinaryOpKind::Modulo => BinaryTypeOperator::Modulo,
_ => unreachable!(), // impossible via operator_allowed check

BinaryOpKind::Equal
| BinaryOpKind::NotEqual
| BinaryOpKind::Less
| BinaryOpKind::LessEqual
| BinaryOpKind::Greater
| BinaryOpKind::GreaterEqual
| BinaryOpKind::And
| BinaryOpKind::Or
| BinaryOpKind::Xor
| BinaryOpKind::ShiftRight
| BinaryOpKind::ShiftLeft => {
unreachable!("impossible via `operator_allowed` check")
}
};
Ok(UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, expr.span))
}
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/type_check/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ impl<'interner> TypeChecker<'interner> {
// Must push new lvalue to the interner, we've resolved any field indices
self.interner.update_statement(stmt_id, |stmt| match stmt {
HirStatement::Assign(assign) => assign.lvalue = new_lvalue,
_ => unreachable!(),
_ => unreachable!("statement is known to be assignment"),
});

let span = self.interner.expr_span(&assign_stmt.expression);
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ fn function_return_type() -> impl NoirParser<((Distinctness, Visibility), Functi
fn attribute() -> impl NoirParser<Attribute> {
token_kind(TokenKind::Attribute).map(|token| match token {
Token::Attribute(attribute) => attribute,
_ => unreachable!(),
_ => unreachable!("Parser should have already errored due to token not being an attribute"),
})
}

Expand Down Expand Up @@ -369,7 +369,7 @@ fn function_parameters<'a>(allow_self: bool) -> impl NoirParser<Vec<Param>> + 'a

/// This parser always parses no input and fails
fn nothing<T>() -> impl NoirParser<T> {
one_of([]).map(|_| unreachable!())
one_of([]).map(|_| unreachable!("parser should always error"))
}

fn self_parameter() -> impl NoirParser<Param> {
Expand Down
21 changes: 11 additions & 10 deletions tooling/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -157,11 +157,7 @@ impl AbiType {
.expect("Cannot have variable sized strings as a parameter to main");
Self::String { length: size }
}
Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"),
Type::Error => unreachable!(),
Type::Unit => unreachable!(),
Type::Constant(_) => unreachable!(),
Type::TraitAsType(..) => unreachable!(),

Type::Struct(def, ref args) => {
let struct_type = def.borrow();
let fields = struct_type.get_fields(args);
Expand All @@ -175,12 +171,17 @@ impl AbiType {
let fields = vecmap(fields, |typ| Self::from_type(context, typ));
Self::Tuple { fields }
}
Type::TypeVariable(_, _) => unreachable!(),
Type::NamedGeneric(..) => unreachable!(),
Type::Forall(..) => unreachable!(),
Type::Function(_, _, _) => unreachable!(),
Type::Error
| Type::Unit
| Type::Constant(_)
| Type::TraitAsType(..)
| Type::TypeVariable(_, _)
| Type::NamedGeneric(..)
| Type::Forall(..)
| Type::NotConstant
| Type::Function(_, _, _) => unreachable!("Type cannot be used in the abi"),
Type::FmtString(_, _) => unreachable!("format strings cannot be used in the abi"),
Type::MutableReference(_) => unreachable!("&mut cannot be used in the abi"),
Type::NotConstant => unreachable!(),
}
}

Expand Down

0 comments on commit 38c732c

Please sign in to comment.