Skip to content

Commit

Permalink
fix: disable modulo for fields (#3009)
Browse files Browse the repository at this point in the history
  • Loading branch information
guipublic authored Oct 9, 2023
1 parent b71b15d commit 7e68976
Show file tree
Hide file tree
Showing 7 changed files with 35 additions and 7 deletions.
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,10 @@ impl BinaryOpKind {
pub fn is_bit_shift(&self) -> bool {
matches!(self, BinaryOpKind::ShiftRight | BinaryOpKind::ShiftLeft)
}

pub fn is_modulo(&self) -> bool {
matches!(self, BinaryOpKind::Modulo)
}
}

#[derive(PartialEq, PartialOrd, Eq, Ord, Hash, Debug, Copy, Clone)]
Expand Down
5 changes: 4 additions & 1 deletion compiler/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,8 @@ pub enum TypeCheckError {
IntegerTypeMismatch { typ: Type, span: Span },
#[error("Cannot use an integer and a Field in a binary operation, try converting the Field into an integer first")]
IntegerAndFieldBinaryOperation { span: Span },
#[error("Cannot do modulo on Fields, try casting to an integer first")]
FieldModulo { span: Span },
#[error("Fields cannot be compared, try casting to an integer first")]
FieldComparison { span: Span },
#[error("The number of bits to use for this bitwise operation is ambiguous. Either the operand's type or return type should be specified")]
Expand Down Expand Up @@ -195,7 +197,8 @@ impl From<TypeCheckError> for Diagnostic {
| TypeCheckError::FieldComparison { span, .. }
| TypeCheckError::AmbiguousBitWidth { span, .. }
| TypeCheckError::IntegerAndFieldBinaryOperation { span }
| TypeCheckError::OverflowingAssignment { span, .. } => {
| TypeCheckError::OverflowingAssignment { span, .. }
| TypeCheckError::FieldModulo { span } => {
Diagnostic::simple_error(error.to_string(), String::new(), span)
}
TypeCheckError::PublicReturnType { typ, span } => Diagnostic::simple_error(
Expand Down
15 changes: 11 additions & 4 deletions compiler/noirc_frontend/src/hir/type_check/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ use crate::{
types::Type,
},
node_interner::{DefinitionKind, ExprId, FuncId, TraitMethodId},
Signedness, TypeBinding, TypeVariableKind, UnaryOp,
BinaryOpKind, Signedness, TypeBinding, TypeVariableKind, UnaryOp,
};

use super::{errors::TypeCheckError, TypeChecker};
Expand Down Expand Up @@ -974,16 +974,20 @@ impl<'interner> TypeChecker<'interner> {
if let TypeBinding::Bound(binding) = &*int.borrow() {
return self.infix_operand_type_rules(binding, op, other, span);
}

if op.is_bitwise() && (other.is_bindable() || other.is_field()) {
if (op.is_modulo() || op.is_bitwise()) && (other.is_bindable() || other.is_field())
{
let other = other.follow_bindings();
let kind = op.kind;
// This will be an error if these types later resolve to a Field, or stay
// polymorphic as the bit size will be unknown. Delay this error until the function
// finishes resolving so we can still allow cases like `let x: u8 = 1 << 2;`.
self.push_delayed_type_check(Box::new(move || {
if other.is_field() {
Err(TypeCheckError::InvalidBitwiseOperationOnField { span })
if kind == BinaryOpKind::Modulo {
Err(TypeCheckError::FieldModulo { span })
} else {
Err(TypeCheckError::InvalidBitwiseOperationOnField { span })
}
} else if other.is_bindable() {
Err(TypeCheckError::AmbiguousBitWidth { span })
} else if kind.is_bit_shift() && other.is_signed() {
Expand Down Expand Up @@ -1054,6 +1058,9 @@ impl<'interner> TypeChecker<'interner> {
if op.is_bitwise() {
return Err(TypeCheckError::InvalidBitwiseOperationOnField { span });
}
if op.is_modulo() {
return Err(TypeCheckError::FieldModulo { span });
}
Ok(FieldElement)
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,10 @@ impl HirBinaryOp {
pub fn is_bit_shift(&self) -> bool {
self.kind.is_bit_shift()
}

pub fn is_modulo(&self) -> bool {
self.kind.is_modulo()
}
}

#[derive(Debug, Clone)]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "field_modulo"
type = "bin"
authors = [""]
compiler_version = "0.9.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@

fn main(x: Field) -> pub Field {
x % 2
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,7 @@
// Binary addition, multiplication, division, constant modulo
// x = 3, y = 4, z = 5
fn main(x : Field, y : Field, z : Field) -> pub Field {
//constant modulo
assert(x % 2 == 1);
//cast
assert(y as u1 == 0);

let a = x + x; // 3 + 3 = 6
Expand Down

0 comments on commit 7e68976

Please sign in to comment.