Skip to content
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 @@
a6e5526851f717c88799926796e36d6b6225be90
b3443c124b19a909bf9cb370b4e0ebc151bb6aa3
2 changes: 1 addition & 1 deletion barretenberg/acir_tests/bootstrap.sh
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ function build {
denoise "cd ../../noir/noir-repo/test_programs/execution_success && git clean -fdx"
cp -R ../../noir/noir-repo/test_programs/execution_success acir_tests
# Running these requires extra gluecode so they're skipped.
rm -rf acir_tests/{diamond_deps_0,workspace,workspace_default_member}
rm -rf acir_tests/{diamond_deps_0,workspace,workspace_default_member,regression_7323}
# TODO(https://github.com/AztecProtocol/barretenberg/issues/1108): problem regardless the proof system used
# TODO: Check if resolved. Move to .test_skip_patterns if not.
rm -rf acir_tests/regression_5045
Expand Down
2 changes: 1 addition & 1 deletion noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ pub(super) fn abi_type_from_hir_type(context: &Context, typ: &Type) -> AbiType {
| Type::Slice(_)
| Type::Function(_, _, _, _) => unreachable!("{typ} 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::Reference(..) => unreachable!("references cannot be used in the abi"),
}
}

Expand Down
71 changes: 62 additions & 9 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ use self::{
value::{Tree, Values},
};

use super::ir::basic_block::BasicBlockId;
use super::ir::dfg::GlobalsGraph;
use super::ir::instruction::ErrorType;
use super::ir::types::NumericType;
Expand Down Expand Up @@ -370,7 +371,7 @@ impl FunctionContext<'_> {
unary.location,
))
}
UnaryOp::MutableReference => {
UnaryOp::Reference { mutable: _ } => {
Ok(self.codegen_reference(&unary.rhs)?.map(|rhs| {
match rhs {
value::Value::Normal(value) => {
Expand Down Expand Up @@ -767,7 +768,15 @@ impl FunctionContext<'_> {
let tag = self.enum_tag(&variable);
let tag_type = self.builder.type_of_value(tag).unwrap_numeric();

let end_block = self.builder.insert_block();
let make_end_block = |this: &mut Self| -> (BasicBlockId, Values) {
let block = this.builder.insert_block();
let results = Self::map_type(&match_expr.typ, |typ| {
this.builder.add_block_parameter(block, typ).into()
});
(block, results)
};

let (end_block, end_results) = make_end_block(self);

// Optimization: if there is no default case we can jump directly to the last case
// when finished with the previous case instead of using a jmpif with an unreachable
Expand All @@ -778,6 +787,8 @@ impl FunctionContext<'_> {
match_expr.cases.len() - 1
};

let mut blocks_to_merge = Vec::with_capacity(last_case);

for i in 0..last_case {
let case = &match_expr.cases[i];
let variant_tag = self.variant_index_value(&case.constructor, tag_type)?;
Expand All @@ -790,28 +801,70 @@ impl FunctionContext<'_> {
self.builder.switch_to_block(case_block);
self.bind_case_arguments(variable.clone(), case);
let results = self.codegen_expression(&case.branch)?.into_value_list(self);
self.builder.terminate_with_jmp(end_block, results);

// Each branch will jump to a different end block for now. We have to merge them all
// later since SSA doesn't support more than two blocks jumping to the same end block.
let local_end_block = make_end_block(self);
self.builder.terminate_with_jmp(local_end_block.0, results);
blocks_to_merge.push(local_end_block);

self.builder.switch_to_block(else_block);
}

let (last_local_end_block, last_results) = make_end_block(self);
blocks_to_merge.push((last_local_end_block, last_results));

if let Some(branch) = &match_expr.default_case {
let results = self.codegen_expression(branch)?.into_value_list(self);
self.builder.terminate_with_jmp(end_block, results);
self.builder.terminate_with_jmp(last_local_end_block, results);
} else {
// If there is no default case, assume we saved the last case from the
// last_case optimization above
let case = match_expr.cases.last().unwrap();
self.bind_case_arguments(variable, case);
let results = self.codegen_expression(&case.branch)?.into_value_list(self);
self.builder.terminate_with_jmp(end_block, results);
self.builder.terminate_with_jmp(last_local_end_block, results);
}

// Merge blocks as last-in first-out:
//
// local_end_block0-----------------------------------------\
// end block
// /
// local_end_block1---------------------\ /
// new merge block2-/
// local_end_block2--\ /
// new merge block1-/
// local_end_block3--/
//
// This is necessary since SSA panics during flattening if we immediately
// try to jump directly to end block instead: https://github.com/noir-lang/noir/issues/7323.
//
// It'd also be more efficient to merge them tournament-bracket style but that
// also leads to panics during flattening for similar reasons.
while let Some((block, results)) = blocks_to_merge.pop() {
self.builder.switch_to_block(block);

if let Some((block2, results2)) = blocks_to_merge.pop() {
// Merge two blocks in the queue together
let (new_merge, new_merge_results) = make_end_block(self);
blocks_to_merge.push((new_merge, new_merge_results));

let results = results.into_value_list(self);
self.builder.terminate_with_jmp(new_merge, results);

self.builder.switch_to_block(block2);
let results2 = results2.into_value_list(self);
self.builder.terminate_with_jmp(new_merge, results2);
} else {
// Finally done, jump to the end
let results = results.into_value_list(self);
self.builder.terminate_with_jmp(end_block, results);
}
}

self.builder.switch_to_block(end_block);
let result = Self::map_type(&match_expr.typ, |typ| {
self.builder.add_block_parameter(end_block, typ).into()
});
Ok(result)
Ok(end_results)
}

fn variant_index_value(
Expand Down
7 changes: 5 additions & 2 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,7 +383,9 @@ impl BinaryOpKind {
pub enum UnaryOp {
Minus,
Not,
MutableReference,
Reference {
mutable: bool,
},

/// If implicitly_added is true, this operation was implicitly added by the compiler for a
/// field dereference. The compiler may undo some of these implicitly added dereferences if
Expand Down Expand Up @@ -732,7 +734,8 @@ impl Display for UnaryOp {
match self {
UnaryOp::Minus => write!(f, "-"),
UnaryOp::Not => write!(f, "!"),
UnaryOp::MutableReference => write!(f, "&mut"),
UnaryOp::Reference { mutable } if *mutable => write!(f, "&mut"),
UnaryOp::Reference { .. } => write!(f, "&"),
UnaryOp::Dereference { .. } => write!(f, "*"),
}
}
Expand Down
11 changes: 6 additions & 5 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ pub enum UnresolvedTypeData {
/// A Trait as return type or parameter of function, including its generics
TraitAsType(Path, GenericTypeArgs),

/// &mut T
MutableReference(Box<UnresolvedType>),
/// &T and &mut T
Reference(Box<UnresolvedType>, /*mutable*/ bool),

// Note: Tuples have no visibility, instead each of their elements may have one.
Tuple(Vec<UnresolvedType>),
Expand Down Expand Up @@ -311,7 +311,8 @@ impl std::fmt::Display for UnresolvedTypeData {
other => write!(f, "fn[{other}]({args}) -> {ret}"),
}
}
MutableReference(element) => write!(f, "&mut {element}"),
Reference(element, false) => write!(f, "&{element}"),
Reference(element, true) => write!(f, "&mut {element}"),
Quoted(quoted) => write!(f, "{}", quoted),
Unit => write!(f, "()"),
Error => write!(f, "error"),
Expand Down Expand Up @@ -346,7 +347,7 @@ impl std::fmt::Display for UnresolvedTypeExpression {
impl UnresolvedType {
pub fn is_synthesized(&self) -> bool {
match &self.typ {
UnresolvedTypeData::MutableReference(ty) => ty.is_synthesized(),
UnresolvedTypeData::Reference(ty, _) => ty.is_synthesized(),
UnresolvedTypeData::Named(_, _, synthesized) => *synthesized,
_ => false,
}
Expand Down Expand Up @@ -424,7 +425,7 @@ impl UnresolvedTypeData {
path_is_wildcard || an_arg_is_unresolved
}
UnresolvedTypeData::TraitAsType(_path, args) => args.contains_unspecified(),
UnresolvedTypeData::MutableReference(typ) => typ.contains_unspecified(),
UnresolvedTypeData::Reference(typ, _) => typ.contains_unspecified(),
UnresolvedTypeData::Tuple(args) => args.iter().any(|arg| arg.contains_unspecified()),
UnresolvedTypeData::Function(args, ret, env, _unconstrained) => {
let args_contains_unspecified = args.iter().any(|arg| arg.contains_unspecified());
Expand Down
6 changes: 3 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -393,7 +393,7 @@ pub trait Visitor {
true
}

fn visit_mutable_reference_type(&mut self, _: &UnresolvedType, _: Span) -> bool {
fn visit_reference_type(&mut self, _: &UnresolvedType, _mutable: bool, _: Span) -> bool {
true
}

Expand Down Expand Up @@ -1382,8 +1382,8 @@ impl UnresolvedType {
generic_type_args.accept(visitor);
}
}
UnresolvedTypeData::MutableReference(unresolved_type) => {
if visitor.visit_mutable_reference_type(unresolved_type, self.location.span) {
UnresolvedTypeData::Reference(unresolved_type, mutable) => {
if visitor.visit_reference_type(unresolved_type, *mutable, self.location.span) {
unresolved_type.accept(visitor);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -938,7 +938,7 @@ impl<'elab, 'ctx> MatchCompiler<'elab, 'ctx> {
| Type::NamedGeneric(_, _)
| Type::CheckedCast { .. }
| Type::Function(_, _, _, _)
| Type::MutableReference(_)
| Type::Reference(..)
| Type::Forall(_, _)
| Type::Constant(_, _)
| Type::Quoted(_)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ use crate::{
token::{FmtStrFragment, Tokens},
};

use super::{Elaborator, LambdaContext, UnsafeBlockStatus};
use super::{Elaborator, LambdaContext, UnsafeBlockStatus, UnstableFeature};

impl Elaborator<'_> {
pub(crate) fn elaborate_expression(&mut self, expr: Expression) -> (ExprId, Type) {
Expand Down Expand Up @@ -344,8 +344,12 @@ impl Elaborator<'_> {

let operator = prefix.operator;

if let UnaryOp::MutableReference = operator {
self.check_can_mutate(rhs, rhs_location);
if let UnaryOp::Reference { mutable } = operator {
if mutable {
self.check_can_mutate(rhs, rhs_location);
} else {
self.use_unstable_feature(UnstableFeature::Ownership, location);
}
}

let expr =
Expand Down
10 changes: 5 additions & 5 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -588,7 +588,7 @@ impl<'context> Elaborator<'context> {
for (mut constraint, expr_id, select_impl) in context.trait_constraints {
let location = self.interner.expr_location(&expr_id);

if matches!(&constraint.typ, Type::MutableReference(_)) {
if matches!(&constraint.typ, Type::Reference(..)) {
let (_, dereferenced_typ) =
self.insert_auto_dereferences(expr_id, constraint.typ.clone());
constraint.typ = dereferenced_typ;
Expand Down Expand Up @@ -1078,7 +1078,7 @@ impl<'context> Elaborator<'context> {
self.mark_type_as_used(from);
self.mark_type_as_used(to);
}
Type::MutableReference(typ) => {
Type::Reference(typ, _) => {
self.mark_type_as_used(typ);
}
Type::InfixExpr(left, _op, right, _) => {
Expand Down Expand Up @@ -1461,8 +1461,8 @@ impl<'context> Elaborator<'context> {
self.self_type = Some(self_type.clone());
let self_type_location = trait_impl.object_type.location;

if matches!(self_type, Type::MutableReference(_)) {
self.push_err(DefCollectorErrorKind::MutableReferenceInTraitImpl {
if matches!(self_type, Type::Reference(..)) {
self.push_err(DefCollectorErrorKind::ReferenceInTraitImpl {
location: self_type_location,
});
}
Expand Down Expand Up @@ -1755,7 +1755,7 @@ impl<'context> Elaborator<'context> {
);
self.check_type_is_not_more_private_then_item(name, visibility, env, location);
}
Type::MutableReference(typ) | Type::Array(_, typ) | Type::Slice(typ) => {
Type::Reference(typ, _) | Type::Array(_, typ) | Type::Slice(typ) => {
self.check_type_is_not_more_private_then_item(name, visibility, typ, location);
}
Type::InfixExpr(left, _op, right, _) => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,14 @@ use std::str::FromStr;
#[derive(Copy, Clone, Debug, PartialEq, Eq)]
pub enum UnstableFeature {
Enums,
ArrayOwnership,
Ownership,
}

impl std::fmt::Display for UnstableFeature {
fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
match self {
Self::Enums => write!(f, "enums"),
Self::ArrayOwnership => write!(f, "array-ownership"),
Self::Ownership => write!(f, "ownership"),
}
}
}
Expand All @@ -21,7 +21,7 @@ impl FromStr for UnstableFeature {
fn from_str(s: &str) -> Result<Self, Self::Err> {
match s {
"enums" => Ok(Self::Enums),
"array-ownership" => Ok(Self::ArrayOwnership),
"ownership" => Ok(Self::Ownership),
other => Err(format!("Unknown unstable feature '{other}'")),
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -437,8 +437,8 @@ impl Elaborator<'_> {
let (mut lvalue, mut lvalue_type, mut mutable) = self.elaborate_lvalue(*array);

// Before we check that the lvalue is an array, try to dereference it as many times
// as needed to unwrap any &mut wrappers.
while let Type::MutableReference(element) = lvalue_type.follow_bindings() {
// as needed to unwrap any `&` or `&mut` wrappers.
while let Type::Reference(element, _) = lvalue_type.follow_bindings() {
let element_type = element.as_ref().clone();
lvalue =
HirLValue::Dereference { lvalue: Box::new(lvalue), element_type, location };
Expand Down Expand Up @@ -482,7 +482,9 @@ impl Elaborator<'_> {
let lvalue = Box::new(lvalue);

let element_type = Type::type_variable(self.interner.next_type_variable_id());
let expected_type = Type::MutableReference(Box::new(element_type.clone()));

// Always expect a mutable reference here since we're storing to it
let expected_type = Type::Reference(Box::new(element_type.clone()), true);

self.unify(&reference_type, &expected_type, || TypeCheckError::TypeMismatch {
expected_typ: expected_type.to_string(),
Expand Down Expand Up @@ -539,9 +541,8 @@ impl Elaborator<'_> {
}
}
}
// If the lhs is a mutable reference we automatically transform
// lhs.field into (*lhs).field
Type::MutableReference(element) => {
// If the lhs is a reference we automatically transform `lhs.field` into `(*lhs).field`
Type::Reference(element, mutable) => {
if let Some(mut dereference_lhs) = dereference_lhs {
dereference_lhs(self, lhs_type.clone(), element.as_ref().clone());
return self.check_field_access(
Expand All @@ -553,7 +554,7 @@ impl Elaborator<'_> {
} else {
let (element, index) =
self.check_field_access(element, field_name, location, dereference_lhs)?;
return Some((Type::MutableReference(Box::new(element)), index));
return Some((Type::Reference(Box::new(element), *mutable), index));
}
}
_ => (),
Expand Down
Loading