From 9b3eed9c4a6923dc55f2593ab9d27abff109cd5e Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Wed, 5 Mar 2025 15:18:16 -0600 Subject: [PATCH 1/4] Enable ownership syntax --- compiler/noirc_driver/src/abi_gen.rs | 2 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 2 +- compiler/noirc_frontend/src/ast/expression.rs | 7 +- compiler/noirc_frontend/src/ast/mod.rs | 11 +-- compiler/noirc_frontend/src/ast/visitor.rs | 8 +- .../noirc_frontend/src/elaborator/enums.rs | 2 +- .../src/elaborator/expressions.rs | 10 ++- compiler/noirc_frontend/src/elaborator/mod.rs | 15 ++-- .../noirc_frontend/src/elaborator/options.rs | 6 +- .../src/elaborator/statements.rs | 15 ++-- .../noirc_frontend/src/elaborator/types.rs | 57 ++++++++---- .../src/hir/comptime/display.rs | 12 ++- .../src/hir/comptime/hir_to_display_ast.rs | 4 +- .../src/hir/comptime/interpreter.rs | 22 ++--- .../src/hir/comptime/interpreter/builtin.rs | 14 +-- .../noirc_frontend/src/hir/comptime/value.rs | 8 +- .../src/hir/def_collector/errors.rs | 10 +-- compiler/noirc_frontend/src/hir_def/types.rs | 87 ++++++++++++++----- compiler/noirc_frontend/src/lexer/lexer.rs | 2 + compiler/noirc_frontend/src/lexer/token.rs | 6 ++ .../src/monomorphization/mod.rs | 13 +-- compiler/noirc_frontend/src/node_interner.rs | 12 +-- compiler/noirc_frontend/src/parser/parser.rs | 1 + .../src/parser/parser/expression.rs | 14 +-- .../src/parser/parser/function.rs | 2 +- .../src/parser/parser/pattern.rs | 10 +-- .../noirc_frontend/src/parser/parser/types.rs | 28 ++++-- compiler/noirc_frontend/src/tests.rs | 32 +++++++ compiler/noirc_printable_type/src/lib.rs | 8 +- tooling/lsp/src/requests/completion.rs | 2 +- .../requests/completion/completion_items.rs | 11 ++- .../lsp/src/requests/hover/from_reference.rs | 4 +- tooling/lsp/src/requests/inlay_hint.rs | 6 +- tooling/lsp/src/requests/signature_help.rs | 2 +- .../src/trait_impl_method_stub_generator.rs | 6 +- tooling/lsp/src/with_file.rs | 4 +- tooling/nargo_fmt/src/formatter/expression.rs | 2 +- tooling/nargo_fmt/src/formatter/types.rs | 8 +- tooling/noirc_abi/src/printable_type.rs | 2 +- 39 files changed, 312 insertions(+), 155 deletions(-) diff --git a/compiler/noirc_driver/src/abi_gen.rs b/compiler/noirc_driver/src/abi_gen.rs index 3bbe2181798..25d79bf16a2 100644 --- a/compiler/noirc_driver/src/abi_gen.rs +++ b/compiler/noirc_driver/src/abi_gen.rs @@ -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"), } } diff --git a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index a954ac3ab93..33d41f29168 100644 --- a/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -370,7 +370,7 @@ impl FunctionContext<'_> { unary.location, )) } - UnaryOp::MutableReference => { + UnaryOp::Reference { mutable: _ } => { Ok(self.codegen_reference(&unary.rhs)?.map(|rhs| { match rhs { value::Value::Normal(value) => { diff --git a/compiler/noirc_frontend/src/ast/expression.rs b/compiler/noirc_frontend/src/ast/expression.rs index 398e5267695..cc4cebafbbe 100644 --- a/compiler/noirc_frontend/src/ast/expression.rs +++ b/compiler/noirc_frontend/src/ast/expression.rs @@ -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 @@ -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, "*"), } } diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index 8e74ce8877e..d4c48466b85 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -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), + /// &T and &mut T + Reference(Box, /*mutable*/ bool), // Note: Tuples have no visibility, instead each of their elements may have one. Tuple(Vec), @@ -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"), @@ -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, } @@ -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()); diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index b2142f26655..00a3200d632 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -393,6 +393,10 @@ pub trait Visitor { true } + fn visit_reference_type(&mut self, _: &UnresolvedType, _mutable: bool, _: Span) -> bool { + true + } + fn visit_mutable_reference_type(&mut self, _: &UnresolvedType, _: Span) -> bool { true } @@ -1382,8 +1386,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); } } diff --git a/compiler/noirc_frontend/src/elaborator/enums.rs b/compiler/noirc_frontend/src/elaborator/enums.rs index c0bc86b51b0..b5933f9dc37 100644 --- a/compiler/noirc_frontend/src/elaborator/enums.rs +++ b/compiler/noirc_frontend/src/elaborator/enums.rs @@ -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(_) diff --git a/compiler/noirc_frontend/src/elaborator/expressions.rs b/compiler/noirc_frontend/src/elaborator/expressions.rs index 2ac779d3e77..e5df79522c6 100644 --- a/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -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) { @@ -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 = diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 23a9ec5246e..018e62dd128 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -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; @@ -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, _) => { @@ -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, }); } @@ -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, _) => { @@ -2168,6 +2168,11 @@ impl<'context> Elaborator<'context> { } } + /// Return true if the given unstable feature is enabled + fn unstable_feature_enabled(&self, feature: UnstableFeature) -> bool { + self.options.enabled_unstable_features.contains(&feature) + } + /// Run the given function using the resolver and return true if any errors (not warnings) /// occurred while running it. pub fn errors_occurred_in(&mut self, f: impl FnOnce(&mut Self) -> T) -> (bool, T) { diff --git a/compiler/noirc_frontend/src/elaborator/options.rs b/compiler/noirc_frontend/src/elaborator/options.rs index 58bb5e73a61..0d72d2955ba 100644 --- a/compiler/noirc_frontend/src/elaborator/options.rs +++ b/compiler/noirc_frontend/src/elaborator/options.rs @@ -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"), } } } @@ -21,7 +21,7 @@ impl FromStr for UnstableFeature { fn from_str(s: &str) -> Result { match s { "enums" => Ok(Self::Enums), - "array-ownership" => Ok(Self::ArrayOwnership), + "ownership" => Ok(Self::Ownership), other => Err(format!("Unknown unstable feature '{other}'")), } } diff --git a/compiler/noirc_frontend/src/elaborator/statements.rs b/compiler/noirc_frontend/src/elaborator/statements.rs index f00b2a87b1e..b9810c7d222 100644 --- a/compiler/noirc_frontend/src/elaborator/statements.rs +++ b/compiler/noirc_frontend/src/elaborator/statements.rs @@ -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 }; @@ -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(), @@ -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( @@ -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)); } } _ => (), diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 9b717f9fca3..69baa390140 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -12,6 +12,7 @@ use crate::{ Signedness, UnaryOp, UnresolvedGeneric, UnresolvedGenerics, UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression, WILDCARD_TYPE, }, + elaborator::UnstableFeature, hir::{ def_collector::dc_crate::CompilationError, def_map::{ModuleDefId, fully_qualified_module_path}, @@ -141,8 +142,11 @@ impl Elaborator<'_> { } } } - MutableReference(element) => { - Type::MutableReference(Box::new(self.resolve_type_inner(*element, kind))) + Reference(element, mutable) => { + if !mutable { + self.use_unstable_feature(UnstableFeature::Ownership, location); + } + Type::Reference(Box::new(self.resolve_type_inner(*element, kind)), mutable) } Parenthesized(typ) => self.resolve_type_inner(*typ, kind), Resolved(id) => self.interner.get_quoted_type(id).clone(), @@ -845,7 +849,7 @@ impl Elaborator<'_> { /// Insert as many dereference operations as necessary to automatically dereference a method /// call object to its base value type T. pub(super) fn insert_auto_dereferences(&mut self, object: ExprId, typ: Type) -> (ExprId, Type) { - if let Type::MutableReference(element) = typ.follow_bindings() { + if let Type::Reference(element, _mut) = typ.follow_bindings() { let location = self.interner.id_location(object); let object = self.interner.push_expr(HirExpression::Prefix(HirPrefixExpression { @@ -1304,17 +1308,34 @@ impl Elaborator<'_> { _ => Ok((rhs_type.clone(), true)), } } - crate::ast::UnaryOp::MutableReference => { - Ok((Type::MutableReference(Box::new(rhs_type.follow_bindings())), false)) + crate::ast::UnaryOp::Reference { mutable } => { + let typ = Type::Reference(Box::new(rhs_type.follow_bindings()), *mutable); + Ok((typ, false)) } crate::ast::UnaryOp::Dereference { implicitly_added: _ } => { let element_type = self.interner.next_type_variable(); - let expected = Type::MutableReference(Box::new(element_type.clone())); - self.unify(rhs_type, &expected, || TypeCheckError::TypeMismatch { - expr_typ: rhs_type.to_string(), - expected_typ: expected.to_string(), - expr_location: location, - }); + let make_expected = + |mutable| Type::Reference(Box::new(element_type.clone()), mutable); + + let immutable = make_expected(false); + let ownership_enabled = self.unstable_feature_enabled(UnstableFeature::Ownership); + + // Both `&mut T` and `&T` should coerce to an expected `&T`. + if !rhs_type.try_reference_coercion(&immutable) { + self.unify(rhs_type, &immutable, || { + let expected_typ = if ownership_enabled { + immutable.to_string() + } else { + make_expected(true).to_string() + }; + + TypeCheckError::TypeMismatch { + expr_typ: rhs_type.to_string(), + expected_typ, + expr_location: location, + } + }); + } Ok((element_type, false)) } } @@ -1437,9 +1458,9 @@ impl Elaborator<'_> { Type::NamedGeneric(_, _) => { self.lookup_method_in_trait_constraints(object_type, method_name, location) } - // Mutable references to another type should resolve to methods of their element type. + // References to another type should resolve to methods of their element type. // This may be a data type or a primitive type. - Type::MutableReference(element) => { + Type::Reference(element, _mutable) => { self.lookup_method(&element, method_name, location, check_self_param) } @@ -1836,12 +1857,12 @@ impl Elaborator<'_> { if let Some(expected_object_type) = expected_object_type { let actual_type = object_type.follow_bindings(); - if matches!(expected_object_type.follow_bindings(), Type::MutableReference(_)) { - if !matches!(actual_type, Type::MutableReference(_)) { + if let Type::Reference(_, mutable) = expected_object_type.follow_bindings() { + if !matches!(actual_type, Type::Reference(..)) { let location = self.interner.id_location(*object); self.check_can_mutate(*object, location); - let new_type = Type::MutableReference(Box::new(actual_type)); + let new_type = Type::Reference(Box::new(actual_type), mutable); *object_type = new_type.clone(); // First try to remove a dereference operator that may have been implicitly @@ -1852,7 +1873,7 @@ impl Elaborator<'_> { *object = new_object.unwrap_or_else(|| { let new_object = self.interner.push_expr(HirExpression::Prefix(HirPrefixExpression { - operator: UnaryOp::MutableReference, + operator: UnaryOp::Reference { mutable }, rhs: *object, trait_method_id: None, })); @@ -1863,7 +1884,7 @@ impl Elaborator<'_> { } // Otherwise if the object type is a mutable reference and the method is not, insert as // many dereferences as needed. - } else if matches!(actual_type, Type::MutableReference(_)) { + } else if matches!(actual_type, Type::Reference(..)) { let (new_object, new_type) = self.insert_auto_dereferences(*object, actual_type); *object_type = new_type; *object = new_object; diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index c0283d9701b..1ea64691e94 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -254,6 +254,7 @@ impl<'interner> TokenPrettyPrinter<'interner> { | Token::Slash | Token::Percent | Token::Ampersand + | Token::SliceStart | Token::ShiftLeft | Token::ShiftRight => { self.last_was_op = true; @@ -381,7 +382,13 @@ impl Display for ValuePrinter<'_, '_> { other => write!(f, "{other}(args)"), } } - Value::Pointer(value, _) => write!(f, "&mut {}", value.borrow().display(self.interner)), + Value::Pointer(value, _, mutable) => { + if *mutable { + write!(f, "&mut {}", value.borrow().display(self.interner)) + } else { + write!(f, "&{}", value.borrow().display(self.interner)) + } + } Value::Array(values, _) => { let values = vecmap(values, |value| value.display(self.interner).to_string()); write!(f, "[{}]", values.join(", ")) @@ -856,8 +863,9 @@ fn remove_interned_in_unresolved_type_data( remove_interned_in_generic_type_args(interner, generic_type_args), ) } - UnresolvedTypeData::MutableReference(typ) => UnresolvedTypeData::MutableReference( + UnresolvedTypeData::Reference(typ, mutable) => UnresolvedTypeData::Reference( Box::new(remove_interned_in_unresolved_type(interner, *typ)), + mutable, ), UnresolvedTypeData::Tuple(types) => UnresolvedTypeData::Tuple(vecmap(types, |typ| { remove_interned_in_unresolved_type(interner, typ) diff --git a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs index 4a4835c8bf7..cb030e1a80d 100644 --- a/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs +++ b/compiler/noirc_frontend/src/hir/comptime/hir_to_display_ast.rs @@ -465,9 +465,9 @@ impl Type { let env = Box::new(env.to_display_ast()); UnresolvedTypeData::Function(args, ret, env, *unconstrained) } - Type::MutableReference(element) => { + Type::Reference(element, mutable) => { let element = Box::new(element.to_display_ast()); - UnresolvedTypeData::MutableReference(element) + UnresolvedTypeData::Reference(element, *mutable) } // Type::Forall is only for generic functions which don't store a type // in their Ast so they don't need to call to_display_ast for their Forall type. diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 5c87e70949a..d5b830d257c 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -393,7 +393,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { } HirPattern::Mutable(pattern, _) => { // Create a mutable reference to store to - let argument = Value::Pointer(Shared::new(argument), true); + let argument = Value::Pointer(Shared::new(argument), true, true); self.define_pattern(pattern, typ, argument, location) } HirPattern::Tuple(pattern_fields, _) => { @@ -471,7 +471,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { for scope in self.elaborator.interner.comptime_scopes.iter_mut().rev() { if let Entry::Occupied(mut entry) = scope.entry(id) { match entry.get() { - Value::Pointer(reference, true) => { + Value::Pointer(reference, true, _) => { *reference.borrow_mut() = argument; } _ => { @@ -507,7 +507,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { /// This will automatically dereference a mutable variable if used. pub fn evaluate(&mut self, id: ExprId) -> IResult { match self.evaluate_no_dereference(id)? { - Value::Pointer(elem, true) => Ok(elem.borrow().clone()), + Value::Pointer(elem, true, _) => Ok(elem.borrow().clone()), other => Ok(other), } } @@ -848,7 +848,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { fn evaluate_prefix(&mut self, prefix: HirPrefixExpression, id: ExprId) -> IResult { let rhs = match prefix.operator { - UnaryOp::MutableReference => self.evaluate_no_dereference(prefix.rhs)?, + UnaryOp::Reference { .. } => self.evaluate_no_dereference(prefix.rhs)?, _ => self.evaluate(prefix.rhs)?, }; @@ -899,17 +899,17 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { Err(InterpreterError::InvalidValueForUnary { typ, location, operator: "not" }) } }, - UnaryOp::MutableReference => { + UnaryOp::Reference { mutable } => { // If this is a mutable variable (auto_deref = true), turn this into an explicit // mutable reference just by switching the value of `auto_deref`. Otherwise, wrap // the value in a fresh reference. match rhs { - Value::Pointer(elem, true) => Ok(Value::Pointer(elem, false)), - other => Ok(Value::Pointer(Shared::new(other), false)), + Value::Pointer(elem, true, _) => Ok(Value::Pointer(elem, false, mutable)), + other => Ok(Value::Pointer(Shared::new(other), false, mutable)), } } UnaryOp::Dereference { implicitly_added: _ } => match rhs { - Value::Pointer(element, _) => Ok(element.borrow().clone()), + Value::Pointer(element, _, _) => Ok(element.borrow().clone()), value => { let location = self.elaborator.interner.expr_location(&id); let typ = value.get_type().into_owned(); @@ -1613,7 +1613,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { HirLValue::Ident(ident, typ) => self.mutate(ident.id, rhs, ident.location), HirLValue::Dereference { lvalue, element_type: _, location } => { match self.evaluate_lvalue(&lvalue)? { - Value::Pointer(value, _) => { + Value::Pointer(value, _, _) => { *value.borrow_mut() = rhs; Ok(()) } @@ -1669,12 +1669,12 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { fn evaluate_lvalue(&mut self, lvalue: &HirLValue) -> IResult { match lvalue { HirLValue::Ident(ident, _) => match self.lookup(ident)? { - Value::Pointer(elem, true) => Ok(elem.borrow().clone()), + Value::Pointer(elem, true, _) => Ok(elem.borrow().clone()), other => Ok(other), }, HirLValue::Dereference { lvalue, element_type, location } => { match self.evaluate_lvalue(lvalue)? { - Value::Pointer(value, _) => Ok(value.borrow().clone()), + Value::Pointer(value, _, _) => Ok(value.borrow().clone()), value => { let typ = value.get_type().into_owned(); Err(InterpreterError::NonPointerDereferenced { typ, location: *location }) diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 34a5535f63c..da0d7ba8181 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -1041,7 +1041,7 @@ fn type_as_mutable_reference( location: Location, ) -> IResult { type_as(arguments, return_type, location, |typ| { - if let Type::MutableReference(typ) = typ { Some(Value::Type(*typ)) } else { None } + if let Type::Reference(typ, true) = typ { Some(Value::Type(*typ)) } else { None } }) } @@ -1321,7 +1321,7 @@ fn unresolved_type_as_mutable_reference( location: Location, ) -> IResult { unresolved_type_as(interner, arguments, return_type, location, |typ| { - if let UnresolvedTypeData::MutableReference(typ) = typ { + if let UnresolvedTypeData::Reference(typ, true) = typ { Some(Value::UnresolvedType(typ.typ)) } else { None @@ -1483,9 +1483,9 @@ fn zeroed(return_type: Type, location: Location) -> Value { // Using Value::Zeroed here is probably safer than using FuncId::dummy_id() or similar Value::Zeroed(typ) } - Type::MutableReference(element) => { + Type::Reference(element, mutable) => { let element = zeroed(*element, location); - Value::Pointer(Shared::new(element), false) + Value::Pointer(Shared::new(element), false, mutable) } // Optimistically assume we can resolve this type later or that the value is unused Type::TypeVariable(_) @@ -2177,7 +2177,11 @@ fn expr_as_unary_op( let unary_op_value: u128 = match prefix_expr.operator { UnaryOp::Minus => 0, UnaryOp::Not => 1, - UnaryOp::MutableReference => 2, + UnaryOp::Reference { mutable: true } => 2, + UnaryOp::Reference { mutable: false } => { + // `&` alone is experimental and currently hidden from the comptime API + return None; + } UnaryOp::Dereference { .. } => 3, }; diff --git a/compiler/noirc_frontend/src/hir/comptime/value.rs b/compiler/noirc_frontend/src/hir/comptime/value.rs index 8d07669497f..ebaaf27f4f6 100644 --- a/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -61,7 +61,7 @@ pub enum Value { Tuple(Vec), Struct(HashMap, Value>, Type), Enum(/*tag*/ usize, /*args*/ Vec, Type), - Pointer(Shared, /* auto_deref */ bool), + Pointer(Shared, /* auto_deref */ bool, /* mutable */ bool), Array(Vector, Type), Slice(Vector, Type), Quoted(Rc>), @@ -151,12 +151,12 @@ impl Value { Value::Slice(_, typ) => return Cow::Borrowed(typ), Value::Quoted(_) => Type::Quoted(QuotedType::Quoted), Value::StructDefinition(_) => Type::Quoted(QuotedType::StructDefinition), - Value::Pointer(element, auto_deref) => { + Value::Pointer(element, auto_deref, mutable) => { if *auto_deref { element.borrow().get_type().into_owned() } else { let element = element.borrow().get_type().into_owned(); - Type::MutableReference(Box::new(element)) + Type::Reference(Box::new(element), *mutable) } } Value::TraitConstraint { .. } => Type::Quoted(QuotedType::TraitConstraint), @@ -452,7 +452,7 @@ impl Value { Value::TypedExpr(TypedExpr::ExprId(expr_id)) => interner.expression(&expr_id), // Only convert pointers with auto_deref = true. These are mutable variables // and we don't need to wrap them in `&mut`. - Value::Pointer(element, true) => { + Value::Pointer(element, true, _) => { return element.unwrap_or_clone().into_hir_expression(interner, location); } Value::Closure(closure) => HirExpression::Lambda(closure.lambda.clone()), diff --git a/compiler/noirc_frontend/src/hir/def_collector/errors.rs b/compiler/noirc_frontend/src/hir/def_collector/errors.rs index 7f17b1e3043..07738c5f7de 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/errors.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/errors.rs @@ -37,8 +37,8 @@ pub enum DefCollectorErrorKind { CannotReexportItemWithLessVisibility { item_name: Ident, desired_visibility: ItemVisibility }, #[error("Non-struct type used in impl")] NonStructTypeInImpl { location: Location }, - #[error("Cannot implement trait on a mutable reference type")] - MutableReferenceInTraitImpl { location: Location }, + #[error("Cannot implement trait on a reference type")] + ReferenceInTraitImpl { location: Location }, #[error("Impl for type `{typ}` overlaps with existing impl")] OverlappingImpl { typ: crate::Type, location: Location, prev_location: Location }, #[error("Cannot `impl` a type defined outside the current crate")] @@ -97,7 +97,7 @@ impl DefCollectorErrorKind { | DefCollectorErrorKind::TestOnAssociatedFunction { location } | DefCollectorErrorKind::ExportOnAssociatedFunction { location } | DefCollectorErrorKind::NonStructTypeInImpl { location } - | DefCollectorErrorKind::MutableReferenceInTraitImpl { location } + | DefCollectorErrorKind::ReferenceInTraitImpl { location } | DefCollectorErrorKind::OverlappingImpl { location, .. } | DefCollectorErrorKind::ModuleAlreadyPartOfCrate { location, .. } | DefCollectorErrorKind::ModuleOriginallyDefined { location, .. } @@ -199,8 +199,8 @@ impl<'a> From<&'a DefCollectorErrorKind> for Diagnostic { "Only struct types may have implementation methods".into(), *location, ), - DefCollectorErrorKind::MutableReferenceInTraitImpl { location } => Diagnostic::simple_error( - "Trait impls are not allowed on mutable reference types".into(), + DefCollectorErrorKind::ReferenceInTraitImpl { location } => Diagnostic::simple_error( + "Trait impls are not allowed on reference types".into(), "Try using a struct type here instead".into(), *location, ), diff --git a/compiler/noirc_frontend/src/hir_def/types.rs b/compiler/noirc_frontend/src/hir_def/types.rs index 1f4b21cb9a9..d1c70a18259 100644 --- a/compiler/noirc_frontend/src/hir_def/types.rs +++ b/compiler/noirc_frontend/src/hir_def/types.rs @@ -111,8 +111,8 @@ pub enum Type { /*unconstrained*/ bool, ), - /// &mut T - MutableReference(Box), + /// &T + Reference(Box, /*mutable*/ bool), /// A type generic over the given type variables. /// Storing both the TypeVariableId and TypeVariable isn't necessary @@ -1047,9 +1047,12 @@ impl std::fmt::Display for Type { write!(f, "fn{closure_env_text}({}) -> {ret}", args.join(", ")) } - Type::MutableReference(element) => { + Type::Reference(element, mutable) if *mutable => { write!(f, "&mut {element}") } + Type::Reference(element, _) => { + write!(f, "&{element}") + } Type::Quoted(quoted) => write!(f, "{}", quoted), Type::InfixExpr(lhs, op, rhs, _) => { let this = self.canonicalize_checked(); @@ -1221,7 +1224,7 @@ impl Type { Type::Alias(alias_type, generics) => { alias_type.borrow().get_type(&generics).is_primitive() } - Type::MutableReference(typ) => typ.is_primitive(), + Type::Reference(typ, _) => typ.is_primitive(), Type::DataType(..) | Type::TypeVariable(..) | Type::TraitAsType(..) @@ -1244,7 +1247,7 @@ impl Type { } pub(crate) fn is_mutable_ref(&self) -> bool { - matches!(self.follow_bindings_shallow().as_ref(), Type::MutableReference(_)) + matches!(self.follow_bindings_shallow().as_ref(), Type::Reference(_, true)) } /// True if this type can be used as a parameter to `main` or a contract function. @@ -1269,7 +1272,7 @@ impl Type { | Type::TypeVariable(_) | Type::NamedGeneric(_, _) | Type::Function(_, _, _, _) - | Type::MutableReference(_) + | Type::Reference(..) | Type::Forall(_, _) | Type::Quoted(_) | Type::Slice(_) @@ -1327,7 +1330,7 @@ impl Type { // This is possible as long as the output size is not dependent upon a witness condition. | Type::Function(_, _, _, _) | Type::Slice(_) - | Type::MutableReference(_) + | Type::Reference(..) | Type::Forall(_, _) // TODO: probably can allow code as it is all compile time | Type::Quoted(_) @@ -1385,7 +1388,7 @@ impl Type { // environment is the interpreter. In this environment, they are valid. Type::Quoted(_) => true, - Type::MutableReference(_) | Type::Forall(_, _) | Type::TraitAsType(..) => false, + Type::Reference(..) | Type::Forall(_, _) | Type::TraitAsType(..) => false, Type::Alias(alias, generics) => { let alias = alias.borrow(); @@ -1477,7 +1480,7 @@ impl Type { | Type::DataType(..) | Type::TraitAsType(..) | Type::Function(..) - | Type::MutableReference(..) + | Type::Reference(..) | Type::Forall(..) | Type::Quoted(..) => Kind::Normal, Type::Error => Kind::Any, @@ -1574,7 +1577,7 @@ impl Type { | Type::TraitAsType(..) | Type::NamedGeneric(_, _) | Type::Function(_, _, _, _) - | Type::MutableReference(_) + | Type::Reference(..) | Type::Forall(_, _) | Type::Constant(_, _) | Type::Quoted(_) @@ -1908,8 +1911,12 @@ impl Type { } } - (MutableReference(elem_a), MutableReference(elem_b)) => { - elem_a.try_unify(elem_b, bindings) + (Reference(elem_a, mutable_a), Reference(elem_b, mutable_b)) => { + if mutable_a == mutable_b { + elem_a.try_unify(elem_b, bindings) + } else { + Err(UnificationError) + } } (InfixExpr(lhs_a, op_a, rhs_a, _), InfixExpr(lhs_b, op_b, rhs_b, _)) => { @@ -2026,6 +2033,10 @@ impl Type { return; } + if self.try_reference_coercion(expected) { + return; + } + // Try to coerce `fn (..) -> T` to `unconstrained fn (..) -> T` match self.try_fn_to_unconstrained_fn_coercion(expected) { FunctionCoercionResult::NoCoercion => errors.push(make_error()), @@ -2094,6 +2105,25 @@ impl Type { false } + /// Attempt to coerce `&mut T` to `&T`, returning true if this is possible. + pub fn try_reference_coercion(&self, target: &Type) -> bool { + let this = self.follow_bindings(); + let target = target.follow_bindings(); + + if let (Type::Reference(this_elem, true), Type::Reference(target_elem, false)) = + (&this, &target) + { + // Still have to ensure the element types match. + // Don't need to issue an error here if not, it will be done in unify_with_coercions + let mut bindings = TypeBindings::new(); + if this_elem.try_unify(target_elem, &mut bindings).is_ok() { + Self::apply_type_bindings(bindings); + return true; + } + } + false + } + /// Apply the given type bindings, making them permanently visible for each /// clone of each type variable bound. pub fn apply_type_bindings(bindings: TypeBindings) { @@ -2453,9 +2483,10 @@ impl Type { let env = Box::new(env.substitute_helper(type_bindings, substitute_bound_typevars)); Type::Function(args, ret, env, *unconstrained) } - Type::MutableReference(element) => Type::MutableReference(Box::new( - element.substitute_helper(type_bindings, substitute_bound_typevars), - )), + Type::Reference(element, mutable) => Type::Reference( + Box::new(element.substitute_helper(type_bindings, substitute_bound_typevars)), + *mutable, + ), Type::TraitAsType(s, name, generics) => { let ordered = vecmap(&generics.ordered, |arg| { @@ -2519,7 +2550,7 @@ impl Type { || ret.occurs(target_id) || env.occurs(target_id) } - Type::MutableReference(element) => element.occurs(target_id), + Type::Reference(element, _) => element.occurs(target_id), Type::InfixExpr(lhs, _op, rhs, _) => lhs.occurs(target_id) || rhs.occurs(target_id), Type::FieldElement @@ -2579,7 +2610,7 @@ impl Type { Function(args, ret, env, *unconstrained) } - MutableReference(element) => MutableReference(Box::new(element.follow_bindings())), + Reference(element, mutable) => Reference(Box::new(element.follow_bindings()), *mutable), TraitAsType(s, name, args) => { let ordered = vecmap(&args.ordered, |arg| arg.follow_bindings()); @@ -2704,7 +2735,7 @@ impl Type { ret.replace_named_generics_with_type_variables(); env.replace_named_generics_with_type_variables(); } - Type::MutableReference(elem) => elem.replace_named_generics_with_type_variables(), + Type::Reference(elem, _) => elem.replace_named_generics_with_type_variables(), Type::Forall(_, typ) => typ.replace_named_generics_with_type_variables(), Type::InfixExpr(lhs, _op, rhs, _) => { lhs.replace_named_generics_with_type_variables(); @@ -2748,7 +2779,7 @@ impl Type { TypeBinding::Bound(typ) => typ.integral_maximum_size(), TypeBinding::Unbound(_, kind) => kind.integral_maximum_size(), }, - Type::MutableReference(typ) => typ.integral_maximum_size(), + Type::Reference(typ, _) => typ.integral_maximum_size(), Type::InfixExpr(lhs, _op, rhs, _) => lhs.infix_kind(rhs).integral_maximum_size(), Type::Constant(_, kind) => kind.integral_maximum_size(), @@ -2975,8 +3006,8 @@ impl From<&Type> for PrintableType { env: Box::new(env.as_ref().into()), unconstrained: *unconstrained, }, - Type::MutableReference(typ) => { - PrintableType::MutableReference { typ: Box::new(typ.as_ref().into()) } + Type::Reference(typ, mutable) => { + PrintableType::Reference { typ: Box::new(typ.as_ref().into()), mutable: *mutable } } Type::Quoted(_) => unreachable!(), Type::InfixExpr(..) => unreachable!(), @@ -3069,7 +3100,10 @@ impl std::fmt::Debug for Type { write!(f, "fn({}) -> {ret:?}{closure_env_text}", args.join(", ")) } - Type::MutableReference(element) => { + Type::Reference(element, false) => { + write!(f, "&{element:?}") + } + Type::Reference(element, true) => { write!(f, "&mut {element:?}") } Type::Quoted(quoted) => write!(f, "{}", quoted), @@ -3151,7 +3185,10 @@ impl std::hash::Hash for Type { env.hash(state); is_unconstrained.hash(state); } - Type::MutableReference(elem) => elem.hash(state), + Type::Reference(elem, mutable) => { + elem.hash(state); + mutable.hash(state); + } Type::Forall(vars, typ) => { vars.hash(state); typ.hash(state); @@ -3219,7 +3256,9 @@ impl PartialEq for Type { let args_and_ret_eq = lhs_args == rhs_args && lhs_ret == rhs_ret; args_and_ret_eq && lhs_env == rhs_env && lhs_unconstrained == rhs_unconstrained } - (MutableReference(lhs_elem), MutableReference(rhs_elem)) => lhs_elem == rhs_elem, + (Reference(lhs_elem, lhs_mut), Reference(rhs_elem, rhs_mut)) => { + lhs_elem == rhs_elem && lhs_mut == rhs_mut + } (Forall(lhs_vars, lhs_type), Forall(rhs_vars, rhs_type)) => { lhs_vars == rhs_vars && lhs_type == rhs_type } diff --git a/compiler/noirc_frontend/src/lexer/lexer.rs b/compiler/noirc_frontend/src/lexer/lexer.rs index 630f192c109..a6657d63866 100644 --- a/compiler/noirc_frontend/src/lexer/lexer.rs +++ b/compiler/noirc_frontend/src/lexer/lexer.rs @@ -106,6 +106,8 @@ impl<'a> Lexer<'a> { // and the next token issued will be the next '&'. let span = Span::inclusive(self.position, self.position + 1); Err(LexerErrorKind::LogicalAnd { location: self.location(span) }) + } else if self.peek_char_is('[') { + self.single_char_token(Token::SliceStart) } else { self.single_char_token(Token::Ampersand) } diff --git a/compiler/noirc_frontend/src/lexer/token.rs b/compiler/noirc_frontend/src/lexer/token.rs index 7367489f625..effdc827f2f 100644 --- a/compiler/noirc_frontend/src/lexer/token.rs +++ b/compiler/noirc_frontend/src/lexer/token.rs @@ -188,6 +188,10 @@ pub enum Token { Percent, /// & Ampersand, + /// & followed immediately by '[' + /// This is a lexer hack to distinguish slices + /// from taking a reference to an array + SliceStart, /// ^ Caret, /// << @@ -287,6 +291,7 @@ pub fn token_to_borrowed_token(token: &Token) -> BorrowedToken<'_> { Token::Slash => BorrowedToken::Slash, Token::Percent => BorrowedToken::Percent, Token::Ampersand => BorrowedToken::Ampersand, + Token::SliceStart => BorrowedToken::Ampersand, Token::Caret => BorrowedToken::Caret, Token::ShiftLeft => BorrowedToken::ShiftLeft, Token::ShiftRight => BorrowedToken::ShiftRight, @@ -522,6 +527,7 @@ impl fmt::Display for Token { Token::Slash => write!(f, "/"), Token::Percent => write!(f, "%"), Token::Ampersand => write!(f, "&"), + Token::SliceStart => write!(f, "&"), Token::Caret => write!(f, "^"), Token::ShiftLeft => write!(f, "<<"), Token::ShiftRight => write!(f, ">>"), diff --git a/compiler/noirc_frontend/src/monomorphization/mod.rs b/compiler/noirc_frontend/src/monomorphization/mod.rs index 4b0c4b68345..ba4640fa104 100644 --- a/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -1280,7 +1280,8 @@ impl<'interner> Monomorphizer<'interner> { } } - HirType::MutableReference(element) => { + // Lower both mutable & immutable references to the same reference type + HirType::Reference(element, _mutable) => { let element = Self::convert_type(element, location)?; ast::Type::MutableReference(Box::new(element)) } @@ -1386,7 +1387,7 @@ impl<'interner> Monomorphizer<'interner> { Self::check_type(env, location) } - HirType::MutableReference(element) => Self::check_type(element, location), + HirType::Reference(element, _mutable) => Self::check_type(element, location), HirType::InfixExpr(lhs, _, rhs, _) => { Self::check_type(lhs, location)?; Self::check_type(rhs, location) @@ -1601,8 +1602,8 @@ impl<'interner> Monomorphizer<'interner> { fn append_printable_type_info_inner(typ: &Type, arguments: &mut Vec) { // Disallow printing slices and mutable references for consistency, // since they cannot be passed from ACIR into Brillig - if matches!(typ, HirType::MutableReference(_)) { - unreachable!("println and format strings do not support mutable references."); + if matches!(typ, HirType::Reference(..)) { + unreachable!("println and format strings do not support references."); } let printable_type: PrintableType = typ.into(); @@ -2102,13 +2103,13 @@ impl<'interner> Monomorphizer<'interner> { })) } ast::Type::MutableReference(element) => { - use crate::ast::UnaryOp::MutableReference; + use crate::ast::UnaryOp::Reference; let rhs = Box::new(self.zeroed_value_of_type(element, location)); let result_type = typ.clone(); ast::Expression::Unary(ast::Unary { rhs, result_type, - operator: MutableReference, + operator: Reference { mutable: true }, location, }) } diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 2430dca8fca..12ef39f6b18 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1411,7 +1411,7 @@ impl NodeInterner { ) -> Option { match self_type { Type::Error => None, - Type::MutableReference(element) => { + Type::Reference(element, _mutable) => { self.add_method(element, method_name, method_id, trait_id) } _ => { @@ -2415,8 +2415,8 @@ impl Methods { return true; } - // Handle auto-dereferencing `&mut T` into `T` - if let Type::MutableReference(object) = object { + // Handle auto-dereferencing `&T` and `&mut T` into `T` + if let Type::Reference(object, _mutable) = object { if object.unify(typ).is_ok() { return true; } @@ -2430,8 +2430,8 @@ impl Methods { return true; } - // Handle auto-dereferencing `&mut T` into `T` - if let Type::MutableReference(method_type) = method_type { + // Handle auto-dereferencing `&T` and `&mut T` into `T` + if let Type::Reference(method_type, _mutable) = method_type { if method_type.unify(typ).is_ok() { return true; } @@ -2488,7 +2488,7 @@ fn get_type_method_key(typ: &Type) -> Option { Type::Function(_, _, _, _) => Some(Function), Type::NamedGeneric(_, _) => Some(Generic), Type::Quoted(quoted) => Some(Quoted(*quoted)), - Type::MutableReference(element) => get_type_method_key(element), + Type::Reference(element, _) => get_type_method_key(element), Type::Alias(alias, _) => get_type_method_key(&alias.borrow().typ), Type::DataType(struct_type, _) => Some(Struct(struct_type.borrow().id)), diff --git a/compiler/noirc_frontend/src/parser/parser.rs b/compiler/noirc_frontend/src/parser/parser.rs index a5ea2ea5fe9..b797a15f0c1 100644 --- a/compiler/noirc_frontend/src/parser/parser.rs +++ b/compiler/noirc_frontend/src/parser/parser.rs @@ -557,6 +557,7 @@ impl<'a> Parser<'a> { ); } + #[allow(unused)] fn expected_mut_after_ampersand(&mut self) { self.push_error( ParserErrorReason::ExpectedMutAfterAmpersand { found: self.token.token().clone() }, diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs index d0f335414da..672d9428d9a 100644 --- a/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -79,10 +79,14 @@ impl Parser<'_> { /// UnaryOp = '&' 'mut' | '-' | '!' | '*' fn parse_unary_op(&mut self) -> Option { - if self.at(Token::Ampersand) && self.next_is(Token::Keyword(Keyword::Mut)) { - self.bump(); + if self.at(Token::Ampersand) { + let mut mutable = false; + if self.next_is(Token::Keyword(Keyword::Mut)) { + mutable = true; + self.bump(); + } self.bump(); - Some(UnaryOp::MutableReference) + Some(UnaryOp::Reference { mutable }) } else if self.eat(Token::Minus) { Some(UnaryOp::Minus) } else if self.eat(Token::Bang) { @@ -738,7 +742,7 @@ impl Parser<'_> { /// SliceExpression = '&' ArrayLiteral fn parse_slice_literal(&mut self) -> Option { - if !(self.at(Token::Ampersand) && self.next_is(Token::LeftBracket)) { + if !(self.at(Token::SliceStart) && self.next_is(Token::LeftBracket)) { return None; } @@ -1257,7 +1261,7 @@ mod tests { let ExpressionKind::Prefix(prefix) = expr.kind else { panic!("Expected prefix expression"); }; - assert!(matches!(prefix.operator, UnaryOp::MutableReference)); + assert!(matches!(prefix.operator, UnaryOp::Reference { mutable: true })); let ExpressionKind::Variable(path) = prefix.rhs.kind else { panic!("Expected variable"); diff --git a/compiler/noirc_frontend/src/parser/parser/function.rs b/compiler/noirc_frontend/src/parser/parser/function.rs index f10b790e63f..caf2cdeb1c3 100644 --- a/compiler/noirc_frontend/src/parser/parser/function.rs +++ b/compiler/noirc_frontend/src/parser/parser/function.rs @@ -229,7 +229,7 @@ impl Parser<'_> { let mut pattern = Pattern::Identifier(ident); if self_pattern.reference { - self_type = UnresolvedTypeData::MutableReference(Box::new(self_type)) + self_type = UnresolvedTypeData::Reference(Box::new(self_type), self_pattern.mutable) .with_location(ident_location); } else if self_pattern.mutable { pattern = Pattern::Mutable(Box::new(pattern), ident_location, true); diff --git a/compiler/noirc_frontend/src/parser/parser/pattern.rs b/compiler/noirc_frontend/src/parser/parser/pattern.rs index 61fb1572c17..e1468845ded 100644 --- a/compiler/noirc_frontend/src/parser/parser/pattern.rs +++ b/compiler/noirc_frontend/src/parser/parser/pattern.rs @@ -66,14 +66,12 @@ impl Parser<'_> { } } - if self.at(Token::Ampersand) && self.next_is(Token::Keyword(Keyword::Mut)) { - self.bump(); + if self.at(Token::Ampersand) { self.bump(); + + let mutable = self.eat_keyword(Keyword::Mut); if !self.next_is_colon() && self.eat_self() { - return Some(PatternOrSelf::SelfPattern(SelfPattern { - reference: true, - mutable: true, - })); + return Some(PatternOrSelf::SelfPattern(SelfPattern { reference: true, mutable })); } else { self.push_error( ParserErrorReason::RefMutCanOnlyBeUsedWithSelf, diff --git a/compiler/noirc_frontend/src/parser/parser/types.rs b/compiler/noirc_frontend/src/parser/parser/types.rs index bcbf57d863d..1ccb68e2677 100644 --- a/compiler/noirc_frontend/src/parser/parser/types.rs +++ b/compiler/noirc_frontend/src/parser/parser/types.rs @@ -370,15 +370,15 @@ impl Parser<'_> { } fn parses_mutable_reference_type(&mut self) -> Option { - if self.eat(Token::Ampersand) { - if !self.eat_keyword(Keyword::Mut) { - self.expected_mut_after_ampersand(); - } + // The `&` may be lexed as a slice start if this is an array or slice type + if self.eat(Token::Ampersand) || self.eat(Token::SliceStart) { + let mutable = self.eat_keyword(Keyword::Mut); - return Some(UnresolvedTypeData::MutableReference(Box::new( - self.parse_type_or_error(), - ))); - }; + return Some(UnresolvedTypeData::Reference( + Box::new(self.parse_type_or_error()), + mutable, + )); + } None } @@ -604,11 +604,21 @@ mod tests { assert!(matches!(typ.typ, UnresolvedTypeData::FieldElement)); } + #[test] + fn parses_reference_type() { + let src = "&Field"; + let typ = parse_type_no_errors(src); + let UnresolvedTypeData::Reference(typ, false) = typ.typ else { + panic!("Expected a reference type") + }; + assert!(matches!(typ.typ, UnresolvedTypeData::FieldElement)); + } + #[test] fn parses_mutable_reference_type() { let src = "&mut Field"; let typ = parse_type_no_errors(src); - let UnresolvedTypeData::MutableReference(typ) = typ.typ else { + let UnresolvedTypeData::Reference(typ, true) = typ.typ else { panic!("Expected a mutable reference type") }; assert!(matches!(typ.typ, UnresolvedTypeData::FieldElement)); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index e53aa392fa9..62da755887b 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/compiler/noirc_frontend/src/tests.rs @@ -4114,3 +4114,35 @@ fn deny_attaching_mut_ref_to_immutable_object() { "#; check_errors(src); } + +#[test] +fn immutable_references_with_ownership_feature() { + let src = r#" + unconstrained fn main() { + let mut array = [1, 2, 3]; + borrow(&array); + } + + fn borrow(_array: &[Field; 3]) {} + "#; + + let (_, _, errors) = get_program_using_features(src, &[UnstableFeature::Ownership]); + assert_eq!(errors.len(), 0); +} + +#[test] +fn immutable_references_without_ownership_feature() { + let src = r#" + fn main() { + let mut array = [1, 2, 3]; + borrow(&array); + ^^^^^^ This requires the unstable feature 'ownership' which is not enabled + ~~~~~~ Pass -Zownership to nargo to enable this feature at your own risk. + } + + fn borrow(_array: &[Field; 3]) {} + ^^^^^^^^^^^ This requires the unstable feature 'ownership' which is not enabled + ~~~~~~~~~~~ Pass -Zownership to nargo to enable this feature at your own risk. + "#; + check_errors(src); +} diff --git a/compiler/noirc_printable_type/src/lib.rs b/compiler/noirc_printable_type/src/lib.rs index 6ff211cac2a..8e25de58b43 100644 --- a/compiler/noirc_printable_type/src/lib.rs +++ b/compiler/noirc_printable_type/src/lib.rs @@ -49,8 +49,9 @@ pub enum PrintableType { env: Box, unconstrained: bool, }, - MutableReference { + Reference { typ: Box, + mutable: bool, }, Unit, } @@ -127,7 +128,10 @@ fn to_string(value: &PrintableValue, typ: &PrintableType) -> Op (PrintableValue::Field(_), PrintableType::Function { arguments, return_type, .. }) => { output.push_str(&format!("< {:?}>>", arguments, return_type,)); } - (_, PrintableType::MutableReference { .. }) => { + (_, PrintableType::Reference { mutable: false, .. }) => { + output.push_str("<>"); + } + (_, PrintableType::Reference { mutable: true, .. }) => { output.push_str("<>"); } (PrintableValue::Vec { array_elements, is_slice }, PrintableType::Array { typ, .. }) diff --git a/tooling/lsp/src/requests/completion.rs b/tooling/lsp/src/requests/completion.rs index 9d11aba9534..65207b4d95d 100644 --- a/tooling/lsp/src/requests/completion.rs +++ b/tooling/lsp/src/requests/completion.rs @@ -579,7 +579,7 @@ impl<'a> NodeFinder<'a> { Type::DataType(struct_type, generics) => { self.complete_struct_fields(&struct_type.borrow(), generics, prefix, self_prefix); } - Type::MutableReference(typ) => { + Type::Reference(typ, _) => { return self.complete_type_fields_and_methods( typ, prefix, diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index bc266c03f76..0bd984dcbea 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -188,8 +188,8 @@ impl NodeFinder<'_> { let func_self_type = if let Some((pattern, typ, _)) = func_meta.parameters.0.first() { if self.hir_pattern_is_self_type(pattern) { - if let Type::MutableReference(mut_typ) = typ { - let typ: &Type = mut_typ; + if let Type::Reference(elem_type, _) = typ { + let typ: &Type = elem_type; Some(typ) } else { Some(typ) @@ -222,9 +222,8 @@ impl NodeFinder<'_> { // Check that the pattern type is the same as self type. // We do this because some types (only Field and integer types) // have their methods in the same HashMap. - - if let Type::MutableReference(mut_typ) = self_type { - self_type = mut_typ; + if let Type::Reference(elem_type, _) = self_type { + self_type = elem_type; } if self_type != func_self_type { @@ -597,7 +596,7 @@ fn func_meta_type_to_string(func_meta: &FuncMeta, name: &str, has_self_type: boo } fn type_to_self_string(typ: &Type, string: &mut String) { - if let Type::MutableReference(..) = typ { + if let Type::Reference(..) = typ { string.push_str("&mut self"); } else { string.push_str("self"); diff --git a/tooling/lsp/src/requests/hover/from_reference.rs b/tooling/lsp/src/requests/hover/from_reference.rs index 3bc3b3bded7..a8ed06c0896 100644 --- a/tooling/lsp/src/requests/hover/from_reference.rs +++ b/tooling/lsp/src/requests/hover/from_reference.rs @@ -498,7 +498,7 @@ fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { let is_self = pattern_is_self(pattern, args.interner); // `&mut self` is represented as a mutable reference type, not as a mutable pattern - if is_self && matches!(typ, Type::MutableReference(..)) { + if is_self && matches!(typ, Type::Reference(..)) { string.push_str("&mut "); } @@ -814,7 +814,7 @@ impl TypeLinksGatherer<'_> { self.gather_type_links(return_type); self.gather_type_links(env); } - Type::MutableReference(typ) => self.gather_type_links(typ), + Type::Reference(typ, _) => self.gather_type_links(typ), Type::InfixExpr(lhs, _, rhs, _) => { self.gather_type_links(lhs); self.gather_type_links(rhs); diff --git a/tooling/lsp/src/requests/inlay_hint.rs b/tooling/lsp/src/requests/inlay_hint.rs index 8c794d28868..37cccb789a2 100644 --- a/tooling/lsp/src/requests/inlay_hint.rs +++ b/tooling/lsp/src/requests/inlay_hint.rs @@ -511,7 +511,11 @@ fn push_type_parts(typ: &Type, parts: &mut Vec, files: &File parts.push(string_part(") -> ")); push_type_parts(return_type, parts, files); } - Type::MutableReference(typ) => { + Type::Reference(typ, false) => { + parts.push(string_part("&")); + push_type_parts(typ, parts, files); + } + Type::Reference(typ, true) => { parts.push(string_part("&mut ")); push_type_parts(typ, parts, files); } diff --git a/tooling/lsp/src/requests/signature_help.rs b/tooling/lsp/src/requests/signature_help.rs index 7563dc50c98..13c37c33a0b 100644 --- a/tooling/lsp/src/requests/signature_help.rs +++ b/tooling/lsp/src/requests/signature_help.rs @@ -146,7 +146,7 @@ impl<'a> SignatureFinder<'a> { } if has_self && index == 0 { - if let Type::MutableReference(..) = typ { + if let Type::Reference(..) = typ { label.push_str("&mut self"); } else { label.push_str("self"); diff --git a/tooling/lsp/src/trait_impl_method_stub_generator.rs b/tooling/lsp/src/trait_impl_method_stub_generator.rs index 3a60a882aea..b24f4dd7d87 100644 --- a/tooling/lsp/src/trait_impl_method_stub_generator.rs +++ b/tooling/lsp/src/trait_impl_method_stub_generator.rs @@ -359,7 +359,11 @@ impl<'a> TraitImplMethodStubGenerator<'a> { self.append_type(ret); } } - Type::MutableReference(typ) => { + Type::Reference(typ, false) => { + self.string.push('&'); + self.append_type(typ); + } + Type::Reference(typ, true) => { self.string.push_str("&mut "); self.append_type(typ); } diff --git a/tooling/lsp/src/with_file.rs b/tooling/lsp/src/with_file.rs index 1f0d02db421..3ba8aecf74a 100644 --- a/tooling/lsp/src/with_file.rs +++ b/tooling/lsp/src/with_file.rs @@ -485,8 +485,8 @@ fn unresolved_type_data_with_file(typ: UnresolvedTypeData, file: FileId) -> Unre generic_type_args_with_file(generic_type_args, file), ) } - UnresolvedTypeData::MutableReference(typ) => { - UnresolvedTypeData::MutableReference(Box::new(unresolved_type_with_file(*typ, file))) + UnresolvedTypeData::Reference(typ, mutable) => { + UnresolvedTypeData::Reference(Box::new(unresolved_type_with_file(*typ, file)), mutable) } UnresolvedTypeData::Tuple(types) => { UnresolvedTypeData::Tuple(unresolved_types_with_file(types, file)) diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index 6b46a4557a2..39f72a0c098 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -715,7 +715,7 @@ impl ChunkFormatter<'_, '_> { fn format_prefix(&mut self, prefix: PrefixExpression) -> ChunkGroup { let mut group = ChunkGroup::new(); group.text(self.chunk(|formatter| { - if let UnaryOp::MutableReference = prefix.operator { + if let UnaryOp::Reference { mutable: true } = prefix.operator { formatter.write_current_token(); formatter.bump(); formatter.skip_comments_and_whitespace(); diff --git a/tooling/nargo_fmt/src/formatter/types.rs b/tooling/nargo_fmt/src/formatter/types.rs index 6a0e66bc1f9..04e78c04162 100644 --- a/tooling/nargo_fmt/src/formatter/types.rs +++ b/tooling/nargo_fmt/src/formatter/types.rs @@ -75,10 +75,12 @@ impl Formatter<'_> { self.format_path(path); self.format_generic_type_args(generic_type_args); } - UnresolvedTypeData::MutableReference(typ) => { + UnresolvedTypeData::Reference(typ, mutable) => { self.write_token(Token::Ampersand); - self.write_keyword(Keyword::Mut); - self.write_space(); + if mutable { + self.write_keyword(Keyword::Mut); + self.write_space(); + } self.format_type(*typ); } UnresolvedTypeData::Tuple(types) => { diff --git a/tooling/noirc_abi/src/printable_type.rs b/tooling/noirc_abi/src/printable_type.rs index e13cab06e9f..75c977cd91a 100644 --- a/tooling/noirc_abi/src/printable_type.rs +++ b/tooling/noirc_abi/src/printable_type.rs @@ -69,7 +69,7 @@ pub fn decode_value( decode_value(field_iterator, env); func_ref } - PrintableType::MutableReference { typ } => { + PrintableType::Reference { typ, .. } => { // we decode the reference, but it's not really used for printing decode_value(field_iterator, typ) } From c79b17a1b530b46833e3c671d1026a4da7d044b6 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 6 Mar 2025 09:37:59 -0600 Subject: [PATCH 2/4] Missed mut cases from PR feedback --- compiler/noirc_frontend/src/ast/visitor.rs | 4 ---- tooling/lsp/src/requests/signature_help.rs | 10 ++++++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/visitor.rs b/compiler/noirc_frontend/src/ast/visitor.rs index 00a3200d632..8e78ca5ec54 100644 --- a/compiler/noirc_frontend/src/ast/visitor.rs +++ b/compiler/noirc_frontend/src/ast/visitor.rs @@ -397,10 +397,6 @@ pub trait Visitor { true } - fn visit_mutable_reference_type(&mut self, _: &UnresolvedType, _: Span) -> bool { - true - } - fn visit_tuple_type(&mut self, _: &[UnresolvedType], _: Span) -> bool { true } diff --git a/tooling/lsp/src/requests/signature_help.rs b/tooling/lsp/src/requests/signature_help.rs index 13c37c33a0b..6706bb098bf 100644 --- a/tooling/lsp/src/requests/signature_help.rs +++ b/tooling/lsp/src/requests/signature_help.rs @@ -146,11 +146,13 @@ impl<'a> SignatureFinder<'a> { } if has_self && index == 0 { - if let Type::Reference(..) = typ { - label.push_str("&mut self"); - } else { - label.push_str("self"); + if let Type::Reference(_, mutable) = typ { + label.push('&'); + if *mutable { + label.push_str("mut "); + } } + label.push_str("self"); } else { let parameter_start = label.chars().count(); From 45ea0b6320c37da08e7b1c6cbb26349dab67b616 Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 6 Mar 2025 11:08:20 -0600 Subject: [PATCH 3/4] Change expected type of a deref to &mut for backwards compat --- compiler/noirc_frontend/src/elaborator/mod.rs | 5 ----- .../noirc_frontend/src/elaborator/types.rs | 18 +++++------------- 2 files changed, 5 insertions(+), 18 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 018e62dd128..fccf99cc7ca 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -2168,11 +2168,6 @@ impl<'context> Elaborator<'context> { } } - /// Return true if the given unstable feature is enabled - fn unstable_feature_enabled(&self, feature: UnstableFeature) -> bool { - self.options.enabled_unstable_features.contains(&feature) - } - /// Run the given function using the resolver and return true if any errors (not warnings) /// occurred while running it. pub fn errors_occurred_in(&mut self, f: impl FnOnce(&mut Self) -> T) -> (bool, T) { diff --git a/compiler/noirc_frontend/src/elaborator/types.rs b/compiler/noirc_frontend/src/elaborator/types.rs index 69baa390140..6f6423017a4 100644 --- a/compiler/noirc_frontend/src/elaborator/types.rs +++ b/compiler/noirc_frontend/src/elaborator/types.rs @@ -1318,22 +1318,14 @@ impl Elaborator<'_> { |mutable| Type::Reference(Box::new(element_type.clone()), mutable); let immutable = make_expected(false); - let ownership_enabled = self.unstable_feature_enabled(UnstableFeature::Ownership); + let mutable = make_expected(true); // Both `&mut T` and `&T` should coerce to an expected `&T`. if !rhs_type.try_reference_coercion(&immutable) { - self.unify(rhs_type, &immutable, || { - let expected_typ = if ownership_enabled { - immutable.to_string() - } else { - make_expected(true).to_string() - }; - - TypeCheckError::TypeMismatch { - expr_typ: rhs_type.to_string(), - expected_typ, - expr_location: location, - } + self.unify(rhs_type, &mutable, || TypeCheckError::TypeMismatch { + expr_typ: rhs_type.to_string(), + expected_typ: mutable.to_string(), + expr_location: location, }); } Ok((element_type, false)) From fa423d4b66dcb26a46c986b47167668b4670f7ed Mon Sep 17 00:00:00 2001 From: Jake Fecher Date: Thu, 6 Mar 2025 11:30:11 -0600 Subject: [PATCH 4/4] Fix slice formatting --- tooling/nargo_fmt/src/formatter/expression.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_fmt/src/formatter/expression.rs b/tooling/nargo_fmt/src/formatter/expression.rs index 39f72a0c098..eb20245e6b6 100644 --- a/tooling/nargo_fmt/src/formatter/expression.rs +++ b/tooling/nargo_fmt/src/formatter/expression.rs @@ -155,7 +155,7 @@ impl ChunkFormatter<'_, '_> { group.text(self.chunk(|formatter| { if is_slice { - formatter.write_token(Token::Ampersand); + formatter.write_token(Token::SliceStart); } formatter.write_left_bracket(); }));