Skip to content

Commit

Permalink
chore: resolved some clippy::pedantic lints (#260)
Browse files Browse the repository at this point in the history
# Rationale for this change

We have cargo clippy running in our CI in order to enforce code quality.
In order to increase our standards, we should enable the
clippy::pedantic lint group.

# What changes are included in this PR?

Resolved the following lint warnings

`bool_to_int_with_if`
`ptr_as_ptr`
`match_wildcard_for_single_variants`
`match_bool`
`manual_assert`
`trivially_copy_pass_by_ref`

# Are these changes tested?
Yes.
  • Loading branch information
mehulmathur16 authored Oct 11, 2024
1 parent 98452c4 commit 40f1f77
Show file tree
Hide file tree
Showing 10 changed files with 50 additions and 34 deletions.
6 changes: 6 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -91,3 +91,9 @@ struct_field_names = "deny"
unicode_not_nfc = "deny"
manual_string_new = "deny"
large_types_passed_by_value = "deny"
bool_to_int_with_if = "deny"
ptr_as_ptr = "deny"
match_wildcard_for_single_variants = "deny"
match_bool = "deny"
manual_assert = "deny"
trivially_copy_pass_by_ref = "deny"
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ pub fn test_random_commitment_evaluation_proof<CP: CommitmentEvaluationProof>(
assert!(r.is_err(), "verification improperly succeeded");

// Invalid offset
let wrong_offset = if offset == 0 { 1 } else { 0 };
let wrong_offset = u64::from(offset == 0);
let mut transcript = Transcript::new(b"evaluation_proof");
let r = proof.verify_proof(
&mut transcript,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ where
}

fn id(&self) -> *const c_void {
self.as_ptr() as *const c_void
self.as_ptr().cast::<c_void>()
}
}

Expand Down
16 changes: 8 additions & 8 deletions crates/proof-of-sql/src/base/scalar/mont_scalar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -363,15 +363,15 @@ impl<T: MontConfig<4>> From<&MontScalar<T>> for [u64; 4] {

impl<T: MontConfig<4>> Display for MontScalar<T> {
fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
let sign = match f.sign_plus() {
true => {
let n = -self;
match self > &n {
true => Some(Some(n)),
false => Some(None),
}
let sign = if f.sign_plus() {
let n = -self;
if self > &n {
Some(Some(n))
} else {
Some(None)
}
false => None,
} else {
None
};
match (f.alternate(), sign) {
(false, None) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,9 +290,12 @@ pub(super) mod tests {
k => Some(k),
})
.enumerate()
.filter_map(|(i, b)| match b % 2 == 0 {
true => None,
false => Some(point.get(i).copied().unwrap_or(F::ZERO)),
.filter_map(|(i, b)| {
if b % 2 == 0 {
None
} else {
Some(point.get(i).copied().unwrap_or(F::ZERO))
}
})
.product()
}
Expand Down
15 changes: 9 additions & 6 deletions crates/proof-of-sql/src/proof_primitive/sumcheck/prover_round.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,11 @@ use rayon::prelude::*;
#[tracing::instrument(level = "debug", skip_all)]
pub fn prove_round<S: Scalar>(prover_state: &mut ProverState<S>, r_maybe: &Option<S>) -> Vec<S> {
if let Some(r) = r_maybe {
if prover_state.round == 0 {
panic!("first round should be prover first.");
}
assert!(
prover_state.round != 0,
"first round should be prover first."
);

prover_state.randomness.push(*r);

// fix argument
Expand All @@ -38,9 +40,10 @@ pub fn prove_round<S: Scalar>(prover_state: &mut ProverState<S>, r_maybe: &Optio

prover_state.round += 1;

if prover_state.round > prover_state.num_vars {
panic!("Prover is not active");
}
assert!(
prover_state.round <= prover_state.num_vars,
"Prover is not active"
);

let degree = prover_state.max_multiplicands; // the degree of univariate polynomial sent by prover at this round
let round_length = 1usize << (prover_state.num_vars - prover_state.round);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,10 @@ pub struct ProverState<S: Scalar> {
impl<S: Scalar> ProverState<S> {
#[tracing::instrument(name = "ProverState::create", level = "debug", skip_all)]
pub fn create(polynomial: &CompositePolynomial<S>) -> Self {
if polynomial.num_variables == 0 {
panic!("Attempt to prove a constant.")
}
assert!(
polynomial.num_variables != 0,
"Attempt to prove a constant."
);

// create a deep copy of all unique MLExtensions
let flattened_ml_extensions = polynomial
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ impl<'a> DynProofExprBuilder<'a> {
}
}

#[allow(clippy::match_wildcard_for_single_variants)]
// Private interface
impl DynProofExprBuilder<'_> {
fn visit_expr<C: Commitment>(
Expand Down
18 changes: 9 additions & 9 deletions crates/proof-of-sql/src/sql/parse/query_context_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,9 @@ impl<'a> QueryContextBuilder<'a> {
Expression::Wildcard => Ok(ColumnType::BigInt), // Since COUNT(*) = COUNT(1)
Expression::Literal(literal) => self.visit_literal(literal),
Expression::Column(_) => self.visit_column_expr(expr),
Expression::Unary { op, expr } => self.visit_unary_expr(op, expr),
Expression::Binary { op, left, right } => self.visit_binary_expr(op, left, right),
Expression::Aggregation { op, expr } => self.visit_agg_expr(op, expr),
Expression::Unary { op, expr } => self.visit_unary_expr(*op, expr),
Expression::Binary { op, left, right } => self.visit_binary_expr(*op, left, right),
Expression::Aggregation { op, expr } => self.visit_agg_expr(*op, expr),
}
}

Expand All @@ -152,13 +152,13 @@ impl<'a> QueryContextBuilder<'a> {

fn visit_binary_expr(
&mut self,
op: &BinaryOperator,
op: BinaryOperator,
left: &Expression,
right: &Expression,
) -> ConversionResult<ColumnType> {
let left_dtype = self.visit_expr(left)?;
let right_dtype = self.visit_expr(right)?;
check_dtypes(left_dtype, right_dtype, *op)?;
check_dtypes(left_dtype, right_dtype, op)?;
match op {
BinaryOperator::And
| BinaryOperator::Or
Expand All @@ -174,7 +174,7 @@ impl<'a> QueryContextBuilder<'a> {

fn visit_unary_expr(
&mut self,
op: &UnaryOperator,
op: UnaryOperator,
expr: &Expression,
) -> ConversionResult<ColumnType> {
match op {
Expand All @@ -193,15 +193,15 @@ impl<'a> QueryContextBuilder<'a> {

fn visit_agg_expr(
&mut self,
op: &AggregationOperator,
op: AggregationOperator,
expr: &Expression,
) -> ConversionResult<ColumnType> {
self.context.set_in_agg_scope(true)?;

let expr_dtype = self.visit_expr(expr)?;

// We only support sum/max/min aggregations on numeric columns.
if op != &AggregationOperator::Count && expr_dtype == ColumnType::VarChar {
if op != AggregationOperator::Count && expr_dtype == ColumnType::VarChar {
return Err(ConversionError::non_numeric_expr_in_agg(
expr_dtype.to_string(),
op.to_string(),
Expand All @@ -211,7 +211,7 @@ impl<'a> QueryContextBuilder<'a> {
self.context.set_in_agg_scope(false)?;

// Count aggregation always results in an integer type
if op == &AggregationOperator::Count {
if op == AggregationOperator::Count {
Ok(ColumnType::BigInt)
} else {
Ok(expr_dtype)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@ pub fn exercise_verification(
table_ref: TableRef,
) {
let verification_result = res.verify(expr, accessor, &());
if verification_result.is_err() {
panic!("Verification failed: {:?}", verification_result.err());
}
assert!(
verification_result.is_ok(),
"Verification failed: {:?}",
verification_result.err()
);

// try changing the result
tamper_result(res, expr, accessor);
Expand Down

0 comments on commit 40f1f77

Please sign in to comment.