From 7fb2a454531db8cef757b5ec2028d97e823bef5f Mon Sep 17 00:00:00 2001 From: Aztec Bot <49558828+AztecBot@users.noreply.github.com> Date: Thu, 19 Sep 2024 10:07:11 -0400 Subject: [PATCH] feat: Sync from noir (#8643) Automated pull of development from the [noir](https://github.com/noir-lang/noir) programming language, a dependency of Aztec. BEGIN_COMMIT_OVERRIDE fix: preserve generic kind on trait methods (https://github.com/noir-lang/noir/pull/6099) feat: (LSP) show global value on hover (https://github.com/noir-lang/noir/pull/6097) feat: (LSP) if in runtime code, always suggest functions that return Quoted as macro calls (https://github.com/noir-lang/noir/pull/6098) chore: fix broken formatting on master (https://github.com/noir-lang/noir/pull/6096) END_COMMIT_OVERRIDE --------- Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- .noir-sync-commit | 2 +- .../noirc_frontend/src/elaborator/traits.rs | 4 +- .../noirc_frontend/src/elaborator/types.rs | 166 +++++++++--------- .../compiler/noirc_frontend/src/tests.rs | 24 +++ .../tooling/lsp/src/requests/completion.rs | 14 ++ .../requests/completion/completion_items.rs | 8 +- .../lsp/src/requests/completion/tests.rs | 22 ++- .../tooling/lsp/src/requests/hover.rs | 96 +++++++++- 8 files changed, 244 insertions(+), 92 deletions(-) diff --git a/.noir-sync-commit b/.noir-sync-commit index 8f0c7e675db..87a3bf56846 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -2174ffb92b5d88e7e0926c91f42bc7f849e8ddc1 \ No newline at end of file +1df102a1ee0eb39dcbada50e10b226c7f7be0f26 \ No newline at end of file diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs index 9263fe9c67d..0faaf409e6c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs @@ -288,10 +288,10 @@ pub(crate) fn check_trait_impl_method_matches_declaration( // Substitute each generic on the trait function with the corresponding generic on the impl function for ( ResolvedGeneric { type_var: trait_fn_generic, .. }, - ResolvedGeneric { name, type_var: impl_fn_generic, .. }, + ResolvedGeneric { name, type_var: impl_fn_generic, kind, .. }, ) in trait_fn_meta.direct_generics.iter().zip(&meta.direct_generics) { - let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone(), Kind::Normal); + let arg = Type::NamedGeneric(impl_fn_generic.clone(), name.clone(), kind.clone()); bindings.insert(trait_fn_generic.id(), (trait_fn_generic.clone(), arg)); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index f40b62f2f31..6be18df7b52 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -622,7 +622,7 @@ impl<'context> Elaborator<'context> { let length = stmt.expression; let span = self.interner.expr_span(&length); - let result = self.try_eval_array_length_id(length, span); + let result = try_eval_array_length_id(self.interner, length, span); match result.map(|length| length.try_into()) { Ok(Ok(length_value)) => return length_value, @@ -633,88 +633,6 @@ impl<'context> Elaborator<'context> { 0 } - fn try_eval_array_length_id( - &self, - rhs: ExprId, - span: Span, - ) -> Result> { - // Arbitrary amount of recursive calls to try before giving up - let fuel = 100; - self.try_eval_array_length_id_with_fuel(rhs, span, fuel) - } - - fn try_eval_array_length_id_with_fuel( - &self, - rhs: ExprId, - span: Span, - fuel: u32, - ) -> Result> { - if fuel == 0 { - // If we reach here, it is likely from evaluating cyclic globals. We expect an error to - // be issued for them after name resolution so issue no error now. - return Err(None); - } - - match self.interner.expression(&rhs) { - HirExpression::Literal(HirLiteral::Integer(int, false)) => { - int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) - } - HirExpression::Ident(ident, _) => { - if let Some(definition) = self.interner.try_definition(ident.id) { - match definition.kind { - DefinitionKind::Global(global_id) => { - let let_statement = self.interner.get_global_let_statement(global_id); - if let Some(let_statement) = let_statement { - let expression = let_statement.expression; - self.try_eval_array_length_id_with_fuel(expression, span, fuel - 1) - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - _ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } - } else { - Err(Some(ResolverError::InvalidArrayLengthExpr { span })) - } - } - HirExpression::Infix(infix) => { - let lhs = self.try_eval_array_length_id_with_fuel(infix.lhs, span, fuel - 1)?; - let rhs = self.try_eval_array_length_id_with_fuel(infix.rhs, span, fuel - 1)?; - - match infix.operator.kind { - BinaryOpKind::Add => Ok(lhs + rhs), - BinaryOpKind::Subtract => Ok(lhs - rhs), - BinaryOpKind::Multiply => Ok(lhs * rhs), - BinaryOpKind::Divide => Ok(lhs / rhs), - BinaryOpKind::Equal => Ok((lhs == rhs) as u128), - BinaryOpKind::NotEqual => Ok((lhs != rhs) as u128), - BinaryOpKind::Less => Ok((lhs < rhs) as u128), - BinaryOpKind::LessEqual => Ok((lhs <= rhs) as u128), - BinaryOpKind::Greater => Ok((lhs > rhs) as u128), - BinaryOpKind::GreaterEqual => Ok((lhs >= rhs) as u128), - BinaryOpKind::And => Ok(lhs & rhs), - BinaryOpKind::Or => Ok(lhs | rhs), - BinaryOpKind::Xor => Ok(lhs ^ rhs), - BinaryOpKind::ShiftRight => Ok(lhs >> rhs), - BinaryOpKind::ShiftLeft => Ok(lhs << rhs), - BinaryOpKind::Modulo => Ok(lhs % rhs), - } - } - HirExpression::Cast(cast) => { - let lhs = self.try_eval_array_length_id_with_fuel(cast.lhs, span, fuel - 1)?; - let lhs_value = Value::Field(lhs.into()); - let evaluated_value = - Interpreter::evaluate_cast_one_step(&cast, rhs, lhs_value, self.interner) - .map_err(|error| Some(ResolverError::ArrayLengthInterpreter { error }))?; - - evaluated_value - .to_u128() - .ok_or_else(|| Some(ResolverError::InvalidArrayLengthExpr { span })) - } - _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), - } - } - pub fn unify( &mut self, actual: &Type, @@ -1851,6 +1769,88 @@ impl<'context> Elaborator<'context> { } } +pub fn try_eval_array_length_id( + interner: &NodeInterner, + rhs: ExprId, + span: Span, +) -> Result> { + // Arbitrary amount of recursive calls to try before giving up + let fuel = 100; + try_eval_array_length_id_with_fuel(interner, rhs, span, fuel) +} + +fn try_eval_array_length_id_with_fuel( + interner: &NodeInterner, + rhs: ExprId, + span: Span, + fuel: u32, +) -> Result> { + if fuel == 0 { + // If we reach here, it is likely from evaluating cyclic globals. We expect an error to + // be issued for them after name resolution so issue no error now. + return Err(None); + } + + match interner.expression(&rhs) { + HirExpression::Literal(HirLiteral::Integer(int, false)) => { + int.try_into_u128().ok_or(Some(ResolverError::IntegerTooLarge { span })) + } + HirExpression::Ident(ident, _) => { + if let Some(definition) = interner.try_definition(ident.id) { + match definition.kind { + DefinitionKind::Global(global_id) => { + let let_statement = interner.get_global_let_statement(global_id); + if let Some(let_statement) = let_statement { + let expression = let_statement.expression; + try_eval_array_length_id_with_fuel(interner, expression, span, fuel - 1) + } else { + Err(Some(ResolverError::InvalidArrayLengthExpr { span })) + } + } + _ => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), + } + } else { + Err(Some(ResolverError::InvalidArrayLengthExpr { span })) + } + } + HirExpression::Infix(infix) => { + let lhs = try_eval_array_length_id_with_fuel(interner, infix.lhs, span, fuel - 1)?; + let rhs = try_eval_array_length_id_with_fuel(interner, infix.rhs, span, fuel - 1)?; + + match infix.operator.kind { + BinaryOpKind::Add => Ok(lhs + rhs), + BinaryOpKind::Subtract => Ok(lhs - rhs), + BinaryOpKind::Multiply => Ok(lhs * rhs), + BinaryOpKind::Divide => Ok(lhs / rhs), + BinaryOpKind::Equal => Ok((lhs == rhs) as u128), + BinaryOpKind::NotEqual => Ok((lhs != rhs) as u128), + BinaryOpKind::Less => Ok((lhs < rhs) as u128), + BinaryOpKind::LessEqual => Ok((lhs <= rhs) as u128), + BinaryOpKind::Greater => Ok((lhs > rhs) as u128), + BinaryOpKind::GreaterEqual => Ok((lhs >= rhs) as u128), + BinaryOpKind::And => Ok(lhs & rhs), + BinaryOpKind::Or => Ok(lhs | rhs), + BinaryOpKind::Xor => Ok(lhs ^ rhs), + BinaryOpKind::ShiftRight => Ok(lhs >> rhs), + BinaryOpKind::ShiftLeft => Ok(lhs << rhs), + BinaryOpKind::Modulo => Ok(lhs % rhs), + } + } + HirExpression::Cast(cast) => { + let lhs = try_eval_array_length_id_with_fuel(interner, cast.lhs, span, fuel - 1)?; + let lhs_value = Value::Field(lhs.into()); + let evaluated_value = + Interpreter::evaluate_cast_one_step(&cast, rhs, lhs_value, interner) + .map_err(|error| Some(ResolverError::ArrayLengthInterpreter { error }))?; + + evaluated_value + .to_u128() + .ok_or_else(|| Some(ResolverError::InvalidArrayLengthExpr { span })) + } + _other => Err(Some(ResolverError::InvalidArrayLengthExpr { span })), + } +} + /// Gives an error if a user tries to create a mutable reference /// to an immutable variable. fn verify_mutable_reference(interner: &NodeInterner, rhs: ExprId) -> Result<(), ResolverError> { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index cdfbf5847e0..22de18b6461 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -3700,3 +3700,27 @@ fn use_non_u32_generic_in_struct() { let errors = get_program_errors(src); assert_eq!(errors.len(), 0); } + +#[test] +fn use_numeric_generic_in_trait_method() { + let src = r#" + trait Foo { + fn foo(self, x: [u8; N]) -> Self; + } + + struct Bar; + + impl Foo for Bar { + fn foo(self, _x: [u8; N]) -> Self { + self + } + } + + fn main() { + let _ = Bar{}.foo([1,2,3]); + } + "#; + + let errors = get_program_errors(src); + assert_eq!(errors.len(), 0); +} diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion.rs b/noir/noir-repo/tooling/lsp/src/requests/completion.rs index 0d1d7af7ae6..c59544a36ea 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion.rs @@ -113,6 +113,7 @@ struct NodeFinder<'a> { /// The line where an auto_import must be inserted auto_import_line: usize, self_type: Option, + in_comptime: bool, } impl<'a> NodeFinder<'a> { @@ -156,6 +157,7 @@ impl<'a> NodeFinder<'a> { nesting: 0, auto_import_line: 0, self_type: None, + in_comptime: false, } } @@ -1056,8 +1058,12 @@ impl<'a> Visitor for NodeFinder<'a> { self.collect_local_variables(¶m.pattern); } + let old_in_comptime = self.in_comptime; + self.in_comptime = noir_function.def.is_comptime; + noir_function.def.body.accept(Some(span), self); + self.in_comptime = old_in_comptime; self.type_parameters = old_type_parameters; self.self_type = None; @@ -1278,8 +1284,12 @@ impl<'a> Visitor for NodeFinder<'a> { let old_local_variables = self.local_variables.clone(); self.local_variables.clear(); + let old_in_comptime = self.in_comptime; + self.in_comptime = true; + statement.accept(self); + self.in_comptime = old_in_comptime; self.local_variables = old_local_variables; false @@ -1424,8 +1434,12 @@ impl<'a> Visitor for NodeFinder<'a> { let old_local_variables = self.local_variables.clone(); self.local_variables.clear(); + let old_in_comptime = self.in_comptime; + self.in_comptime = true; + block_expression.accept(Some(span), self); + self.in_comptime = old_in_comptime; self.local_variables = old_local_variables; false diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs index 5273e49d4b4..c0155096dc8 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/completion_items.rs @@ -232,7 +232,13 @@ impl<'a> NodeFinder<'a> { if modifiers.is_comptime && matches!(func_meta.return_type(), Type::Quoted(QuotedType::Quoted)) { - vec![make_completion_item(false), make_completion_item(true)] + if self.in_comptime { + vec![make_completion_item(false), make_completion_item(true)] + } else { + // If not in a comptime block we can't operate with comptime values so the only thing + // we can do is call a macro. + vec![make_completion_item(true)] + } } else { vec![make_completion_item(false)] } diff --git a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs index 23d137c1647..7c596bccd11 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/completion/tests.rs @@ -2126,7 +2126,9 @@ mod completion_tests { comptime fn foobar() -> Quoted {} fn main() { - fooba>|< + comptime { + fooba>|< + } } "#; @@ -2140,6 +2142,24 @@ mod completion_tests { .await; } + #[test] + async fn test_suggests_only_macro_call_if_comptime_function_returns_quoted_and_outside_comptime( + ) { + let src = r#" + comptime fn foobar() -> Quoted {} + + fn main() { + fooba>|< + } + "#; + + assert_completion_excluding_auto_import( + src, + vec![function_completion_item("foobar!()", "foobar!()", "fn() -> Quoted")], + ) + .await; + } + #[test] async fn test_only_suggests_macro_call_for_unquote() { let src = r#" diff --git a/noir/noir-repo/tooling/lsp/src/requests/hover.rs b/noir/noir-repo/tooling/lsp/src/requests/hover.rs index edc71e94b08..46d2a5cfc8f 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/hover.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/hover.rs @@ -5,11 +5,12 @@ use fm::FileMap; use lsp_types::{Hover, HoverContents, HoverParams, MarkupContent, MarkupKind}; use noirc_frontend::{ ast::Visibility, + elaborator::types::try_eval_array_length_id, hir::def_map::ModuleId, - hir_def::{stmt::HirPattern, traits::Trait}, - macros_api::{NodeInterner, StructId}, + hir_def::{expr::HirArrayLiteral, stmt::HirPattern, traits::Trait}, + macros_api::{HirExpression, HirLiteral, NodeInterner, StructId}, node_interner::{ - DefinitionId, DefinitionKind, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, + DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, }, Generics, Shared, StructType, Type, TypeAlias, TypeBinding, TypeVariable, }; @@ -166,17 +167,34 @@ fn format_trait(id: TraitId, args: &ProcessRequestCallbackArgs) -> String { fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { let global_info = args.interner.get_global(id); let definition_id = global_info.definition_id; + let definition = args.interner.definition(definition_id); let typ = args.interner.definition_type(definition_id); let mut string = String::new(); if format_parent_module(ReferenceId::Global(id), args, &mut string) { string.push('\n'); } + string.push_str(" "); + if definition.comptime { + string.push_str("comptime "); + } + if definition.mutable { + string.push_str("mut "); + } string.push_str("global "); string.push_str(&global_info.ident.0.contents); string.push_str(": "); string.push_str(&format!("{}", typ)); + + // See if we can figure out what's the global's value + if let Some(stmt) = args.interner.get_global_let_statement(id) { + if let Some(value) = get_global_value(args.interner, stmt.expression) { + string.push_str(" = "); + string.push_str(&value); + } + } + string.push_str(&go_to_type_links(&typ, args.interner, args.files)); append_doc_comments(args.interner, ReferenceId::Global(id), &mut string); @@ -184,6 +202,72 @@ fn format_global(id: GlobalId, args: &ProcessRequestCallbackArgs) -> String { string } +fn get_global_value(interner: &NodeInterner, expr: ExprId) -> Option { + let span = interner.expr_span(&expr); + + // Globals as array lengths are extremely common, so we try that first. + if let Ok(result) = try_eval_array_length_id(interner, expr, span) { + return Some(result.to_string()); + } + + match interner.expression(&expr) { + HirExpression::Literal(literal) => match literal { + HirLiteral::Array(hir_array_literal) => { + get_global_array_value(interner, hir_array_literal, false) + } + HirLiteral::Slice(hir_array_literal) => { + get_global_array_value(interner, hir_array_literal, true) + } + HirLiteral::Bool(value) => Some(value.to_string()), + HirLiteral::Integer(field_element, _) => Some(field_element.to_string()), + HirLiteral::Str(string) => Some(format!("{:?}", string)), + HirLiteral::FmtStr(..) => None, + HirLiteral::Unit => Some("()".to_string()), + }, + HirExpression::Tuple(values) => { + get_exprs_global_value(interner, &values).map(|value| format!("({})", value)) + } + _ => None, + } +} + +fn get_global_array_value( + interner: &NodeInterner, + literal: HirArrayLiteral, + is_slice: bool, +) -> Option { + match literal { + HirArrayLiteral::Standard(values) => { + get_exprs_global_value(interner, &values).map(|value| { + if is_slice { + format!("&[{}]", value) + } else { + format!("[{}]", value) + } + }) + } + HirArrayLiteral::Repeated { repeated_element, length } => { + get_global_value(interner, repeated_element).map(|value| { + if is_slice { + format!("&[{}; {}]", value, length) + } else { + format!("[{}; {}]", value, length) + } + }) + } + } +} + +fn get_exprs_global_value(interner: &NodeInterner, exprs: &[ExprId]) -> Option { + let strings: Vec = + exprs.iter().filter_map(|value| get_global_value(interner, *value)).collect(); + if strings.len() == exprs.len() { + Some(strings.join(", ")) + } else { + None + } +} + fn format_function(id: FuncId, args: &ProcessRequestCallbackArgs) -> String { let func_meta = args.interner.function_meta(&id); let func_name_definition_id = args.interner.definition(func_meta.name.id); @@ -263,6 +347,10 @@ fn format_alias(id: TypeAliasId, args: &ProcessRequestCallbackArgs) -> String { fn format_local(id: DefinitionId, args: &ProcessRequestCallbackArgs) -> String { let definition_info = args.interner.definition(id); + if let DefinitionKind::Global(global_id) = &definition_info.kind { + return format_global(*global_id, args); + } + let DefinitionKind::Local(expr_id) = definition_info.kind else { panic!("Expected a local reference to reference a local definition") }; @@ -629,7 +717,7 @@ mod hover_tests { "two/src/lib.nr", Position { line: 15, character: 25 }, r#" one::subone - global some_global: Field"#, + global some_global: Field = 2"#, ) .await; }