Skip to content

Commit

Permalink
feat: Sync from noir (#7332)
Browse files Browse the repository at this point in the history
Automated pull of development from the
[noir](https://github.com/noir-lang/noir) programming language, a
dependency of Aztec.
BEGIN_COMMIT_OVERRIDE
fix(docs): Fix broken docs link to gihtub
(noir-lang/noir#5398)
chore(docs): Remove persona boxes from the landing page
(noir-lang/noir#5400)
feat: Sync from aztec-packages
(noir-lang/noir#5401)
chore: refactor conversion between `FieldElement` and signed integers
(noir-lang/noir#5397)
fix: ICE when using a comptime let variable in runtime code
(noir-lang/noir#5391)
fix: Don't panic when using undefined variables in the interpreter
(noir-lang/noir#5381)
feat: lsp "find all references"
(noir-lang/noir#5395)
fix: correctly detect signed/unsigned integer overflows/underflows
(noir-lang/noir#5375)
fix: go to definition from aliased use
(noir-lang/noir#5396)
END_COMMIT_OVERRIDE
  • Loading branch information
AztecBot authored Jul 4, 2024
1 parent b7c6593 commit 10076d9
Show file tree
Hide file tree
Showing 57 changed files with 510 additions and 169 deletions.
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

0 comments on commit 10076d9

Please sign in to comment.