Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Sync from noir #7332

Merged
merged 2 commits into from
Jul 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
f35614a43cf8c5cfb244d9f6ffc9d63282a63e6d
70ebf607e566a95ff7eb2c7a0eee7c36465ba5b4
1 change: 1 addition & 0 deletions noir/noir-repo/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions noir/noir-repo/aztec_macros/src/utils/hir_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -230,6 +230,7 @@ pub fn inject_global(
file_id,
global.attributes.clone(),
false,
false,
);

// Add the statement to the scope so its path can be looked up later
Expand Down
3 changes: 3 additions & 0 deletions noir/noir-repo/compiler/noirc_evaluator/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,3 +21,6 @@ im.workspace = true
serde.workspace = true
tracing.workspace = true
chrono = "0.4.37"

[dev-dependencies]
proptest.workspace = true
Original file line number Diff line number Diff line change
Expand Up @@ -329,20 +329,16 @@ fn eval_constant_binary_op(
}
let result = function(lhs, rhs)?;
// Check for overflow
if result >= 2u128.pow(*bit_size) {
if result >= 1 << *bit_size {
return None;
}
result.into()
}
Type::Numeric(NumericType::Signed { bit_size }) => {
let function = operator.get_i128_function();

let lhs = truncate(lhs.try_into_u128()?, *bit_size);
let rhs = truncate(rhs.try_into_u128()?, *bit_size);
let l_pos = lhs < 2u128.pow(bit_size - 1);
let r_pos = rhs < 2u128.pow(bit_size - 1);
let lhs = if l_pos { lhs as i128 } else { -((2u128.pow(*bit_size) - lhs) as i128) };
let rhs = if r_pos { rhs as i128 } else { -((2u128.pow(*bit_size) - rhs) as i128) };
let lhs = try_convert_field_element_to_signed_integer(lhs, *bit_size)?;
let rhs = try_convert_field_element_to_signed_integer(rhs, *bit_size)?;
// The divisor is being truncated into the type of the operand, which can potentially
// lead to the rhs being zero.
// If the rhs of a division is zero, attempting to evaluate the division will cause a compiler panic.
Expand All @@ -354,12 +350,11 @@ fn eval_constant_binary_op(

let result = function(lhs, rhs)?;
// Check for overflow
if result >= 2i128.pow(*bit_size - 1) || result < -(2i128.pow(*bit_size - 1)) {
let two_pow_bit_size_minus_one = 1i128 << (*bit_size - 1);
if result >= two_pow_bit_size_minus_one || result < -two_pow_bit_size_minus_one {
return None;
}
let result =
if result >= 0 { result as u128 } else { (2i128.pow(*bit_size) + result) as u128 };
result.into()
convert_signed_integer_to_field_element(result, *bit_size)
}
_ => return None,
};
Expand All @@ -371,8 +366,37 @@ fn eval_constant_binary_op(
Some((value, operand_type))
}

/// Values in the range `[0, 2^(bit_size-1))` are interpreted as positive integers
///
/// Values in the range `[2^(bit_size-1), 2^bit_size)` are interpreted as negative integers.
fn try_convert_field_element_to_signed_integer(field: FieldElement, bit_size: u32) -> Option<i128> {
let unsigned_int = truncate(field.try_into_u128()?, bit_size);

let max_positive_value = 1 << (bit_size - 1);
let is_positive = unsigned_int < max_positive_value;

let signed_int = if is_positive {
unsigned_int as i128
} else {
let x = (1u128 << bit_size) - unsigned_int;
-(x as i128)
};

Some(signed_int)
}

fn convert_signed_integer_to_field_element(int: i128, bit_size: u32) -> FieldElement {
if int >= 0 {
FieldElement::from(int)
} else {
// We add an offset of `bit_size` bits to shift the negative values into the range [2^(bitsize-1), 2^bitsize)
let offset_int = (1i128 << bit_size) + int;
FieldElement::from(offset_int)
}
}

fn truncate(int: u128, bit_size: u32) -> u128 {
let max = 2u128.pow(bit_size);
let max = 1 << bit_size;
int % max
}

Expand Down Expand Up @@ -429,3 +453,24 @@ impl BinaryOp {
}
}
}

#[cfg(test)]
mod test {
use proptest::prelude::*;

use super::{
convert_signed_integer_to_field_element, try_convert_field_element_to_signed_integer,
};

proptest! {
#[test]
fn signed_int_roundtrip(int: i128, bit_size in 1u32..=64) {
let int = int % (1i128 << (bit_size - 1));

let int_as_field = convert_signed_integer_to_field_element(int, bit_size);
let recovered_int = try_convert_field_element_to_signed_integer(int_as_field, bit_size).unwrap();

prop_assert_eq!(int, recovered_int);
}
}
}
36 changes: 34 additions & 2 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,20 @@ impl NumericType {

/// Returns true if the given Field value is within the numeric limits
/// for the current NumericType.
pub(crate) fn value_is_within_limits(self, field: FieldElement) -> bool {
pub(crate) fn value_is_within_limits(self, field: FieldElement, negative: bool) -> bool {
match self {
NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => {
NumericType::Unsigned { bit_size } => {
if negative {
return false;
}
let max = 2u128.pow(bit_size) - 1;
field <= max.into()
}
NumericType::Signed { bit_size } => {
let max =
if negative { 2u128.pow(bit_size - 1) } else { 2u128.pow(bit_size - 1) - 1 };
field <= max.into()
}
NumericType::NativeField => true,
}
}
Expand Down Expand Up @@ -210,3 +218,27 @@ impl std::fmt::Display for NumericType {
}
}
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn test_u8_is_within_limits() {
let u8 = NumericType::Unsigned { bit_size: 8 };
assert!(!u8.value_is_within_limits(FieldElement::from(1_i128), true));
assert!(u8.value_is_within_limits(FieldElement::from(0_i128), false));
assert!(u8.value_is_within_limits(FieldElement::from(255_i128), false));
assert!(!u8.value_is_within_limits(FieldElement::from(256_i128), false));
}

#[test]
fn test_i8_is_within_limits() {
let i8 = NumericType::Signed { bit_size: 8 };
assert!(!i8.value_is_within_limits(FieldElement::from(129_i128), true));
assert!(i8.value_is_within_limits(FieldElement::from(128_i128), true));
assert!(i8.value_is_within_limits(FieldElement::from(0_i128), false));
assert!(i8.value_is_within_limits(FieldElement::from(127_i128), false));
assert!(!i8.value_is_within_limits(FieldElement::from(128_i128), false));
}
}
27 changes: 22 additions & 5 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -266,20 +266,37 @@ impl<'a> FunctionContext<'a> {
pub(super) fn checked_numeric_constant(
&mut self,
value: impl Into<FieldElement>,
negative: bool,
typ: Type,
) -> Result<ValueId, RuntimeError> {
let value = value.into();

if let Type::Numeric(typ) = typ {
if !typ.value_is_within_limits(value) {
if let Type::Numeric(numeric_type) = typ {
if !numeric_type.value_is_within_limits(value, negative) {
let call_stack = self.builder.get_call_stack();
return Err(RuntimeError::IntegerOutOfBounds { value, typ, call_stack });
return Err(RuntimeError::IntegerOutOfBounds {
value,
typ: numeric_type,
call_stack,
});
}

let value = if negative {
match numeric_type {
NumericType::NativeField => -value,
NumericType::Signed { bit_size } | NumericType::Unsigned { bit_size } => {
let base = 1_u128 << bit_size;
FieldElement::from(base) - value
}
}
} else {
value
};

Ok(self.builder.numeric_constant(value, typ))
} else {
panic!("Expected type for numeric constant to be a numeric type, found {typ}");
}

Ok(self.builder.numeric_constant(value, typ))
}

/// helper function which add instructions to the block computing the absolute value of the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -219,10 +219,10 @@ impl<'a> FunctionContext<'a> {
_ => unreachable!("ICE: unexpected slice literal type, got {}", array.typ),
})
}
ast::Literal::Integer(value, typ, location) => {
ast::Literal::Integer(value, negative, typ, location) => {
self.builder.set_location(*location);
let typ = Self::convert_non_tuple_type(typ);
self.checked_numeric_constant(*value, typ).map(Into::into)
self.checked_numeric_constant(*value, *negative, typ).map(Into::into)
}
ast::Literal::Bool(value) => {
// Don't need to call checked_numeric_constant here since `value` can only be true or false
Expand Down
33 changes: 24 additions & 9 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,8 +136,6 @@ pub struct Elaborator<'context> {
/// Each constraint in the `where` clause of the function currently being resolved.
trait_bounds: Vec<TraitConstraint>,

current_function: Option<FuncId>,

/// This is a stack of function contexts. Most of the time, for each function we
/// expect this to be of length one, containing each type variable and trait constraint
/// used in the function. This is also pushed to when a `comptime {}` block is used within
Expand Down Expand Up @@ -196,7 +194,6 @@ impl<'context> Elaborator<'context> {
crate_id,
resolving_ids: BTreeSet::new(),
trait_bounds: Vec::new(),
current_function: None,
function_context: vec![FunctionContext::default()],
current_trait_impl: None,
comptime_scopes: vec![HashMap::default()],
Expand Down Expand Up @@ -329,8 +326,6 @@ impl<'context> Elaborator<'context> {
FunctionBody::Resolving => return,
};

let old_function = std::mem::replace(&mut self.current_function, Some(id));

self.scopes.start_function();
let old_item = std::mem::replace(&mut self.current_item, Some(DependencyId::Function(id)));

Expand Down Expand Up @@ -407,7 +402,6 @@ impl<'context> Elaborator<'context> {

self.trait_bounds.clear();
self.interner.update_fn(id, hir_func);
self.current_function = old_function;
self.current_item = old_item;
}

Expand Down Expand Up @@ -615,8 +609,6 @@ impl<'context> Elaborator<'context> {
func_id: FuncId,
is_trait_function: bool,
) {
self.current_function = Some(func_id);

let in_contract = if self.self_type.is_some() {
// Without this, impl methods can accidentally be placed in contracts.
// See: https://github.com/noir-lang/noir/issues/3254
Expand Down Expand Up @@ -738,7 +730,6 @@ impl<'context> Elaborator<'context> {
};

self.interner.push_fn_meta(meta, func_id);
self.current_function = None;
self.scopes.end_function();
self.current_item = None;
}
Expand Down Expand Up @@ -1468,6 +1459,30 @@ impl<'context> Elaborator<'context> {
}
}

/// True if we're currently within a `comptime` block, function, or global
fn in_comptime_context(&self) -> bool {
// The first context is the global context, followed by the function-specific context.
// Any context after that is a `comptime {}` block's.
if self.function_context.len() > 2 {
return true;
}

match self.current_item {
Some(DependencyId::Function(id)) => self.interner.function_modifiers(&id).is_comptime,
Some(DependencyId::Global(id)) => self.interner.get_global_definition(id).comptime,
_ => false,
}
}

/// True if we're currently within a constrained function.
/// Defaults to `true` if the current function is unknown.
fn in_constrained_function(&self) -> bool {
self.current_item.map_or(true, |id| match id {
DependencyId::Function(id) => !self.interner.function_modifiers(&id).is_unconstrained,
_ => true,
})
}

/// Filters out comptime items from non-comptime items.
/// Returns a pair of (comptime items, non-comptime items)
fn filter_comptime_items(mut items: CollectedItems) -> (CollectedItems, CollectedItems) {
Expand Down
26 changes: 21 additions & 5 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use rustc_hash::FxHashSet as HashSet;
use crate::{
ast::ERROR_IDENT,
hir::{
comptime::Interpreter,
def_collector::dc_crate::CompilationError,
resolution::errors::ResolverError,
type_check::{Source, TypeCheckError},
Expand Down Expand Up @@ -285,19 +286,21 @@ impl<'context> Elaborator<'context> {
}

let location = Location::new(name.span(), self.file);
let name = name.0.contents;
let comptime = self.in_comptime_context();
let id =
self.interner.push_definition(name.0.contents.clone(), mutable, definition, location);
self.interner.push_definition(name.clone(), mutable, comptime, definition, location);
let ident = HirIdent::non_trait_method(id, location);
let resolver_meta =
ResolverMeta { num_times_used: 0, ident: ident.clone(), warn_if_unused };

let scope = self.scopes.get_mut_scope();
let old_value = scope.add_key_value(name.0.contents.clone(), resolver_meta);
let old_value = scope.add_key_value(name.clone(), resolver_meta);

if !allow_shadowing {
if let Some(old_value) = old_value {
self.push_err(ResolverError::DuplicateDefinition {
name: name.0.contents,
name,
first_span: old_value.ident.location.span,
second_span: location.span,
});
Expand Down Expand Up @@ -329,6 +332,7 @@ impl<'context> Elaborator<'context> {
name: Ident,
definition: DefinitionKind,
) -> HirIdent {
let comptime = self.in_comptime_context();
let scope = self.scopes.get_mut_scope();

// This check is necessary to maintain the same definition ids in the interner. Currently, each function uses a new resolver that has its own ScopeForest and thus global scope.
Expand All @@ -350,8 +354,8 @@ impl<'context> Elaborator<'context> {
(hir_ident, resolver_meta)
} else {
let location = Location::new(name.span(), self.file);
let id =
self.interner.push_definition(name.0.contents.clone(), false, definition, location);
let name = name.0.contents.clone();
let id = self.interner.push_definition(name, false, comptime, definition, location);
let ident = HirIdent::non_trait_method(id, location);
let resolver_meta =
ResolverMeta { num_times_used: 0, ident: ident.clone(), warn_if_unused: true };
Expand Down Expand Up @@ -400,12 +404,24 @@ impl<'context> Elaborator<'context> {
) -> (ExprId, Type) {
let span = variable.span;
let expr = self.resolve_variable(variable);
let definition_id = expr.id;

let id = self.interner.push_expr(HirExpression::Ident(expr.clone(), generics.clone()));

self.interner.push_expr_location(id, span, self.file);
let typ = self.type_check_variable(expr, id, generics);
self.interner.push_expr_type(id, typ.clone());

// Comptime variables must be replaced with their values
if let Some(definition) = self.interner.try_definition(definition_id) {
if definition.comptime && !self.in_comptime_context() {
let mut interpreter =
Interpreter::new(self.interner, &mut self.comptime_scopes, self.crate_id);
let value = interpreter.evaluate(id);
return self.inline_comptime_value(value, span);
}
}

(id, typ)
}

Expand Down
Loading
Loading