From 12612bf6bf8c140bc623702359118d59c1245543 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Thu, 6 Mar 2025 17:25:15 -0300 Subject: [PATCH 01/57] Start working on a tool to show code after macro expansions --- tooling/nargo_cli/src/cli/expand_cmd.rs | 251 ++++++++++++++++++++++++ tooling/nargo_cli/src/cli/mod.rs | 3 + 2 files changed, 254 insertions(+) create mode 100644 tooling/nargo_cli/src/cli/expand_cmd.rs diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs new file mode 100644 index 00000000000..cf420c850da --- /dev/null +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -0,0 +1,251 @@ +use clap::Args; +use fm::FileManager; +use nargo::{ + errors::CompileError, insert_all_files_for_workspace_into_file_manager, package::Package, + parse_all, prepare_package, workspace::Workspace, +}; +use nargo_toml::PackageSelection; +use noirc_driver::CompileOptions; +use noirc_frontend::{ + Generics, Type, + ast::{ItemVisibility, Visibility}, + hir::{ + Context, ParsedFiles, + def_map::{CrateDefMap, ModuleDefId, ModuleId}, + }, + hir_def::{expr::HirExpression, stmt::HirPattern}, + node_interner::{FuncId, NodeInterner}, +}; + +use crate::errors::CliError; + +use super::{LockType, PackageOptions, WorkspaceCommand, check_cmd::check_crate_and_report_errors}; + +/// Expands macros +#[derive(Debug, Clone, Args)] +pub(crate) struct ExpandCommand { + #[clap(flatten)] + pub(super) package_options: PackageOptions, + + #[clap(flatten)] + compile_options: CompileOptions, +} + +impl WorkspaceCommand for ExpandCommand { + fn package_selection(&self) -> PackageSelection { + self.package_options.package_selection() + } + fn lock_type(&self) -> LockType { + // Creates a `Prover.toml` template if it doesn't exist, otherwise only writes if `allow_overwrite` is true, + // so it shouldn't lead to accidental conflicts. Doesn't produce compilation artifacts. + LockType::None + } +} + +pub(crate) fn run(args: ExpandCommand, workspace: Workspace) -> Result<(), CliError> { + let mut workspace_file_manager = workspace.new_file_manager(); + insert_all_files_for_workspace_into_file_manager(&workspace, &mut workspace_file_manager); + let parsed_files = parse_all(&workspace_file_manager); + + for package in &workspace { + check_package(&workspace_file_manager, &parsed_files, package, &args.compile_options)?; + } + + Ok(()) +} + +fn check_package( + file_manager: &FileManager, + parsed_files: &ParsedFiles, + package: &Package, + compile_options: &CompileOptions, +) -> Result<(), CompileError> { + let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); + check_crate_and_report_errors(&mut context, crate_id, compile_options)?; + + let def_map = &context.def_maps[&crate_id]; + let root_module_id = def_map.root(); + let module_id = ModuleId { krate: crate_id, local_id: root_module_id }; + + let mut string = String::new(); + show_module(module_id, &context, def_map, &mut string); + println!("{}", string); + + Ok(()) +} + +fn show_module(module_id: ModuleId, context: &Context, def_map: &CrateDefMap, string: &mut String) { + let attributes = context.def_interner.try_module_attributes(&module_id); + let name = + attributes.map(|attributes| attributes.name.clone()).unwrap_or_else(|| String::new()); + + let module_data = &def_map.modules()[module_id.local_id.0]; + let definitions = module_data.definitions(); + + for (name, scope) in definitions.types().iter().chain(definitions.values()) { + for (_trait_id, (module_def_id, visibility, _is_prelude)) in scope { + show_module_def_id(*module_def_id, *visibility, &context.def_interner, string); + } + } +} + +fn show_module_def_id( + module_def_id: ModuleDefId, + visibility: ItemVisibility, + interner: &NodeInterner, + string: &mut String, +) { + if visibility != ItemVisibility::Private { + string.push_str(&visibility.to_string()); + string.push(' '); + }; + + match module_def_id { + ModuleDefId::ModuleId(module_id) => todo!("Show modules"), + ModuleDefId::FunctionId(func_id) => show_function(func_id, interner, string), + ModuleDefId::TypeId(_) => todo!("Show types"), + ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), + ModuleDefId::TraitId(trait_id) => todo!("Show traits"), + ModuleDefId::GlobalId(global_id) => todo!("Show globals"), + } + + string.push_str("\n\n"); +} + +fn show_function(func_id: FuncId, interner: &NodeInterner, string: &mut String) { + let modifiers = interner.function_modifiers(&func_id); + let func_meta = interner.function_meta(&func_id); + let name = &modifiers.name; + + if modifiers.is_unconstrained { + string.push_str("unconstrained "); + } + if modifiers.is_comptime { + string.push_str("comptime "); + } + + string.push_str("fn "); + string.push_str(name); + + show_generics(&func_meta.direct_generics, string); + + string.push('('); + let parameters = &func_meta.parameters; + for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { + let is_self = pattern_is_self(pattern, interner); + + // `&mut self` is represented as a mutable reference type, not as a mutable pattern + if is_self && matches!(typ, Type::Reference(..)) { + string.push_str("&mut "); + } + + show_pattern(pattern, interner, string); + + // Don't add type for `self` param + if !is_self { + string.push_str(": "); + if matches!(visibility, Visibility::Public) { + string.push_str("pub "); + } + string.push_str(&format!("{}", typ)); + } + + if index != parameters.len() - 1 { + string.push_str(", "); + } + } + string.push(')'); + + let return_type = func_meta.return_type(); + match return_type { + Type::Unit => (), + _ => { + string.push_str(" -> "); + string.push_str(&format!("{}", return_type)); + } + } + + string.push(' '); + + let hir_function = interner.function(&func_id); + let block = hir_function.block(interner); + let block = HirExpression::Block(block); + let block = block.to_display_ast(interner, func_meta.location); + string.push_str(&block.to_string()); +} + +fn show_generics(generics: &Generics, string: &mut String) { + show_generics_impl( + generics, false, // only show names + string, + ); +} + +fn show_generic_names(generics: &Generics, string: &mut String) { + show_generics_impl( + generics, true, // only show names + string, + ); +} + +fn show_generics_impl(generics: &Generics, only_show_names: bool, string: &mut String) { + if generics.is_empty() { + return; + } + + string.push('<'); + for (index, generic) in generics.iter().enumerate() { + if index > 0 { + string.push_str(", "); + } + + if only_show_names { + string.push_str(&generic.name); + } else { + match generic.kind() { + noirc_frontend::Kind::Any | noirc_frontend::Kind::Normal => { + string.push_str(&generic.name); + } + noirc_frontend::Kind::IntegerOrField | noirc_frontend::Kind::Integer => { + string.push_str("let "); + string.push_str(&generic.name); + string.push_str(": u32"); + } + noirc_frontend::Kind::Numeric(typ) => { + string.push_str("let "); + string.push_str(&generic.name); + string.push_str(": "); + string.push_str(&typ.to_string()); + } + } + } + } + string.push('>'); +} + +fn show_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut String) { + match pattern { + HirPattern::Identifier(ident) => { + let definition = interner.definition(ident.id); + string.push_str(&definition.name); + } + HirPattern::Mutable(pattern, _) => { + string.push_str("mut "); + show_pattern(pattern, interner, string); + } + HirPattern::Tuple(..) | HirPattern::Struct(..) => { + string.push('_'); + } + } +} + +fn pattern_is_self(pattern: &HirPattern, interner: &NodeInterner) -> bool { + match pattern { + HirPattern::Identifier(ident) => { + let definition = interner.definition(ident.id); + definition.name == "self" + } + HirPattern::Mutable(pattern, _) => pattern_is_self(pattern, interner), + HirPattern::Tuple(..) | HirPattern::Struct(..) => false, + } +} diff --git a/tooling/nargo_cli/src/cli/mod.rs b/tooling/nargo_cli/src/cli/mod.rs index 0c644ae554c..7f001d0afc3 100644 --- a/tooling/nargo_cli/src/cli/mod.rs +++ b/tooling/nargo_cli/src/cli/mod.rs @@ -19,6 +19,7 @@ mod compile_cmd; mod dap_cmd; mod debug_cmd; mod execute_cmd; +mod expand_cmd; mod export_cmd; mod fmt_cmd; mod generate_completion_script_cmd; @@ -106,6 +107,7 @@ enum NargoCommand { Lsp(lsp_cmd::LspCommand), #[command(hide = true)] Dap(dap_cmd::DapCommand), + Expand(expand_cmd::ExpandCommand), GenerateCompletionScript(generate_completion_script_cmd::GenerateCompletionScriptCommand), } @@ -147,6 +149,7 @@ pub(crate) fn start_cli() -> eyre::Result<()> { NargoCommand::Lsp(_) => lsp_cmd::run(), NargoCommand::Dap(args) => dap_cmd::run(args), NargoCommand::Fmt(args) => with_workspace(args, config, fmt_cmd::run), + NargoCommand::Expand(args) => with_workspace(args, config, expand_cmd::run), NargoCommand::GenerateCompletionScript(args) => generate_completion_script_cmd::run(args), }?; From b7aab101ec20835d87a5f94decb8a380eefd8c00 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 10:00:44 -0300 Subject: [PATCH 02/57] Move printing code to a Printer type --- tooling/nargo_cli/src/cli/expand_cmd.rs | 179 +----------------- .../nargo_cli/src/cli/expand_cmd/printer.rs | 175 +++++++++++++++++ 2 files changed, 183 insertions(+), 171 deletions(-) create mode 100644 tooling/nargo_cli/src/cli/expand_cmd/printer.rs diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index cf420c850da..9c247020da7 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -6,21 +6,18 @@ use nargo::{ }; use nargo_toml::PackageSelection; use noirc_driver::CompileOptions; -use noirc_frontend::{ - Generics, Type, - ast::{ItemVisibility, Visibility}, - hir::{ - Context, ParsedFiles, - def_map::{CrateDefMap, ModuleDefId, ModuleId}, - }, - hir_def::{expr::HirExpression, stmt::HirPattern}, - node_interner::{FuncId, NodeInterner}, +use noirc_frontend::hir::{ + Context, ParsedFiles, + def_map::{CrateDefMap, ModuleId}, }; +use printer::Printer; use crate::errors::CliError; use super::{LockType, PackageOptions, WorkspaceCommand, check_cmd::check_crate_and_report_errors}; +mod printer; + /// Expands macros #[derive(Debug, Clone, Args)] pub(crate) struct ExpandCommand { @@ -84,168 +81,8 @@ fn show_module(module_id: ModuleId, context: &Context, def_map: &CrateDefMap, st for (name, scope) in definitions.types().iter().chain(definitions.values()) { for (_trait_id, (module_def_id, visibility, _is_prelude)) in scope { - show_module_def_id(*module_def_id, *visibility, &context.def_interner, string); - } - } -} - -fn show_module_def_id( - module_def_id: ModuleDefId, - visibility: ItemVisibility, - interner: &NodeInterner, - string: &mut String, -) { - if visibility != ItemVisibility::Private { - string.push_str(&visibility.to_string()); - string.push(' '); - }; - - match module_def_id { - ModuleDefId::ModuleId(module_id) => todo!("Show modules"), - ModuleDefId::FunctionId(func_id) => show_function(func_id, interner, string), - ModuleDefId::TypeId(_) => todo!("Show types"), - ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), - ModuleDefId::TraitId(trait_id) => todo!("Show traits"), - ModuleDefId::GlobalId(global_id) => todo!("Show globals"), - } - - string.push_str("\n\n"); -} - -fn show_function(func_id: FuncId, interner: &NodeInterner, string: &mut String) { - let modifiers = interner.function_modifiers(&func_id); - let func_meta = interner.function_meta(&func_id); - let name = &modifiers.name; - - if modifiers.is_unconstrained { - string.push_str("unconstrained "); - } - if modifiers.is_comptime { - string.push_str("comptime "); - } - - string.push_str("fn "); - string.push_str(name); - - show_generics(&func_meta.direct_generics, string); - - string.push('('); - let parameters = &func_meta.parameters; - for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { - let is_self = pattern_is_self(pattern, interner); - - // `&mut self` is represented as a mutable reference type, not as a mutable pattern - if is_self && matches!(typ, Type::Reference(..)) { - string.push_str("&mut "); - } - - show_pattern(pattern, interner, string); - - // Don't add type for `self` param - if !is_self { - string.push_str(": "); - if matches!(visibility, Visibility::Public) { - string.push_str("pub "); - } - string.push_str(&format!("{}", typ)); - } - - if index != parameters.len() - 1 { - string.push_str(", "); - } - } - string.push(')'); - - let return_type = func_meta.return_type(); - match return_type { - Type::Unit => (), - _ => { - string.push_str(" -> "); - string.push_str(&format!("{}", return_type)); - } - } - - string.push(' '); - - let hir_function = interner.function(&func_id); - let block = hir_function.block(interner); - let block = HirExpression::Block(block); - let block = block.to_display_ast(interner, func_meta.location); - string.push_str(&block.to_string()); -} - -fn show_generics(generics: &Generics, string: &mut String) { - show_generics_impl( - generics, false, // only show names - string, - ); -} - -fn show_generic_names(generics: &Generics, string: &mut String) { - show_generics_impl( - generics, true, // only show names - string, - ); -} - -fn show_generics_impl(generics: &Generics, only_show_names: bool, string: &mut String) { - if generics.is_empty() { - return; - } - - string.push('<'); - for (index, generic) in generics.iter().enumerate() { - if index > 0 { - string.push_str(", "); - } - - if only_show_names { - string.push_str(&generic.name); - } else { - match generic.kind() { - noirc_frontend::Kind::Any | noirc_frontend::Kind::Normal => { - string.push_str(&generic.name); - } - noirc_frontend::Kind::IntegerOrField | noirc_frontend::Kind::Integer => { - string.push_str("let "); - string.push_str(&generic.name); - string.push_str(": u32"); - } - noirc_frontend::Kind::Numeric(typ) => { - string.push_str("let "); - string.push_str(&generic.name); - string.push_str(": "); - string.push_str(&typ.to_string()); - } - } - } - } - string.push('>'); -} - -fn show_pattern(pattern: &HirPattern, interner: &NodeInterner, string: &mut String) { - match pattern { - HirPattern::Identifier(ident) => { - let definition = interner.definition(ident.id); - string.push_str(&definition.name); - } - HirPattern::Mutable(pattern, _) => { - string.push_str("mut "); - show_pattern(pattern, interner, string); - } - HirPattern::Tuple(..) | HirPattern::Struct(..) => { - string.push('_'); - } - } -} - -fn pattern_is_self(pattern: &HirPattern, interner: &NodeInterner) -> bool { - match pattern { - HirPattern::Identifier(ident) => { - let definition = interner.definition(ident.id); - definition.name == "self" + let mut printer = Printer::new(&context.def_interner, string); + printer.show_module_def_id(*module_def_id, *visibility); } - HirPattern::Mutable(pattern, _) => pattern_is_self(pattern, interner), - HirPattern::Tuple(..) | HirPattern::Struct(..) => false, } } diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs new file mode 100644 index 00000000000..384ae385b36 --- /dev/null +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -0,0 +1,175 @@ +use noirc_frontend::{ + Generics, Type, + ast::{ItemVisibility, Visibility}, + hir::def_map::ModuleDefId, + hir_def::{expr::HirExpression, stmt::HirPattern}, + node_interner::{FuncId, NodeInterner}, +}; + +pub(super) struct Printer<'interner, 'string> { + interner: &'interner NodeInterner, + string: &'string mut String, +} + +impl<'interner, 'string> Printer<'interner, 'string> { + pub(super) fn new(interner: &'interner NodeInterner, string: &'string mut String) -> Self { + Self { interner, string } + } + + pub(super) fn show_module_def_id( + &mut self, + module_def_id: ModuleDefId, + visibility: ItemVisibility, + ) { + if visibility != ItemVisibility::Private { + self.string.push_str(&visibility.to_string()); + self.string.push(' '); + }; + + match module_def_id { + ModuleDefId::ModuleId(module_id) => todo!("Show modules"), + ModuleDefId::FunctionId(func_id) => self.show_function(func_id), + ModuleDefId::TypeId(_) => todo!("Show types"), + ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), + ModuleDefId::TraitId(trait_id) => todo!("Show traits"), + ModuleDefId::GlobalId(global_id) => todo!("Show globals"), + } + self.string.push_str("\n\n"); + } + + fn show_function(&mut self, func_id: FuncId) { + let modifiers = self.interner.function_modifiers(&func_id); + let func_meta = self.interner.function_meta(&func_id); + let name = &modifiers.name; + + if modifiers.is_unconstrained { + self.string.push_str("unconstrained "); + } + if modifiers.is_comptime { + self.string.push_str("comptime "); + } + + self.string.push_str("fn "); + self.string.push_str(name); + + self.show_generics(&func_meta.direct_generics); + + self.string.push('('); + let parameters = &func_meta.parameters; + for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { + let is_self = self.pattern_is_self(pattern); + + // `&mut self` is represented as a mutable reference type, not as a mutable pattern + if is_self && matches!(typ, Type::Reference(..)) { + self.string.push_str("&mut "); + } + + self.show_pattern(pattern); + + // Don't add type for `self` param + if !is_self { + self.string.push_str(": "); + if matches!(visibility, Visibility::Public) { + self.string.push_str("pub "); + } + self.string.push_str(&format!("{}", typ)); + } + + if index != parameters.len() - 1 { + self.string.push_str(", "); + } + } + self.string.push(')'); + + let return_type = func_meta.return_type(); + match return_type { + Type::Unit => (), + _ => { + self.string.push_str(" -> "); + self.string.push_str(&format!("{}", return_type)); + } + } + + self.string.push(' '); + + let hir_function = self.interner.function(&func_id); + let block = hir_function.block(self.interner); + let block = HirExpression::Block(block); + let block = block.to_display_ast(self.interner, func_meta.location); + self.string.push_str(&block.to_string()); + } + + fn show_generics(&mut self, generics: &Generics) { + self.show_generics_impl( + generics, false, // only show names + ); + } + + fn show_generic_names(&mut self, generics: &Generics) { + self.show_generics_impl( + generics, true, // only show names + ); + } + + fn show_generics_impl(&mut self, generics: &Generics, only_show_names: bool) { + if generics.is_empty() { + return; + } + + self.string.push('<'); + for (index, generic) in generics.iter().enumerate() { + if index > 0 { + self.string.push_str(", "); + } + + if only_show_names { + self.string.push_str(&generic.name); + } else { + match generic.kind() { + noirc_frontend::Kind::Any | noirc_frontend::Kind::Normal => { + self.string.push_str(&generic.name); + } + noirc_frontend::Kind::IntegerOrField | noirc_frontend::Kind::Integer => { + self.string.push_str("let "); + self.string.push_str(&generic.name); + self.string.push_str(": u32"); + } + noirc_frontend::Kind::Numeric(typ) => { + self.string.push_str("let "); + self.string.push_str(&generic.name); + self.string.push_str(": "); + self.string.push_str(&typ.to_string()); + } + } + } + } + self.string.push('>'); + } + + fn show_pattern(&mut self, pattern: &HirPattern) { + match pattern { + HirPattern::Identifier(ident) => { + let definition = self.interner.definition(ident.id); + self.string.push_str(&definition.name); + } + HirPattern::Mutable(pattern, _) => { + self.string.push_str("mut "); + self.show_pattern(pattern); + } + HirPattern::Tuple(..) | HirPattern::Struct(..) => { + self.string.push('_'); + } + } + } + + fn pattern_is_self(&self, pattern: &HirPattern) -> bool { + match pattern { + HirPattern::Identifier(ident) => { + let definition = self.interner.definition(ident.id); + definition.name == "self" + } + HirPattern::Mutable(pattern, _) => self.pattern_is_self(pattern), + HirPattern::Tuple(..) | HirPattern::Struct(..) => false, + } + } +} From f87c0f4a71854d1595f174eac996dfbce37888e7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 10:13:32 -0300 Subject: [PATCH 03/57] Show structs --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 113 ++++++++++++------ 1 file changed, 76 insertions(+), 37 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 384ae385b36..ab2a17bb4c7 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1,9 +1,9 @@ use noirc_frontend::{ - Generics, Type, + DataType, Generics, Type, ast::{ItemVisibility, Visibility}, hir::def_map::ModuleDefId, hir_def::{expr::HirExpression, stmt::HirPattern}, - node_interner::{FuncId, NodeInterner}, + node_interner::{FuncId, NodeInterner, TypeId}, }; pub(super) struct Printer<'interner, 'string> { @@ -22,19 +22,50 @@ impl<'interner, 'string> Printer<'interner, 'string> { visibility: ItemVisibility, ) { if visibility != ItemVisibility::Private { - self.string.push_str(&visibility.to_string()); - self.string.push(' '); + self.push_str(&visibility.to_string()); + self.push(' '); }; match module_def_id { ModuleDefId::ModuleId(module_id) => todo!("Show modules"), - ModuleDefId::FunctionId(func_id) => self.show_function(func_id), - ModuleDefId::TypeId(_) => todo!("Show types"), + ModuleDefId::TypeId(type_id) => self.show_type(type_id), ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), ModuleDefId::TraitId(trait_id) => todo!("Show traits"), ModuleDefId::GlobalId(global_id) => todo!("Show globals"), + ModuleDefId::FunctionId(func_id) => self.show_function(func_id), + } + self.push_str("\n\n"); + } + + fn show_type(&mut self, type_id: TypeId) { + let data_type = self.interner.get_type(type_id); + let data_type = data_type.borrow(); + if data_type.is_struct() { + self.show_struct(&data_type); + } else if data_type.is_enum() { + self.show_enum(&data_type); + } else { + unreachable!("DataType should either be a struct or an enum") + } + } + + fn show_struct(&mut self, data_type: &DataType) { + self.push_str("struct "); + self.push_str(&data_type.name.to_string()); + self.show_generics(&data_type.generics); + self.push_str(" {\n"); + for field in data_type.get_fields_as_written().unwrap() { + self.push_str(" "); + self.push_str(&field.name.to_string()); + self.push_str(": "); + self.push_str(&field.typ.to_string()); + self.push_str(",\n"); } - self.string.push_str("\n\n"); + self.push('}'); + } + + fn show_enum(&mut self, data_type: &DataType) { + todo!("Show enums") } fn show_function(&mut self, func_id: FuncId) { @@ -43,60 +74,60 @@ impl<'interner, 'string> Printer<'interner, 'string> { let name = &modifiers.name; if modifiers.is_unconstrained { - self.string.push_str("unconstrained "); + self.push_str("unconstrained "); } if modifiers.is_comptime { - self.string.push_str("comptime "); + self.push_str("comptime "); } - self.string.push_str("fn "); - self.string.push_str(name); + self.push_str("fn "); + self.push_str(name); self.show_generics(&func_meta.direct_generics); - self.string.push('('); + self.push('('); let parameters = &func_meta.parameters; for (index, (pattern, typ, visibility)) in parameters.iter().enumerate() { let is_self = self.pattern_is_self(pattern); // `&mut self` is represented as a mutable reference type, not as a mutable pattern if is_self && matches!(typ, Type::Reference(..)) { - self.string.push_str("&mut "); + self.push_str("&mut "); } self.show_pattern(pattern); // Don't add type for `self` param if !is_self { - self.string.push_str(": "); + self.push_str(": "); if matches!(visibility, Visibility::Public) { - self.string.push_str("pub "); + self.push_str("pub "); } - self.string.push_str(&format!("{}", typ)); + self.push_str(&format!("{}", typ)); } if index != parameters.len() - 1 { - self.string.push_str(", "); + self.push_str(", "); } } - self.string.push(')'); + self.push(')'); let return_type = func_meta.return_type(); match return_type { Type::Unit => (), _ => { - self.string.push_str(" -> "); - self.string.push_str(&format!("{}", return_type)); + self.push_str(" -> "); + self.push_str(&format!("{}", return_type)); } } - self.string.push(' '); + self.push(' '); let hir_function = self.interner.function(&func_id); let block = hir_function.block(self.interner); let block = HirExpression::Block(block); let block = block.to_display_ast(self.interner, func_meta.location); - self.string.push_str(&block.to_string()); + self.push_str(&block.to_string()); } fn show_generics(&mut self, generics: &Generics) { @@ -116,48 +147,48 @@ impl<'interner, 'string> Printer<'interner, 'string> { return; } - self.string.push('<'); + self.push('<'); for (index, generic) in generics.iter().enumerate() { if index > 0 { - self.string.push_str(", "); + self.push_str(", "); } if only_show_names { - self.string.push_str(&generic.name); + self.push_str(&generic.name); } else { match generic.kind() { noirc_frontend::Kind::Any | noirc_frontend::Kind::Normal => { - self.string.push_str(&generic.name); + self.push_str(&generic.name); } noirc_frontend::Kind::IntegerOrField | noirc_frontend::Kind::Integer => { - self.string.push_str("let "); - self.string.push_str(&generic.name); - self.string.push_str(": u32"); + self.push_str("let "); + self.push_str(&generic.name); + self.push_str(": u32"); } noirc_frontend::Kind::Numeric(typ) => { - self.string.push_str("let "); - self.string.push_str(&generic.name); - self.string.push_str(": "); - self.string.push_str(&typ.to_string()); + self.push_str("let "); + self.push_str(&generic.name); + self.push_str(": "); + self.push_str(&typ.to_string()); } } } } - self.string.push('>'); + self.push('>'); } fn show_pattern(&mut self, pattern: &HirPattern) { match pattern { HirPattern::Identifier(ident) => { let definition = self.interner.definition(ident.id); - self.string.push_str(&definition.name); + self.push_str(&definition.name); } HirPattern::Mutable(pattern, _) => { - self.string.push_str("mut "); + self.push_str("mut "); self.show_pattern(pattern); } HirPattern::Tuple(..) | HirPattern::Struct(..) => { - self.string.push('_'); + self.push('_'); } } } @@ -172,4 +203,12 @@ impl<'interner, 'string> Printer<'interner, 'string> { HirPattern::Tuple(..) | HirPattern::Struct(..) => false, } } + + fn push_str(&mut self, str: &str) { + self.string.push_str(str); + } + + fn push(&mut self, char: char) { + self.string.push(char); + } } From f0436a2d199b9e18789a661626d43bdd997526ff Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 10:54:57 -0300 Subject: [PATCH 04/57] Also move `show_module` to printer --- tooling/nargo_cli/src/cli/expand_cmd.rs | 19 +----- .../nargo_cli/src/cli/expand_cmd/printer.rs | 59 +++++++++++++++---- 2 files changed, 48 insertions(+), 30 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index 9c247020da7..ac691b6e029 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -65,24 +65,9 @@ fn check_package( let module_id = ModuleId { krate: crate_id, local_id: root_module_id }; let mut string = String::new(); - show_module(module_id, &context, def_map, &mut string); + let mut printer = Printer::new(&context.def_interner, def_map, &mut string); + printer.show_module(module_id); println!("{}", string); Ok(()) } - -fn show_module(module_id: ModuleId, context: &Context, def_map: &CrateDefMap, string: &mut String) { - let attributes = context.def_interner.try_module_attributes(&module_id); - let name = - attributes.map(|attributes| attributes.name.clone()).unwrap_or_else(|| String::new()); - - let module_data = &def_map.modules()[module_id.local_id.0]; - let definitions = module_data.definitions(); - - for (name, scope) in definitions.types().iter().chain(definitions.values()) { - for (_trait_id, (module_def_id, visibility, _is_prelude)) in scope { - let mut printer = Printer::new(&context.def_interner, string); - printer.show_module_def_id(*module_def_id, *visibility); - } - } -} diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index ab2a17bb4c7..15a417bded9 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1,26 +1,43 @@ use noirc_frontend::{ DataType, Generics, Type, ast::{ItemVisibility, Visibility}, - hir::def_map::ModuleDefId, + hir::def_map::{CrateDefMap, ModuleDefId, ModuleId}, hir_def::{expr::HirExpression, stmt::HirPattern}, node_interner::{FuncId, NodeInterner, TypeId}, }; -pub(super) struct Printer<'interner, 'string> { +pub(super) struct Printer<'interner, 'def_map, 'string> { interner: &'interner NodeInterner, + def_map: &'def_map CrateDefMap, string: &'string mut String, + indent: usize, } -impl<'interner, 'string> Printer<'interner, 'string> { - pub(super) fn new(interner: &'interner NodeInterner, string: &'string mut String) -> Self { - Self { interner, string } +impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { + pub(super) fn new( + interner: &'interner NodeInterner, + def_map: &'def_map CrateDefMap, + string: &'string mut String, + ) -> Self { + Self { interner, def_map, string, indent: 0 } } - pub(super) fn show_module_def_id( - &mut self, - module_def_id: ModuleDefId, - visibility: ItemVisibility, - ) { + pub(super) fn show_module(&mut self, module_id: ModuleId) { + let attributes = self.interner.try_module_attributes(&module_id); + let name = + attributes.map(|attributes| attributes.name.clone()).unwrap_or_else(|| String::new()); + + let module_data = &self.def_map.modules()[module_id.local_id.0]; + let definitions = module_data.definitions(); + + for (name, scope) in definitions.types().iter().chain(definitions.values()) { + for (_trait_id, (module_def_id, visibility, _is_prelude)) in scope { + self.show_module_def_id(*module_def_id, *visibility); + } + } + } + + fn show_module_def_id(&mut self, module_def_id: ModuleDefId, visibility: ItemVisibility) { if visibility != ItemVisibility::Private { self.push_str(&visibility.to_string()); self.push(' '); @@ -38,8 +55,8 @@ impl<'interner, 'string> Printer<'interner, 'string> { } fn show_type(&mut self, type_id: TypeId) { - let data_type = self.interner.get_type(type_id); - let data_type = data_type.borrow(); + let shared_data_type = self.interner.get_type(type_id); + let data_type = shared_data_type.borrow(); if data_type.is_struct() { self.show_struct(&data_type); } else if data_type.is_enum() { @@ -54,13 +71,15 @@ impl<'interner, 'string> Printer<'interner, 'string> { self.push_str(&data_type.name.to_string()); self.show_generics(&data_type.generics); self.push_str(" {\n"); + self.increase_indent(); for field in data_type.get_fields_as_written().unwrap() { - self.push_str(" "); + self.write_indent(); self.push_str(&field.name.to_string()); self.push_str(": "); self.push_str(&field.typ.to_string()); self.push_str(",\n"); } + self.decrease_indent(); self.push('}'); } @@ -204,6 +223,20 @@ impl<'interner, 'string> Printer<'interner, 'string> { } } + fn increase_indent(&mut self) { + self.indent += 1; + } + + fn decrease_indent(&mut self) { + self.indent -= 1; + } + + fn write_indent(&mut self) { + for _ in 0..self.indent { + self.push_str(" "); + } + } + fn push_str(&mut self, str: &str) { self.string.push_str(str); } From f79d35d81f2ad8f6b8b38c29bbef56bd657d01b2 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 11:13:25 -0300 Subject: [PATCH 05/57] Nested modules and cleaner output --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 59 +++++++++++++++++-- 1 file changed, 53 insertions(+), 6 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 15a417bded9..07008e9962e 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -24,17 +24,39 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { pub(super) fn show_module(&mut self, module_id: ModuleId) { let attributes = self.interner.try_module_attributes(&module_id); - let name = - attributes.map(|attributes| attributes.name.clone()).unwrap_or_else(|| String::new()); + let name = attributes.map(|attributes| &attributes.name); + + if let Some(name) = name { + self.write_indent(); + self.push_str("mod "); + self.push_str(name); + self.push_str(" {"); + self.increase_indent(); + } let module_data = &self.def_map.modules()[module_id.local_id.0]; let definitions = module_data.definitions(); - for (name, scope) in definitions.types().iter().chain(definitions.values()) { + let mut first = true; + for (_name, scope) in definitions.types().iter().chain(definitions.values()) { for (_trait_id, (module_def_id, visibility, _is_prelude)) in scope { + if first { + self.push_str("\n"); + first = false; + } else { + self.push_str("\n\n"); + } + self.write_indent(); self.show_module_def_id(*module_def_id, *visibility); } } + + if name.is_some() { + self.push('\n'); + self.decrease_indent(); + self.write_indent(); + self.push_str("}"); + } } fn show_module_def_id(&mut self, module_def_id: ModuleDefId, visibility: ItemVisibility) { @@ -44,14 +66,15 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { }; match module_def_id { - ModuleDefId::ModuleId(module_id) => todo!("Show modules"), + ModuleDefId::ModuleId(module_id) => { + self.show_module(module_id); + } ModuleDefId::TypeId(type_id) => self.show_type(type_id), ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), ModuleDefId::TraitId(trait_id) => todo!("Show traits"), ModuleDefId::GlobalId(global_id) => todo!("Show globals"), ModuleDefId::FunctionId(func_id) => self.show_function(func_id), } - self.push_str("\n\n"); } fn show_type(&mut self, type_id: TypeId) { @@ -146,7 +169,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { let block = hir_function.block(self.interner); let block = HirExpression::Block(block); let block = block.to_display_ast(self.interner, func_meta.location); - self.push_str(&block.to_string()); + let block_str = block.to_string(); + let block_str = indent_lines(block_str, self.indent); + self.push_str(&block_str); } fn show_generics(&mut self, generics: &Generics) { @@ -245,3 +270,25 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.string.push(char); } } + +fn indent_lines(string: String, indent: usize) -> String { + if indent == 0 { + return string; + } + + let lines = string.lines(); + let lines_count = lines.clone().count(); + + lines + .enumerate() + .map(|(index, line)| { + if index == lines_count - 1 { + format!("{}{}", " ".repeat(indent), line) + } else if index == 0 { + format!("{}\n", line) + } else { + format!("{}{}\n", " ".repeat(indent), line) + } + }) + .collect() +} From 67ad4fb96a410e94d56eef512866ed01e4b4b32e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 11:25:31 -0300 Subject: [PATCH 06/57] Sort definitions --- tooling/nargo_cli/src/cli/expand_cmd.rs | 5 +- .../nargo_cli/src/cli/expand_cmd/printer.rs | 48 ++++++++++++++----- 2 files changed, 37 insertions(+), 16 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index ac691b6e029..3309089ebbc 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -6,10 +6,7 @@ use nargo::{ }; use nargo_toml::PackageSelection; use noirc_driver::CompileOptions; -use noirc_frontend::hir::{ - Context, ParsedFiles, - def_map::{CrateDefMap, ModuleId}, -}; +use noirc_frontend::hir::{ParsedFiles, def_map::ModuleId}; use printer::Printer; use crate::errors::CliError; diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 07008e9962e..a1dfae06f37 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1,9 +1,10 @@ +use noirc_errors::Location; use noirc_frontend::{ DataType, Generics, Type, ast::{ItemVisibility, Visibility}, hir::def_map::{CrateDefMap, ModuleDefId, ModuleId}, hir_def::{expr::HirExpression, stmt::HirPattern}, - node_interner::{FuncId, NodeInterner, TypeId}, + node_interner::{FuncId, NodeInterner, ReferenceId, TypeId}, }; pub(super) struct Printer<'interner, 'def_map, 'string> { @@ -37,18 +38,28 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { let module_data = &self.def_map.modules()[module_id.local_id.0]; let definitions = module_data.definitions(); - let mut first = true; - for (_name, scope) in definitions.types().iter().chain(definitions.values()) { - for (_trait_id, (module_def_id, visibility, _is_prelude)) in scope { - if first { - self.push_str("\n"); - first = false; - } else { - self.push_str("\n\n"); - } - self.write_indent(); - self.show_module_def_id(*module_def_id, *visibility); + let mut definitions = definitions + .types() + .iter() + .chain(definitions.values()) + .flat_map(|(_name, scope)| scope.values()) + .map(|(module_def_id, visibility, _is_prelude)| { + let location = self.module_def_id_location(*module_def_id); + (*module_def_id, *visibility, location) + }) + .collect::>(); + + // Make sure definitions are sorted according to location so the output is more similar to the original code + definitions.sort_by_key(|(_module_def_id, _visibility, location)| *location); + + for (index, (module_def_id, visibility, _location)) in definitions.iter().enumerate() { + if index == 0 { + self.push_str("\n"); + } else { + self.push_str("\n\n"); } + self.write_indent(); + self.show_module_def_id(*module_def_id, *visibility); } if name.is_some() { @@ -248,6 +259,19 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn module_def_id_location(&self, module_def_id: ModuleDefId) -> Location { + // We already have logic to go from a ReferenceId to a location, so we use that here + let reference_id = match module_def_id { + ModuleDefId::ModuleId(module_id) => ReferenceId::Module(module_id), + ModuleDefId::FunctionId(func_id) => ReferenceId::Function(func_id), + ModuleDefId::TypeId(type_id) => ReferenceId::Type(type_id), + ModuleDefId::TypeAliasId(type_alias_id) => ReferenceId::Alias(type_alias_id), + ModuleDefId::TraitId(trait_id) => ReferenceId::Trait(trait_id), + ModuleDefId::GlobalId(global_id) => ReferenceId::Global(global_id), + }; + self.interner.reference_location(reference_id) + } + fn increase_indent(&mut self) { self.indent += 1; } From 170ed2c3bc5fe8ecfebb9cacc57e63c74f7ac97e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 12:09:36 -0300 Subject: [PATCH 07/57] Show globals --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 84 ++++++++++++++++++- 1 file changed, 80 insertions(+), 4 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index a1dfae06f37..f35f21c8f83 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -2,9 +2,15 @@ use noirc_errors::Location; use noirc_frontend::{ DataType, Generics, Type, ast::{ItemVisibility, Visibility}, - hir::def_map::{CrateDefMap, ModuleDefId, ModuleId}, - hir_def::{expr::HirExpression, stmt::HirPattern}, - node_interner::{FuncId, NodeInterner, ReferenceId, TypeId}, + hir::{ + comptime::Value, + def_map::{CrateDefMap, ModuleDefId, ModuleId}, + }, + hir_def::{ + expr::HirExpression, + stmt::{HirLetStatement, HirPattern}, + }, + node_interner::{FuncId, GlobalId, GlobalValue, NodeInterner, ReferenceId, TypeId}, }; pub(super) struct Printer<'interner, 'def_map, 'string> { @@ -83,7 +89,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { ModuleDefId::TypeId(type_id) => self.show_type(type_id), ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), ModuleDefId::TraitId(trait_id) => todo!("Show traits"), - ModuleDefId::GlobalId(global_id) => todo!("Show globals"), + ModuleDefId::GlobalId(global_id) => self.show_global(global_id), ModuleDefId::FunctionId(func_id) => self.show_function(func_id), } } @@ -121,6 +127,31 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { todo!("Show enums") } + fn show_global(&mut self, global_id: GlobalId) { + let global_info = self.interner.get_global(global_id); + let definition_id = global_info.definition_id; + let definition = self.interner.definition(definition_id); + let typ = self.interner.definition_type(definition_id); + + if let Some(HirLetStatement { comptime: true, .. }) = + self.interner.get_global_let_statement(global_id) + { + self.push_str("comptime "); + } + if definition.mutable { + self.push_str("mut "); + } + self.push_str("global "); + self.push_str(&global_info.ident.to_string()); + self.push_str(": "); + self.push_str(&typ.to_string()); + if let GlobalValue::Resolved(value) = &global_info.value { + self.push_str(" = "); + self.show_value(value); + }; + self.push_str(";"); + } + fn show_function(&mut self, func_id: FuncId) { let modifiers = self.interner.function_modifiers(&func_id); let func_meta = self.interner.function_meta(&func_id); @@ -248,6 +279,51 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_value(&mut self, value: &Value) { + match value { + Value::Unit => self.push_str("()"), + Value::Bool(bool) => self.push_str(&bool.to_string()), + Value::Field(value) => self.push_str(&value.to_string()), + Value::I8(value) => self.push_str(&value.to_string()), + Value::I16(value) => self.push_str(&value.to_string()), + Value::I32(value) => self.push_str(&value.to_string()), + Value::I64(value) => self.push_str(&value.to_string()), + Value::U1(value) => self.push_str(&value.to_string()), + Value::U8(value) => self.push_str(&value.to_string()), + Value::U16(value) => self.push_str(&value.to_string()), + Value::U32(value) => self.push_str(&value.to_string()), + Value::U64(value) => self.push_str(&value.to_string()), + Value::U128(value) => self.push_str(&value.to_string()), + Value::String(string) => self.push_str(&format!("{:?}", string)), + Value::FormatString(_, _) => todo!("Show format string"), + Value::CtString(_) => todo!("Show CtString"), + Value::Function(func_id, _, hash_map) => todo!("Show function"), + Value::Tuple(values) => todo!("Show tuple"), + Value::Struct(hash_map, _) => todo!("Show struct"), + Value::Enum(_, values, _) => todo!("Show enum"), + Value::Array(vector, _) => todo!("Show array"), + Value::Slice(vector, _) => todo!("Show slice"), + Value::Quoted(located_tokens) => todo!("Show quoted"), + Value::Pointer(value, ..) => { + self.show_value(&value.borrow()); + } + Value::Closure(_) + | Value::StructDefinition(_) + | Value::TraitConstraint(..) + | Value::TraitDefinition(_) + | Value::TraitImpl(_) + | Value::FunctionDefinition(_) + | Value::ModuleDefinition(_) + | Value::Type(_) + | Value::Zeroed(_) + | Value::Expr(_) + | Value::TypedExpr(_) + | Value::UnresolvedType(_) => { + panic!("Theis value shouldn't be held by globals: {:?}", value) + } + } + } + fn pattern_is_self(&self, pattern: &HirPattern) -> bool { match pattern { HirPattern::Identifier(ident) => { From c364c580033da7ff1b9462681d7e66feca49d726 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 12:12:38 -0300 Subject: [PATCH 08/57] Anticipate showing types will be more complex than just `to_string()` --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index f35f21c8f83..d19a587203a 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -86,7 +86,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { ModuleDefId::ModuleId(module_id) => { self.show_module(module_id); } - ModuleDefId::TypeId(type_id) => self.show_type(type_id), + ModuleDefId::TypeId(type_id) => self.show_data_type(type_id), ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), ModuleDefId::TraitId(trait_id) => todo!("Show traits"), ModuleDefId::GlobalId(global_id) => self.show_global(global_id), @@ -94,7 +94,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } - fn show_type(&mut self, type_id: TypeId) { + fn show_data_type(&mut self, type_id: TypeId) { let shared_data_type = self.interner.get_type(type_id); let data_type = shared_data_type.borrow(); if data_type.is_struct() { @@ -116,7 +116,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.write_indent(); self.push_str(&field.name.to_string()); self.push_str(": "); - self.push_str(&field.typ.to_string()); + self.show_type(&field.typ); self.push_str(",\n"); } self.decrease_indent(); @@ -144,7 +144,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str("global "); self.push_str(&global_info.ident.to_string()); self.push_str(": "); - self.push_str(&typ.to_string()); + self.show_type(&typ); if let GlobalValue::Resolved(value) = &global_info.value { self.push_str(" = "); self.show_value(value); @@ -187,7 +187,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { if matches!(visibility, Visibility::Public) { self.push_str("pub "); } - self.push_str(&format!("{}", typ)); + self.show_type(typ); } if index != parameters.len() - 1 { @@ -201,7 +201,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Type::Unit => (), _ => { self.push_str(" -> "); - self.push_str(&format!("{}", return_type)); + self.show_type(return_type); } } @@ -255,7 +255,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str("let "); self.push_str(&generic.name); self.push_str(": "); - self.push_str(&typ.to_string()); + self.show_type(&typ); } } } @@ -324,6 +324,10 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_type(&mut self, typ: &Type) { + self.push_str(&typ.to_string()); + } + fn pattern_is_self(&self, pattern: &HirPattern) -> bool { match pattern { HirPattern::Identifier(ident) => { From 8a8516f679f5258f4c8648cdec5457fff56bd5af Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 12:16:11 -0300 Subject: [PATCH 09/57] Show type aliases --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index d19a587203a..d267c2a7197 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -10,7 +10,9 @@ use noirc_frontend::{ expr::HirExpression, stmt::{HirLetStatement, HirPattern}, }, - node_interner::{FuncId, GlobalId, GlobalValue, NodeInterner, ReferenceId, TypeId}, + node_interner::{ + FuncId, GlobalId, GlobalValue, NodeInterner, ReferenceId, TypeAliasId, TypeId, + }, }; pub(super) struct Printer<'interner, 'def_map, 'string> { @@ -87,7 +89,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_module(module_id); } ModuleDefId::TypeId(type_id) => self.show_data_type(type_id), - ModuleDefId::TypeAliasId(type_alias_id) => todo!("Show type aliases"), + ModuleDefId::TypeAliasId(type_alias_id) => self.show_type_alias(type_alias_id), ModuleDefId::TraitId(trait_id) => todo!("Show traits"), ModuleDefId::GlobalId(global_id) => self.show_global(global_id), ModuleDefId::FunctionId(func_id) => self.show_function(func_id), @@ -127,6 +129,18 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { todo!("Show enums") } + fn show_type_alias(&mut self, type_alias_id: TypeAliasId) { + let type_alias = self.interner.get_type_alias(type_alias_id); + let type_alias = type_alias.borrow(); + + self.push_str("type "); + self.push_str(&type_alias.name.to_string()); + self.show_generics(&type_alias.generics); + self.push_str(" = "); + self.show_type(&type_alias.typ); + self.push(';'); + } + fn show_global(&mut self, global_id: GlobalId) { let global_info = self.interner.get_global(global_id); let definition_id = global_info.definition_id; From 741a293466b0c71ed952f80210d8d2df257054f5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 12:59:30 -0300 Subject: [PATCH 10/57] Show traits (without methods yet) --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 93 ++++++++++++++++++- 1 file changed, 91 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index d267c2a7197..cadf6444917 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -5,13 +5,15 @@ use noirc_frontend::{ hir::{ comptime::Value, def_map::{CrateDefMap, ModuleDefId, ModuleId}, + type_check::generics::TraitGenerics, }, hir_def::{ expr::HirExpression, stmt::{HirLetStatement, HirPattern}, + traits::{ResolvedTraitBound, TraitConstraint}, }, node_interner::{ - FuncId, GlobalId, GlobalValue, NodeInterner, ReferenceId, TypeAliasId, TypeId, + FuncId, GlobalId, GlobalValue, NodeInterner, ReferenceId, TraitId, TypeAliasId, TypeId, }, }; @@ -90,7 +92,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } ModuleDefId::TypeId(type_id) => self.show_data_type(type_id), ModuleDefId::TypeAliasId(type_alias_id) => self.show_type_alias(type_alias_id), - ModuleDefId::TraitId(trait_id) => todo!("Show traits"), + ModuleDefId::TraitId(trait_id) => self.show_trait(trait_id), ModuleDefId::GlobalId(global_id) => self.show_global(global_id), ModuleDefId::FunctionId(func_id) => self.show_function(func_id), } @@ -141,6 +143,39 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push(';'); } + fn show_trait(&mut self, trait_id: TraitId) { + let trait_ = self.interner.get_trait(trait_id); + + self.push_str("trait "); + self.push_str(&trait_.name.to_string()); + self.show_generics(&trait_.generics); + + if !trait_.trait_bounds.is_empty() { + self.push_str(": "); + for (index, trait_bound) in trait_.trait_bounds.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_trait_bound(trait_bound); + } + } + + self.show_where_clause(&trait_.where_clause); + self.push_str(" {\n"); + self.increase_indent(); + + for associated_type in &trait_.associated_types { + self.write_indent(); + self.push_str("type "); + self.push_str(&associated_type.name); + self.push_str(";\n"); + } + + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } + fn show_global(&mut self, global_id: GlobalId) { let global_info = self.interner.get_global(global_id); let definition_id = global_info.definition_id; @@ -277,6 +312,60 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('>'); } + fn show_trait_generics(&mut self, generics: &TraitGenerics) { + if generics.is_empty() { + return; + } + + let mut printed_type = false; + + self.push('<'); + + for typ in &generics.ordered { + if printed_type { + self.push_str(", "); + } + + self.show_type(typ); + printed_type = true; + } + + for named_type in &generics.named { + if printed_type { + self.push_str(", "); + } + + self.push_str(&named_type.name.to_string()); + self.push_str(" = "); + self.show_type(&named_type.typ); + printed_type = true; + } + + self.push('>'); + } + + fn show_where_clause(&mut self, constraints: &[TraitConstraint]) { + if constraints.is_empty() { + return; + } + + self.push_str(" where "); + for (index, constraint) in constraints.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(&constraint.typ); + self.push_str(": "); + self.show_trait_bound(&constraint.trait_bound); + } + } + + fn show_trait_bound(&mut self, bound: &ResolvedTraitBound) { + let trait_ = self.interner.get_trait(bound.trait_id); + self.push_str(&trait_.name.to_string()); + self.show_trait_generics(&bound.trait_generics); + } + fn show_pattern(&mut self, pattern: &HirPattern) { match pattern { HirPattern::Identifier(ident) => { From 12bc89c6ac5447cfdcfd21d953d3fdc47458283d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:09:08 -0300 Subject: [PATCH 11/57] Show trait functions --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 51 +++++++++++++++---- 1 file changed, 42 insertions(+), 9 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index cadf6444917..12ac732ecbf 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -164,13 +164,43 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(" {\n"); self.increase_indent(); + let mut printed_type_or_function = false; + for associated_type in &trait_.associated_types { + if printed_type_or_function { + self.push_str("\n\n"); + } + self.write_indent(); self.push_str("type "); self.push_str(&associated_type.name); - self.push_str(";\n"); + self.push_str(";"); + printed_type_or_function = true; } + let mut func_ids = trait_ + .method_ids + .iter() + .map(|(_, func_id)| { + let location = self.interner.function_meta(func_id).location; + (func_id, location) + }) + .collect::>(); + + // Make sure functions are shown in the same order they were defined + func_ids.sort_by_key(|(_func_id, location)| *location); + + for (func_id, _location) in func_ids { + if printed_type_or_function { + self.push_str("\n\n"); + } + + self.write_indent(); + self.show_function(*func_id); + printed_type_or_function = true; + } + + self.push('\n'); self.decrease_indent(); self.write_indent(); self.push('}'); @@ -254,15 +284,18 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } - self.push(' '); - let hir_function = self.interner.function(&func_id); - let block = hir_function.block(self.interner); - let block = HirExpression::Block(block); - let block = block.to_display_ast(self.interner, func_meta.location); - let block_str = block.to_string(); - let block_str = indent_lines(block_str, self.indent); - self.push_str(&block_str); + if hir_function.try_as_expr().is_some() { + let block = hir_function.block(self.interner); + let block = HirExpression::Block(block); + let block = block.to_display_ast(self.interner, func_meta.location); + let block_str = block.to_string(); + let block_str = indent_lines(block_str, self.indent); + self.push(' '); + self.push_str(&block_str); + } else { + self.push(';'); + } } fn show_generics(&mut self, generics: &Generics) { From c4fffeb629e1b50a143d327bf2080eda308186a6 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:12:11 -0300 Subject: [PATCH 12/57] Show function where clause --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 12ac732ecbf..944239a18f0 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -284,6 +284,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + self.show_where_clause(&func_meta.trait_constraints); + let hir_function = self.interner.function(&func_id); if hir_function.try_as_expr().is_some() { let block = hir_function.block(self.interner); From 42771d803505f2bea191c465a2ced9fdab5d9fc5 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:15:59 -0300 Subject: [PATCH 13/57] Show enums --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 25 ++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 944239a18f0..74750e30d45 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -124,11 +124,34 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(",\n"); } self.decrease_indent(); + self.write_indent(); self.push('}'); } fn show_enum(&mut self, data_type: &DataType) { - todo!("Show enums") + self.push_str("enum "); + self.push_str(&data_type.name.to_string()); + self.show_generics(&data_type.generics); + self.push_str(" {\n"); + self.increase_indent(); + for variant in data_type.get_variants_as_written().unwrap() { + self.write_indent(); + self.push_str(&variant.name.to_string()); + if variant.is_function { + self.push('('); + for (index, typ) in variant.params.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(typ); + } + self.push(')'); + } + self.push_str(",\n"); + } + self.decrease_indent(); + self.write_indent(); + self.push('}'); } fn show_type_alias(&mut self, type_alias_id: TypeAliasId) { From 3ab7ac659634f0e2257114552fa428f2b4b36abe Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:17:29 -0300 Subject: [PATCH 14/57] clippy --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 24 +++++++------------ 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 74750e30d45..b1ac0348806 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -203,8 +203,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { let mut func_ids = trait_ .method_ids - .iter() - .map(|(_, func_id)| { + .values() + .map(|func_id| { let location = self.interner.function_meta(func_id).location; (func_id, location) }) @@ -329,12 +329,6 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { ); } - fn show_generic_names(&mut self, generics: &Generics) { - self.show_generics_impl( - generics, true, // only show names - ); - } - fn show_generics_impl(&mut self, generics: &Generics, only_show_names: bool) { if generics.is_empty() { return; @@ -458,13 +452,13 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Value::String(string) => self.push_str(&format!("{:?}", string)), Value::FormatString(_, _) => todo!("Show format string"), Value::CtString(_) => todo!("Show CtString"), - Value::Function(func_id, _, hash_map) => todo!("Show function"), - Value::Tuple(values) => todo!("Show tuple"), - Value::Struct(hash_map, _) => todo!("Show struct"), - Value::Enum(_, values, _) => todo!("Show enum"), - Value::Array(vector, _) => todo!("Show array"), - Value::Slice(vector, _) => todo!("Show slice"), - Value::Quoted(located_tokens) => todo!("Show quoted"), + Value::Function(..) => todo!("Show function"), + Value::Tuple(..) => todo!("Show tuple"), + Value::Struct(..) => todo!("Show struct"), + Value::Enum(..) => todo!("Show enum"), + Value::Array(..) => todo!("Show array"), + Value::Slice(..) => todo!("Show slice"), + Value::Quoted(..) => todo!("Show quoted"), Value::Pointer(value, ..) => { self.show_value(&value.borrow()); } From ea428c5d19102a552e4c0304a0ea2c76cb6ed810 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:25:50 -0300 Subject: [PATCH 15/57] Show comptime structs --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 20 ++++++++++++++++++- 1 file changed, 19 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index b1ac0348806..abfb3890e66 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -454,7 +454,25 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Value::CtString(_) => todo!("Show CtString"), Value::Function(..) => todo!("Show function"), Value::Tuple(..) => todo!("Show tuple"), - Value::Struct(..) => todo!("Show struct"), + Value::Struct(fields, typ) => { + self.show_type(typ); + if fields.is_empty() { + self.push_str(" {}"); + } else { + self.push_str(" {\n"); + self.increase_indent(); + for (name, value) in fields { + self.write_indent(); + self.push_str(name); + self.push_str(": "); + self.show_value(value); + self.push_str(",\n"); + } + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } + } Value::Enum(..) => todo!("Show enum"), Value::Array(..) => todo!("Show array"), Value::Slice(..) => todo!("Show slice"), From 9518e99989cbac3b45666c4a642d94102d52dd0b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:26:50 -0300 Subject: [PATCH 16/57] Show comptime tuples --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index abfb3890e66..18e82de524c 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -453,7 +453,16 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Value::FormatString(_, _) => todo!("Show format string"), Value::CtString(_) => todo!("Show CtString"), Value::Function(..) => todo!("Show function"), - Value::Tuple(..) => todo!("Show tuple"), + Value::Tuple(values) => { + self.push('('); + for (index, value) in values.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_value(value); + } + self.push(')'); + } Value::Struct(fields, typ) => { self.show_type(typ); if fields.is_empty() { From a36de4e9e1aeb44583bab61239e9509e559c0093 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:27:42 -0300 Subject: [PATCH 17/57] Show comptime arrays --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 18e82de524c..956d83ccab6 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -483,7 +483,16 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } Value::Enum(..) => todo!("Show enum"), - Value::Array(..) => todo!("Show array"), + Value::Array(values, _) => { + self.push('['); + for (index, value) in values.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_value(value); + } + self.push(']'); + } Value::Slice(..) => todo!("Show slice"), Value::Quoted(..) => todo!("Show quoted"), Value::Pointer(value, ..) => { From b01cc4cde1cc0339d7758f63e3032b50144c44af Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:28:30 -0300 Subject: [PATCH 18/57] Show slices --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 956d83ccab6..18ce2b84e2f 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -493,7 +493,16 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } self.push(']'); } - Value::Slice(..) => todo!("Show slice"), + Value::Slice(values, _) => { + self.push_str("&["); + for (index, value) in values.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_value(value); + } + self.push(']'); + } Value::Quoted(..) => todo!("Show quoted"), Value::Pointer(value, ..) => { self.show_value(&value.borrow()); From b8fe29b6b653f3d3d2e59306d7b9a44ba6a41864 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 13:36:51 -0300 Subject: [PATCH 19/57] mod vs contract --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 18ce2b84e2f..62ed04fe122 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -36,16 +36,21 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { pub(super) fn show_module(&mut self, module_id: ModuleId) { let attributes = self.interner.try_module_attributes(&module_id); let name = attributes.map(|attributes| &attributes.name); + let module_data = &self.def_map.modules()[module_id.local_id.0]; + let is_contract = module_data.is_contract; if let Some(name) = name { self.write_indent(); - self.push_str("mod "); + if is_contract { + self.push_str("contract "); + } else { + self.push_str("mod "); + } self.push_str(name); self.push_str(" {"); self.increase_indent(); } - let module_data = &self.def_map.modules()[module_id.local_id.0]; let definitions = module_data.definitions(); let mut definitions = definitions From af0560f42e485a7dfea3a1d389680fdc257cabbd Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 14:04:19 -0300 Subject: [PATCH 20/57] Show comptime CtString --- tooling/nargo_cli/src/cli/expand_cmd.rs | 2 +- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 13 +++++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index 3309089ebbc..6cf2e3d5fd6 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -62,7 +62,7 @@ fn check_package( let module_id = ModuleId { krate: crate_id, local_id: root_module_id }; let mut string = String::new(); - let mut printer = Printer::new(&context.def_interner, def_map, &mut string); + let mut printer = Printer::new(crate_id, &context.def_interner, def_map, &mut string); printer.show_module(module_id); println!("{}", string); diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 62ed04fe122..b622da89f89 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1,3 +1,4 @@ +use noirc_driver::CrateId; use noirc_errors::Location; use noirc_frontend::{ DataType, Generics, Type, @@ -18,6 +19,7 @@ use noirc_frontend::{ }; pub(super) struct Printer<'interner, 'def_map, 'string> { + crate_id: CrateId, interner: &'interner NodeInterner, def_map: &'def_map CrateDefMap, string: &'string mut String, @@ -26,11 +28,12 @@ pub(super) struct Printer<'interner, 'def_map, 'string> { impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { pub(super) fn new( + crate_id: CrateId, interner: &'interner NodeInterner, def_map: &'def_map CrateDefMap, string: &'string mut String, ) -> Self { - Self { interner, def_map, string, indent: 0 } + Self { crate_id, interner, def_map, string, indent: 0 } } pub(super) fn show_module(&mut self, module_id: ModuleId) { @@ -456,7 +459,13 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Value::U128(value) => self.push_str(&value.to_string()), Value::String(string) => self.push_str(&format!("{:?}", string)), Value::FormatString(_, _) => todo!("Show format string"), - Value::CtString(_) => todo!("Show CtString"), + Value::CtString(string) => { + let std = if self.crate_id.is_stdlib() { "std" } else { "crate" }; + self.push_str(&format!( + "{}::meta::ctstring::AsCtString::as_ctstring({:?})", + std, string + )); + } Value::Function(..) => todo!("Show function"), Value::Tuple(values) => { self.push('('); From b0441130cca3496f4b15d5c6516a819ba7d2db56 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 14:08:03 -0300 Subject: [PATCH 21/57] Show quoted values --- compiler/noirc_frontend/src/hir/comptime/display.rs | 2 +- compiler/noirc_frontend/src/hir/comptime/mod.rs | 1 + tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 8 ++++++-- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index af85027389e..2b2a9ecf891 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -63,7 +63,7 @@ impl Display for TokensPrettyPrinter<'_, '_> { } } -pub(super) fn tokens_to_string(tokens: &[LocatedToken], interner: &NodeInterner) -> String { +pub fn tokens_to_string(tokens: &[LocatedToken], interner: &NodeInterner) -> String { TokensPrettyPrinter { tokens, interner, indent: 0 }.to_string() } diff --git a/compiler/noirc_frontend/src/hir/comptime/mod.rs b/compiler/noirc_frontend/src/hir/comptime/mod.rs index c4a987e5419..aff1b68dfe1 100644 --- a/compiler/noirc_frontend/src/hir/comptime/mod.rs +++ b/compiler/noirc_frontend/src/hir/comptime/mod.rs @@ -5,6 +5,7 @@ mod interpreter; mod tests; mod value; +pub use display::tokens_to_string; pub use errors::{ComptimeError, InterpreterError}; pub use interpreter::Interpreter; pub use value::Value; diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index b622da89f89..a2c3416737b 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -4,7 +4,7 @@ use noirc_frontend::{ DataType, Generics, Type, ast::{ItemVisibility, Visibility}, hir::{ - comptime::Value, + comptime::{Value, tokens_to_string}, def_map::{CrateDefMap, ModuleDefId, ModuleId}, type_check::generics::TraitGenerics, }, @@ -517,7 +517,11 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } self.push(']'); } - Value::Quoted(..) => todo!("Show quoted"), + Value::Quoted(tokens) => { + self.push_str("quote { "); + self.push_str(&tokens_to_string(tokens, self.interner)); + self.push_str(" }"); + } Value::Pointer(value, ..) => { self.show_value(&value.borrow()); } From 34838171125e48316b85e006f70cbe0de73e1a01 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 14:11:30 -0300 Subject: [PATCH 22/57] Show format strings --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index a2c3416737b..66709084ab5 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -458,7 +458,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Value::U64(value) => self.push_str(&value.to_string()), Value::U128(value) => self.push_str(&value.to_string()), Value::String(string) => self.push_str(&format!("{:?}", string)), - Value::FormatString(_, _) => todo!("Show format string"), + Value::FormatString(string, _typ) => { + // Note: at this point the format string was already expanded so we can't recover the original + // interpolation and this will result in a compile-error. But... the expanded code is meant + // to be browsed, not compiled. + self.push_str(&format!("f{:?}", string)); + } Value::CtString(string) => { let std = if self.crate_id.is_stdlib() { "std" } else { "crate" }; self.push_str(&format!( From 658b51da5326040bd5f591ccd56e1880ae318f2e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 14:13:03 -0300 Subject: [PATCH 23/57] Show comptime function references --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 66709084ab5..dad001971b4 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -471,7 +471,11 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { std, string )); } - Value::Function(..) => todo!("Show function"), + Value::Function(func_id, ..) => { + // TODO: the name might need to be fully-qualified + let name = self.interner.function_name(func_id); + self.push_str(name); + } Value::Tuple(values) => { self.push('('); for (index, value) in values.iter().enumerate() { From c7620c2c88d06e118dd39901874453eed3bc7d31 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 14:20:44 -0300 Subject: [PATCH 24/57] Show comptime enum values --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 35 ++++++++++++++++++- 1 file changed, 34 insertions(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index dad001971b4..06a1b9fcec6 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -505,7 +505,40 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } } - Value::Enum(..) => todo!("Show enum"), + Value::Enum(index, args, typ) => { + let Type::DataType(data_type, generics) = typ else { + panic!("Expected typ to be a data type"); + }; + let data_type = data_type.borrow(); + + // TODO: we might need to fully-qualify this enum + self.push_str(&data_type.name.to_string()); + + if !generics.is_empty() { + self.push_str("::<"); + for (index, generic) in generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + + let variant = data_type.variant_at(*index); + self.push_str("::"); + self.push_str(&variant.name.to_string()); + if variant.is_function { + self.push('('); + for (index, arg) in args.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_value(arg); + } + self.push(')'); + } + } Value::Array(values, _) => { self.push('['); for (index, value) in values.iter().enumerate() { From 9fe171dac145b244218e382d42250923ec190b9c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 15:12:47 -0300 Subject: [PATCH 25/57] check_package -> expand_package --- tooling/nargo_cli/src/cli/expand_cmd.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index 6cf2e3d5fd6..9bb19344536 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -42,13 +42,13 @@ pub(crate) fn run(args: ExpandCommand, workspace: Workspace) -> Result<(), CliEr let parsed_files = parse_all(&workspace_file_manager); for package in &workspace { - check_package(&workspace_file_manager, &parsed_files, package, &args.compile_options)?; + expand_package(&workspace_file_manager, &parsed_files, package, &args.compile_options)?; } Ok(()) } -fn check_package( +fn expand_package( file_manager: &FileManager, parsed_files: &ParsedFiles, package: &Package, From eeb0cff2d1e643589186ddf97f562b14490db35b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 15:32:42 -0300 Subject: [PATCH 26/57] Start showing impl methods --- Cargo.lock | 1 + tooling/nargo_cli/Cargo.toml | 1 + .../nargo_cli/src/cli/expand_cmd/printer.rs | 63 ++++++++++++++++++- 3 files changed, 63 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7a64f0ec49e..69382069829 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3088,6 +3088,7 @@ dependencies = [ "prettytable-rs", "proptest", "rayon", + "rustc-hash 1.1.0", "serde", "serde_json", "sha2", diff --git a/tooling/nargo_cli/Cargo.toml b/tooling/nargo_cli/Cargo.toml index a80828356ff..33793a0d515 100644 --- a/tooling/nargo_cli/Cargo.toml +++ b/tooling/nargo_cli/Cargo.toml @@ -67,6 +67,7 @@ color-eyre.workspace = true tokio = { version = "1.0", features = ["io-std", "rt"] } dap.workspace = true clap-markdown = { git = "https://github.com/noir-lang/clap-markdown", rev = "450d759532c88f0dba70891ceecdbc9ff8f25d2b", optional = true } +rustc-hash = "1.1.0" notify = "6.1.1" notify-debouncer-full = "0.3.1" diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 06a1b9fcec6..bb2b6ab0e0e 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1,7 +1,9 @@ +use std::collections::HashMap; + use noirc_driver::CrateId; use noirc_errors::Location; use noirc_frontend::{ - DataType, Generics, Type, + DataType, Generics, Shared, Type, ast::{ItemVisibility, Visibility}, hir::{ comptime::{Value, tokens_to_string}, @@ -14,7 +16,8 @@ use noirc_frontend::{ traits::{ResolvedTraitBound, TraitConstraint}, }, node_interner::{ - FuncId, GlobalId, GlobalValue, NodeInterner, ReferenceId, TraitId, TypeAliasId, TypeId, + FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, TraitId, + TypeAliasId, TypeId, }, }; @@ -116,6 +119,13 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } else { unreachable!("DataType should either be a struct or an enum") } + drop(data_type); + + if let Some(methods) = + self.interner.get_type_methods(&Type::DataType(shared_data_type.clone(), vec![])) + { + self.show_data_type_methods(shared_data_type, methods); + } } fn show_struct(&mut self, data_type: &DataType) { @@ -162,6 +172,55 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } + fn show_data_type_methods( + &mut self, + data_type: Shared, + methods: &rustc_hash::FxHashMap, + ) { + // First split methods by impl methods and trait impl methods + let mut impl_methods = Vec::new(); + let mut trait_impl_methods = Vec::new(); + + for (_, methods) in methods { + impl_methods.extend(methods.direct.clone()); + trait_impl_methods.extend(methods.trait_impl_methods.clone()); + } + + // For impl methods, split them by the impl type. For example here we'll group + // all of `Foo` methods in one bucket, all of `Foo` in another, and + // all of `Foo` in another one. + let mut impl_methods_by_type: HashMap> = HashMap::new(); + for method in impl_methods { + impl_methods_by_type.entry(method.typ.clone()).or_default().push(method); + } + + for (typ, methods) in impl_methods_by_type { + self.push_str("\n\n"); + self.write_indent(); + self.show_impl(typ, methods); + } + } + + fn show_impl(&mut self, typ: Type, methods: Vec) { + self.push_str("impl "); + // TODO: figure out impl generics + self.show_type(&typ); + // TODO: figure out where clause + self.push_str(" {\n"); + self.increase_indent(); + for (index, method) in methods.iter().enumerate() { + if index != 0 { + self.push_str("\n\n"); + } + self.write_indent(); + self.show_function(method.method); + } + self.push('\n'); + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } + fn show_type_alias(&mut self, type_alias_id: TypeAliasId) { let type_alias = self.interner.get_type_alias(type_alias_id); let type_alias = type_alias.borrow(); From 0269ba4486ddce0f004bd3701b4f2bc0e754fcf0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 15:56:51 -0300 Subject: [PATCH 27/57] Show impl generics --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 88 ++++++++++++++++++- 1 file changed, 84 insertions(+), 4 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index bb2b6ab0e0e..c5a88e11826 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1,4 +1,4 @@ -use std::collections::HashMap; +use std::collections::{HashMap, HashSet}; use noirc_driver::CrateId; use noirc_errors::Location; @@ -202,10 +202,24 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } fn show_impl(&mut self, typ: Type, methods: Vec) { - self.push_str("impl "); - // TODO: figure out impl generics + self.push_str("impl"); + + let mut type_vars = HashSet::new(); + gather_named_type_vars(&typ, &mut type_vars); + + if !type_vars.is_empty() { + self.push('<'); + for (index, name) in type_vars.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.push_str(name); + } + self.push('>'); + } + + self.push(' '); self.show_type(&typ); - // TODO: figure out where clause self.push_str(" {\n"); self.increase_indent(); for (index, method) in methods.iter().enumerate() { @@ -715,3 +729,69 @@ fn indent_lines(string: String, indent: usize) -> String { }) .collect() } + +fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { + match typ { + Type::Array(length, typ) => { + gather_named_type_vars(length, type_vars); + gather_named_type_vars(typ, type_vars); + } + Type::Slice(typ) => { + gather_named_type_vars(typ, type_vars); + } + Type::FmtString(length, typ) => { + gather_named_type_vars(&length, type_vars); + gather_named_type_vars(&typ, type_vars); + } + Type::Tuple(types) => { + for typ in types { + gather_named_type_vars(typ, type_vars); + } + } + Type::DataType(_, generics) | Type::Alias(_, generics) => { + for typ in generics { + gather_named_type_vars(typ, type_vars); + } + } + Type::TraitAsType(_, _, trait_generics) => { + for typ in &trait_generics.ordered { + gather_named_type_vars(&typ, type_vars); + } + for named_type in &trait_generics.named { + gather_named_type_vars(&named_type.typ, type_vars); + } + } + Type::NamedGeneric(_, name) => { + type_vars.insert(name.to_string()); + } + Type::CheckedCast { from, to: _ } => { + gather_named_type_vars(&from, type_vars); + } + Type::Function(args, ret, env, _) => { + for typ in args { + gather_named_type_vars(typ, type_vars); + } + gather_named_type_vars(ret, type_vars); + gather_named_type_vars(env, type_vars); + } + Type::Reference(typ, _) => { + gather_named_type_vars(typ, type_vars); + } + Type::Forall(_, typ) => { + gather_named_type_vars(typ, type_vars); + } + Type::InfixExpr(lhs, _, rhs, _) => { + gather_named_type_vars(lhs, type_vars); + gather_named_type_vars(rhs, type_vars); + } + Type::Unit + | Type::FieldElement + | Type::Integer(..) + | Type::Bool + | Type::String(_) + | Type::Quoted(_) + | Type::Constant(..) + | Type::TypeVariable(_) + | Type::Error => (), + } +} From b8acd24192d8e6d68300313fa82c7dc5a77a6f41 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 16:00:20 -0300 Subject: [PATCH 28/57] Remove unused parameter --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index c5a88e11826..26837f7221a 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use noirc_driver::CrateId; use noirc_errors::Location; use noirc_frontend::{ - DataType, Generics, Shared, Type, + DataType, Generics, Type, ast::{ItemVisibility, Visibility}, hir::{ comptime::{Value, tokens_to_string}, @@ -124,7 +124,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { if let Some(methods) = self.interner.get_type_methods(&Type::DataType(shared_data_type.clone(), vec![])) { - self.show_data_type_methods(shared_data_type, methods); + self.show_data_type_methods(methods); } } @@ -172,11 +172,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } - fn show_data_type_methods( - &mut self, - data_type: Shared, - methods: &rustc_hash::FxHashMap, - ) { + fn show_data_type_methods(&mut self, methods: &rustc_hash::FxHashMap) { // First split methods by impl methods and trait impl methods let mut impl_methods = Vec::new(); let mut trait_impl_methods = Vec::new(); From d552c53380c92a4bdc837382ddb60bf6b0653b68 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 17:08:32 -0300 Subject: [PATCH 29/57] Show trait impls next to trait, for primitive types --- compiler/noirc_frontend/src/elaborator/mod.rs | 2 + compiler/noirc_frontend/src/hir_def/traits.rs | 2 + compiler/noirc_frontend/src/node_interner.rs | 9 ++ .../nargo_cli/src/cli/expand_cmd/printer.rs | 139 +++++++++++++++++- 4 files changed, 145 insertions(+), 7 deletions(-) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 9600fc509cb..4b386749161 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -1511,10 +1511,12 @@ impl<'context> Elaborator<'context> { let resolved_trait_impl = Shared::new(TraitImpl { ident, + location, typ: self_type.clone(), trait_id, trait_generics, file: trait_impl.file_id, + crate_id: self.crate_id, where_clause, methods, }); diff --git a/compiler/noirc_frontend/src/hir_def/traits.rs b/compiler/noirc_frontend/src/hir_def/traits.rs index a344b276913..ed39dd957cc 100644 --- a/compiler/noirc_frontend/src/hir_def/traits.rs +++ b/compiler/noirc_frontend/src/hir_def/traits.rs @@ -83,6 +83,7 @@ pub struct Trait { #[derive(Debug)] pub struct TraitImpl { pub ident: Ident, + pub location: Location, pub typ: Type, pub trait_id: TraitId, @@ -95,6 +96,7 @@ pub struct TraitImpl { pub trait_generics: Vec, pub file: FileId, + pub crate_id: CrateId, pub methods: Vec, // methods[i] is the implementation of trait.methods[i] for Type typ /// The where clause, if present, contains each trait requirement which must diff --git a/compiler/noirc_frontend/src/node_interner.rs b/compiler/noirc_frontend/src/node_interner.rs index 12ef39f6b18..165f135db42 100644 --- a/compiler/noirc_frontend/src/node_interner.rs +++ b/compiler/noirc_frontend/src/node_interner.rs @@ -1,4 +1,5 @@ use std::borrow::Cow; +use std::collections::HashSet; use std::fmt; use std::hash::Hash; use std::marker::Copy; @@ -1450,6 +1451,14 @@ impl NodeInterner { self.trait_implementations[&id].clone() } + pub fn get_trait_implementations_in_crate(&self, crate_id: CrateId) -> HashSet { + let trait_impls = self.trait_implementations.iter(); + let trait_impls = trait_impls.filter_map(|(id, trait_impl)| { + if trait_impl.borrow().crate_id == crate_id { Some(*id) } else { None } + }); + trait_impls.collect() + } + /// If the given function belongs to a trait impl, return its trait method id. /// Otherwise, return None. pub fn get_trait_method_id(&self, function: FuncId) -> Option { diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 26837f7221a..6e025fe9e83 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -17,7 +17,7 @@ use noirc_frontend::{ }, node_interner::{ FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, TraitId, - TypeAliasId, TypeId, + TraitImplId, TypeAliasId, TypeId, }, }; @@ -27,6 +27,7 @@ pub(super) struct Printer<'interner, 'def_map, 'string> { def_map: &'def_map CrateDefMap, string: &'string mut String, indent: usize, + trait_impls: HashSet, } impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { @@ -36,7 +37,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { def_map: &'def_map CrateDefMap, string: &'string mut String, ) -> Self { - Self { crate_id, interner, def_map, string, indent: 0 } + let trait_impls = interner.get_trait_implementations_in_crate(crate_id); + Self { crate_id, interner, def_map, string, indent: 0, trait_impls } } pub(super) fn show_module(&mut self, module_id: ModuleId) { @@ -103,7 +105,10 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } ModuleDefId::TypeId(type_id) => self.show_data_type(type_id), ModuleDefId::TypeAliasId(type_alias_id) => self.show_type_alias(type_alias_id), - ModuleDefId::TraitId(trait_id) => self.show_trait(trait_id), + ModuleDefId::TraitId(trait_id) => { + self.show_trait(trait_id); + self.show_trait_impls_for_trait(trait_id); + } ModuleDefId::GlobalId(global_id) => self.show_global(global_id), ModuleDefId::FunctionId(func_id) => self.show_function(func_id), } @@ -200,12 +205,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { fn show_impl(&mut self, typ: Type, methods: Vec) { self.push_str("impl"); - let mut type_vars = HashSet::new(); - gather_named_type_vars(&typ, &mut type_vars); + let mut type_var_names = HashSet::new(); + gather_named_type_vars(&typ, &mut type_var_names); - if !type_vars.is_empty() { + if !type_var_names.is_empty() { self.push('<'); - for (index, name) in type_vars.iter().enumerate() { + for (index, name) in type_var_names.iter().enumerate() { if index != 0 { self.push_str(", "); } @@ -306,6 +311,91 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } + /// Shows trait impls for traits, but only when those impls are + /// only for primitive types, or combination of primitive types + /// (like `[Field; 3]`, [T; 2], etc.) as they are likely defined next + /// to the trait. + fn show_trait_impls_for_trait(&mut self, trait_id: TraitId) { + let mut trait_impls = self + .trait_impls + .iter() + .filter_map(|trait_impl_id| { + let trait_impl = self.interner.get_trait_implementation(*trait_impl_id); + let trait_impl = trait_impl.borrow(); + if trait_impl.trait_id == trait_id && type_has_primitives_only(&trait_impl.typ) { + Some((*trait_impl_id, trait_impl.location)) + } else { + None + } + }) + .collect::>(); + + trait_impls.sort_by_key(|(_trait_impl_id, location)| *location); + + for (trait_impl, _) in trait_impls { + self.push_str("\n\n"); + self.write_indent(); + self.show_trait_impl(trait_impl); + } + } + + fn show_trait_impl(&mut self, trait_impl_id: TraitImplId) { + // Remove the trait impl from the set so we don't show it again + self.trait_impls.remove(&trait_impl_id); + + let trait_impl = self.interner.get_trait_implementation(trait_impl_id); + let trait_impl = trait_impl.borrow(); + let trait_ = self.interner.get_trait(trait_impl.trait_id); + + self.push_str("impl"); + + let mut type_var_names = HashSet::new(); + for generic in &trait_impl.trait_generics { + gather_named_type_vars(generic, &mut type_var_names); + } + gather_named_type_vars(&trait_impl.typ, &mut type_var_names); + + if !type_var_names.is_empty() { + self.push('<'); + for (index, name) in type_var_names.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.push_str(&name); + } + self.push('>'); + } + + self.push(' '); + self.push_str(&trait_.name.to_string()); + if !trait_impl.trait_generics.is_empty() { + self.push('<'); + for (index, generic) in trait_impl.trait_generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + self.push_str(" for "); + self.show_type(&trait_impl.typ); + self.show_where_clause(&trait_impl.where_clause); + self.push_str(" {\n"); + self.increase_indent(); + for (index, method) in trait_impl.methods.iter().enumerate() { + if index != 0 { + self.push_str("\n\n"); + } + self.write_indent(); + self.show_function(*method); + } + self.push('\n'); + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } + fn show_global(&mut self, global_id: GlobalId) { let global_info = self.interner.get_global(global_id); let definition_id = global_info.definition_id; @@ -791,3 +881,38 @@ fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { | Type::Error => (), } } + +fn type_has_primitives_only(typ: &Type) -> bool { + match typ { + Type::Array(length, typ) => { + type_has_primitives_only(&length) && type_has_primitives_only(&typ) + } + Type::Slice(typ) => type_has_primitives_only(typ), + Type::FmtString(length, typ) => { + type_has_primitives_only(&length) && type_has_primitives_only(typ) + } + Type::Tuple(types) => types.iter().all(type_has_primitives_only), + Type::DataType(..) | Type::Alias(..) | Type::TraitAsType(..) => false, + Type::CheckedCast { from, to: _ } => type_has_primitives_only(from), + Type::Function(args, ret, env, _) => { + args.iter().all(type_has_primitives_only) + && type_has_primitives_only(ret) + && type_has_primitives_only(env) + } + Type::Reference(typ, _) => type_has_primitives_only(typ), + Type::Forall(_, typ) => type_has_primitives_only(typ), + Type::InfixExpr(lhs, _, rhs, _) => { + type_has_primitives_only(lhs) && type_has_primitives_only(rhs) + } + Type::Unit + | Type::Bool + | Type::Integer(..) + | Type::FieldElement + | Type::String(_) + | Type::Quoted(_) + | Type::Constant(..) + | Type::TypeVariable(..) + | Type::NamedGeneric(..) + | Type::Error => true, + } +} From e2439340f813ceb1a0cc4ef230d06ed511596751 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 17:18:20 -0300 Subject: [PATCH 30/57] We actually need to show trait impls for non-primitive types too --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 102 +++++++++++------- 1 file changed, 66 insertions(+), 36 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 6e025fe9e83..fc2c665fb6b 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -322,7 +322,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { .filter_map(|trait_impl_id| { let trait_impl = self.interner.get_trait_implementation(*trait_impl_id); let trait_impl = trait_impl.borrow(); - if trait_impl.trait_id == trait_id && type_has_primitives_only(&trait_impl.typ) { + if trait_impl.trait_id == trait_id + && self.type_only_mention_types_outside_current_crate(&trait_impl.typ) + { Some((*trait_impl_id, trait_impl.location)) } else { None @@ -771,6 +773,69 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.interner.reference_location(reference_id) } + fn type_only_mention_types_outside_current_crate(&self, typ: &Type) -> bool { + match typ { + Type::Array(length, typ) => { + self.type_only_mention_types_outside_current_crate(&length) + && self.type_only_mention_types_outside_current_crate(&typ) + } + Type::Slice(typ) => self.type_only_mention_types_outside_current_crate(typ), + Type::FmtString(length, typ) => { + self.type_only_mention_types_outside_current_crate(&length) + && self.type_only_mention_types_outside_current_crate(typ) + } + Type::Tuple(types) => { + types.iter().all(|typ| self.type_only_mention_types_outside_current_crate(typ)) + } + Type::DataType(data_type, generics) => { + let data_type = data_type.borrow(); + data_type.id.krate() != self.crate_id + && generics + .iter() + .all(|typ| self.type_only_mention_types_outside_current_crate(typ)) + } + Type::Alias(_type_alias, generics) => { + // TODO: check _type_alias + generics.iter().all(|typ| self.type_only_mention_types_outside_current_crate(typ)) + } + Type::TraitAsType(trait_id, _, generics) => { + let trait_ = self.interner.get_trait(*trait_id); + trait_.id.0.krate != self.crate_id + && generics + .ordered + .iter() + .all(|typ| self.type_only_mention_types_outside_current_crate(typ)) + && generics.named.iter().all(|named_type| { + self.type_only_mention_types_outside_current_crate(&named_type.typ) + }) + } + Type::CheckedCast { from, to: _ } => { + self.type_only_mention_types_outside_current_crate(from) + } + Type::Function(args, ret, env, _) => { + args.iter().all(|typ| self.type_only_mention_types_outside_current_crate(typ)) + && self.type_only_mention_types_outside_current_crate(ret) + && self.type_only_mention_types_outside_current_crate(env) + } + Type::Reference(typ, _) => self.type_only_mention_types_outside_current_crate(typ), + Type::Forall(_, typ) => self.type_only_mention_types_outside_current_crate(typ), + Type::InfixExpr(lhs, _, rhs, _) => { + self.type_only_mention_types_outside_current_crate(lhs) + && self.type_only_mention_types_outside_current_crate(rhs) + } + Type::Unit + | Type::Bool + | Type::Integer(..) + | Type::FieldElement + | Type::String(_) + | Type::Quoted(_) + | Type::Constant(..) + | Type::TypeVariable(..) + | Type::NamedGeneric(..) + | Type::Error => true, + } + } + fn increase_indent(&mut self) { self.indent += 1; } @@ -881,38 +946,3 @@ fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { | Type::Error => (), } } - -fn type_has_primitives_only(typ: &Type) -> bool { - match typ { - Type::Array(length, typ) => { - type_has_primitives_only(&length) && type_has_primitives_only(&typ) - } - Type::Slice(typ) => type_has_primitives_only(typ), - Type::FmtString(length, typ) => { - type_has_primitives_only(&length) && type_has_primitives_only(typ) - } - Type::Tuple(types) => types.iter().all(type_has_primitives_only), - Type::DataType(..) | Type::Alias(..) | Type::TraitAsType(..) => false, - Type::CheckedCast { from, to: _ } => type_has_primitives_only(from), - Type::Function(args, ret, env, _) => { - args.iter().all(type_has_primitives_only) - && type_has_primitives_only(ret) - && type_has_primitives_only(env) - } - Type::Reference(typ, _) => type_has_primitives_only(typ), - Type::Forall(_, typ) => type_has_primitives_only(typ), - Type::InfixExpr(lhs, _, rhs, _) => { - type_has_primitives_only(lhs) && type_has_primitives_only(rhs) - } - Type::Unit - | Type::Bool - | Type::Integer(..) - | Type::FieldElement - | Type::String(_) - | Type::Quoted(_) - | Type::Constant(..) - | Type::TypeVariable(..) - | Type::NamedGeneric(..) - | Type::Error => true, - } -} From 0acf35a466f229afb39695c2db9d7be03b4215c0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 17:41:10 -0300 Subject: [PATCH 31/57] Show all trait impls --- tooling/nargo_cli/src/cli/expand_cmd.rs | 1 + .../nargo_cli/src/cli/expand_cmd/printer.rs | 97 +++++++++++++++++-- 2 files changed, 92 insertions(+), 6 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index 9bb19344536..512b452b286 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -64,6 +64,7 @@ fn expand_package( let mut string = String::new(); let mut printer = Printer::new(crate_id, &context.def_interner, def_map, &mut string); printer.show_module(module_id); + printer.show_stray_trait_impls(); println!("{}", string); Ok(()) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index fc2c665fb6b..fd2fd9b4a37 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -27,7 +27,7 @@ pub(super) struct Printer<'interner, 'def_map, 'string> { def_map: &'def_map CrateDefMap, string: &'string mut String, indent: usize, - trait_impls: HashSet, + pub(super) trait_impls: HashSet, } impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { @@ -129,8 +129,11 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { if let Some(methods) = self.interner.get_type_methods(&Type::DataType(shared_data_type.clone(), vec![])) { - self.show_data_type_methods(methods); + self.show_data_type_impls(methods); } + + let data_type = shared_data_type.borrow(); + self.show_data_type_trait_impls(&data_type); } fn show_struct(&mut self, data_type: &DataType) { @@ -177,17 +180,16 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } - fn show_data_type_methods(&mut self, methods: &rustc_hash::FxHashMap) { + fn show_data_type_impls(&mut self, methods: &rustc_hash::FxHashMap) { + // Gather all impl methods // First split methods by impl methods and trait impl methods let mut impl_methods = Vec::new(); - let mut trait_impl_methods = Vec::new(); for (_, methods) in methods { impl_methods.extend(methods.direct.clone()); - trait_impl_methods.extend(methods.trait_impl_methods.clone()); } - // For impl methods, split them by the impl type. For example here we'll group + // Split them by the impl type. For example here we'll group // all of `Foo` methods in one bucket, all of `Foo` in another, and // all of `Foo` in another one. let mut impl_methods_by_type: HashMap> = HashMap::new(); @@ -236,6 +238,30 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } + fn show_data_type_trait_impls(&mut self, data_type: &DataType) { + let mut trait_impls = self + .trait_impls + .iter() + .filter_map(|trait_impl_id| { + let trait_impl = self.interner.get_trait_implementation(*trait_impl_id); + let trait_impl = trait_impl.borrow(); + if type_mentions_data_type(&trait_impl.typ, data_type) { + Some((*trait_impl_id, trait_impl.location)) + } else { + None + } + }) + .collect::>(); + + trait_impls.sort_by_key(|(_trait_impl_id, location)| *location); + + for (trait_impl, _) in trait_impls { + self.push_str("\n\n"); + self.write_indent(); + self.show_trait_impl(trait_impl); + } + } + fn show_type_alias(&mut self, type_alias_id: TypeAliasId) { let type_alias = self.interner.get_type_alias(type_alias_id); let type_alias = type_alias.borrow(); @@ -341,6 +367,15 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + pub(super) fn show_stray_trait_impls(&mut self) { + let trait_impls = std::mem::take(&mut self.trait_impls); + for trait_impl in trait_impls { + self.push_str("\n\n"); + self.write_indent(); + self.show_trait_impl(trait_impl); + } + } + fn show_trait_impl(&mut self, trait_impl_id: TraitImplId) { // Remove the trait impl from the set so we don't show it again self.trait_impls.remove(&trait_impl_id); @@ -946,3 +981,53 @@ fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { | Type::Error => (), } } + +fn type_mentions_data_type(typ: &Type, data_type: &DataType) -> bool { + match typ { + Type::Array(length, typ) => { + type_mentions_data_type(&length, data_type) && type_mentions_data_type(&typ, data_type) + } + Type::Slice(typ) => type_mentions_data_type(typ, data_type), + Type::FmtString(length, typ) => { + type_mentions_data_type(&length, data_type) || type_mentions_data_type(typ, data_type) + } + Type::Tuple(types) => types.iter().any(|typ| type_mentions_data_type(typ, data_type)), + Type::DataType(other_data_type, generics) => { + let other_data_type = other_data_type.borrow(); + data_type.id == other_data_type.id + || generics.iter().any(|typ| type_mentions_data_type(typ, data_type)) + } + Type::Alias(_type_alias, generics) => { + // TODO: check _type_alias + generics.iter().any(|typ| type_mentions_data_type(typ, data_type)) + } + Type::TraitAsType(_, _, generics) => { + generics.ordered.iter().any(|typ| type_mentions_data_type(typ, data_type)) + || generics + .named + .iter() + .any(|named_type| type_mentions_data_type(&named_type.typ, data_type)) + } + Type::CheckedCast { from, to: _ } => type_mentions_data_type(from, data_type), + Type::Function(args, ret, env, _) => { + args.iter().any(|typ| type_mentions_data_type(typ, data_type)) + || type_mentions_data_type(ret, data_type) + || type_mentions_data_type(env, data_type) + } + Type::Reference(typ, _) => type_mentions_data_type(typ, data_type), + Type::Forall(_, typ) => type_mentions_data_type(typ, data_type), + Type::InfixExpr(lhs, _, rhs, _) => { + type_mentions_data_type(lhs, data_type) || type_mentions_data_type(rhs, data_type) + } + Type::Unit + | Type::Bool + | Type::Integer(..) + | Type::FieldElement + | Type::String(_) + | Type::Quoted(_) + | Type::Constant(..) + | Type::TypeVariable(..) + | Type::NamedGeneric(..) + | Type::Error => true, + } +} From 340f946eb4f4d550cc2806f28d2d28c36457fdb0 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 17:43:25 -0300 Subject: [PATCH 32/57] clippy --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 23 ++++++++++--------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index fd2fd9b4a37..70594501e92 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -185,13 +185,14 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { // First split methods by impl methods and trait impl methods let mut impl_methods = Vec::new(); - for (_, methods) in methods { + for methods in methods.values() { impl_methods.extend(methods.direct.clone()); } // Split them by the impl type. For example here we'll group // all of `Foo` methods in one bucket, all of `Foo` in another, and // all of `Foo` in another one. + #[allow(clippy::mutable_key_type)] let mut impl_methods_by_type: HashMap> = HashMap::new(); for method in impl_methods { impl_methods_by_type.entry(method.typ.clone()).or_default().push(method); @@ -398,7 +399,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { if index != 0 { self.push_str(", "); } - self.push_str(&name); + self.push_str(name); } self.push('>'); } @@ -811,12 +812,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { fn type_only_mention_types_outside_current_crate(&self, typ: &Type) -> bool { match typ { Type::Array(length, typ) => { - self.type_only_mention_types_outside_current_crate(&length) - && self.type_only_mention_types_outside_current_crate(&typ) + self.type_only_mention_types_outside_current_crate(length) + && self.type_only_mention_types_outside_current_crate(typ) } Type::Slice(typ) => self.type_only_mention_types_outside_current_crate(typ), Type::FmtString(length, typ) => { - self.type_only_mention_types_outside_current_crate(&length) + self.type_only_mention_types_outside_current_crate(length) && self.type_only_mention_types_outside_current_crate(typ) } Type::Tuple(types) => { @@ -926,8 +927,8 @@ fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { gather_named_type_vars(typ, type_vars); } Type::FmtString(length, typ) => { - gather_named_type_vars(&length, type_vars); - gather_named_type_vars(&typ, type_vars); + gather_named_type_vars(length, type_vars); + gather_named_type_vars(typ, type_vars); } Type::Tuple(types) => { for typ in types { @@ -941,7 +942,7 @@ fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { } Type::TraitAsType(_, _, trait_generics) => { for typ in &trait_generics.ordered { - gather_named_type_vars(&typ, type_vars); + gather_named_type_vars(typ, type_vars); } for named_type in &trait_generics.named { gather_named_type_vars(&named_type.typ, type_vars); @@ -951,7 +952,7 @@ fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { type_vars.insert(name.to_string()); } Type::CheckedCast { from, to: _ } => { - gather_named_type_vars(&from, type_vars); + gather_named_type_vars(from, type_vars); } Type::Function(args, ret, env, _) => { for typ in args { @@ -985,11 +986,11 @@ fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { fn type_mentions_data_type(typ: &Type, data_type: &DataType) -> bool { match typ { Type::Array(length, typ) => { - type_mentions_data_type(&length, data_type) && type_mentions_data_type(&typ, data_type) + type_mentions_data_type(length, data_type) && type_mentions_data_type(typ, data_type) } Type::Slice(typ) => type_mentions_data_type(typ, data_type), Type::FmtString(length, typ) => { - type_mentions_data_type(&length, data_type) || type_mentions_data_type(typ, data_type) + type_mentions_data_type(length, data_type) || type_mentions_data_type(typ, data_type) } Type::Tuple(types) => types.iter().any(|typ| type_mentions_data_type(typ, data_type)), Type::DataType(other_data_type, generics) => { From 8d783f49b3edf1498f60613c192ece8bd9b68a6c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 18:08:27 -0300 Subject: [PATCH 33/57] Apparently not all functions store a block --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 28 +++++++++++++------ 1 file changed, 20 insertions(+), 8 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 70594501e92..cedd077b2a4 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -515,14 +515,26 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_where_clause(&func_meta.trait_constraints); let hir_function = self.interner.function(&func_id); - if hir_function.try_as_expr().is_some() { - let block = hir_function.block(self.interner); - let block = HirExpression::Block(block); - let block = block.to_display_ast(self.interner, func_meta.location); - let block_str = block.to_string(); - let block_str = indent_lines(block_str, self.indent); - self.push(' '); - self.push_str(&block_str); + if let Some(expr) = hir_function.try_as_expr() { + let hir_expr = self.interner.expression(&expr); + if let HirExpression::Block(block) = hir_expr { + let block = HirExpression::Block(block); + let block = block.to_display_ast(self.interner, func_meta.location); + let block_str = block.to_string(); + let block_str = indent_lines(block_str, self.indent); + self.push(' '); + self.push_str(&block_str); + } else { + self.push_str("{\n"); + self.increase_indent(); + self.push_str( + &hir_expr.to_display_ast(self.interner, func_meta.location).to_string(), + ); + self.push('\n'); + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } } else { self.push(';'); } From faecd5a4168ee7404ee38bd0b7dc38da5418990d Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 18:08:39 -0300 Subject: [PATCH 34/57] Some comptime values are impossible to represent --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index cedd077b2a4..c88cfc4626c 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -776,6 +776,10 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Value::Pointer(value, ..) => { self.show_value(&value.borrow()); } + Value::Zeroed(_) => { + let std = if self.crate_id.is_stdlib() { "std" } else { "crate" }; + self.push_str(&format!("{std}::mem::zeroed()")); + } Value::Closure(_) | Value::StructDefinition(_) | Value::TraitConstraint(..) @@ -784,11 +788,18 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { | Value::FunctionDefinition(_) | Value::ModuleDefinition(_) | Value::Type(_) - | Value::Zeroed(_) | Value::Expr(_) | Value::TypedExpr(_) | Value::UnresolvedType(_) => { - panic!("Theis value shouldn't be held by globals: {:?}", value) + if self.crate_id.is_stdlib() { + self.push_str( + "crate::panic(f\"comptime value that cannot be represented with code\")", + ); + } else { + self.push_str( + "panic(f\"comptime value that cannot be represented with code\")", + ); + } } } } From a27c0879c1fd3727373a26fa1aef3d7a98e7d7e4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Fri, 7 Mar 2025 18:14:42 -0300 Subject: [PATCH 35/57] Fix after merge --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index c88cfc4626c..d6e93badc46 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -781,7 +781,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(&format!("{std}::mem::zeroed()")); } Value::Closure(_) - | Value::StructDefinition(_) + | Value::TypeDefinition(_) | Value::TraitConstraint(..) | Value::TraitDefinition(_) | Value::TraitImpl(_) From 4b516a27417cfffad2f321c20038c7f525873bf4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 10:34:07 -0300 Subject: [PATCH 36/57] Don't use `to_display_ast` --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 457 ++++++++++++++++-- 1 file changed, 421 insertions(+), 36 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index d6e93badc46..d52514206af 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -11,14 +11,17 @@ use noirc_frontend::{ type_check::generics::TraitGenerics, }, hir_def::{ - expr::HirExpression, - stmt::{HirLetStatement, HirPattern}, + expr::{ + HirArrayLiteral, HirBlockExpression, HirExpression, HirIdent, HirLiteral, HirMatch, + }, + stmt::{HirLValue, HirLetStatement, HirPattern, HirStatement}, traits::{ResolvedTraitBound, TraitConstraint}, }, node_interner::{ - FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, TraitId, - TraitImplId, TypeAliasId, TypeId, + ExprId, FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, + StmtId, TraitId, TraitImplId, TypeAliasId, TypeId, }, + token::FmtStrFragment, }; pub(super) struct Printer<'interner, 'def_map, 'string> { @@ -189,6 +192,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { impl_methods.extend(methods.direct.clone()); } + // Don't show enum variant functions + impl_methods.retain(|method| { + let meta = self.interner.function_meta(&method.method); + meta.enum_variant_index.is_none() + }); + // Split them by the impl type. For example here we'll group // all of `Foo` methods in one bucket, all of `Foo` in another, and // all of `Foo` in another one. @@ -517,19 +526,14 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { let hir_function = self.interner.function(&func_id); if let Some(expr) = hir_function.try_as_expr() { let hir_expr = self.interner.expression(&expr); - if let HirExpression::Block(block) = hir_expr { - let block = HirExpression::Block(block); - let block = block.to_display_ast(self.interner, func_meta.location); - let block_str = block.to_string(); - let block_str = indent_lines(block_str, self.indent); + if let HirExpression::Block(_) = &hir_expr { self.push(' '); - self.push_str(&block_str); + self.show_hir_expression(hir_expr); } else { - self.push_str("{\n"); + self.push_str(" {\n"); self.increase_indent(); - self.push_str( - &hir_expr.to_display_ast(self.interner, func_meta.location).to_string(), - ); + self.write_indent(); + self.show_hir_expression(hir_expr); self.push('\n'); self.decrease_indent(); self.write_indent(); @@ -808,6 +812,409 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(&typ.to_string()); } + fn show_hir_expression_id(&mut self, expr_id: ExprId) { + let hir_expr = self.interner.expression(&expr_id); + self.show_hir_expression(hir_expr); + } + + fn show_hir_expression(&mut self, hir_expr: HirExpression) { + match hir_expr { + HirExpression::Ident(hir_ident, generics) => { + self.show_hir_ident(hir_ident); + if let Some(generics) = generics { + self.push_str("::<"); + for (index, generic) in generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + } + HirExpression::Literal(hir_literal) => { + self.show_hir_literal(hir_literal); + } + HirExpression::Block(hir_block_expression) => { + self.show_hir_block_expression(hir_block_expression); + } + HirExpression::Prefix(hir_prefix_expression) => { + self.push_str(&hir_prefix_expression.operator.to_string()); + self.show_hir_expression_id(hir_prefix_expression.rhs); + } + HirExpression::Infix(hir_infix_expression) => { + self.show_hir_expression_id(hir_infix_expression.lhs); + self.push(' '); + self.push_str(&hir_infix_expression.operator.kind.to_string()); + self.push(' '); + self.show_hir_expression_id(hir_infix_expression.rhs); + } + HirExpression::Index(hir_index_expression) => { + self.show_hir_expression_id(hir_index_expression.collection); + self.push('['); + self.show_hir_expression_id(hir_index_expression.index); + self.push(']'); + } + HirExpression::Constructor(hir_constructor_expression) => { + // TODO: we might need to fully-qualify this name + let typ = hir_constructor_expression.r#type.borrow(); + let name = typ.name.to_string(); + self.push_str(&name); + + if !hir_constructor_expression.struct_generics.is_empty() { + self.push_str("::<"); + for (index, typ) in + hir_constructor_expression.struct_generics.iter().enumerate() + { + if index != 0 { + self.push_str(", "); + } + self.show_type(typ); + } + self.push('>'); + } + + self.push_str(" { "); + for (index, (name, value)) in hir_constructor_expression.fields.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.push_str(&name.to_string()); + self.push_str(" = "); + self.show_hir_expression_id(*value); + } + self.push('}'); + } + HirExpression::EnumConstructor(constructor) => { + // TODO: we might need to fully-qualify this name + let typ = constructor.r#type.borrow(); + let name = typ.name.to_string(); + self.push_str(&name); + + let variant = typ.variant_at(constructor.variant_index); + self.push_str("::"); + self.push_str(&variant.name.to_string()); + if variant.is_function { + self.push('('); + self.show_hir_expression_ids_separated_by_comma(&constructor.arguments); + self.push(')'); + } + } + HirExpression::MemberAccess(hir_member_access) => { + self.show_hir_expression_id(hir_member_access.lhs); + self.push('.'); + self.push_str(&hir_member_access.rhs.to_string()); + } + HirExpression::Call(hir_call_expression) => { + self.show_hir_expression_id(hir_call_expression.func); + if hir_call_expression.is_macro_call { + self.push('!'); + } + self.push('('); + self.show_hir_expression_ids_separated_by_comma(&hir_call_expression.arguments); + self.push(')'); + } + HirExpression::Constrain(hir_constrain_expression) => { + self.push_str("assert("); + self.show_hir_expression_id(hir_constrain_expression.0); + if let Some(message_id) = hir_constrain_expression.2 { + self.push_str(", "); + self.show_hir_expression_id(message_id); + } + self.push(')'); + } + HirExpression::Cast(hir_cast_expression) => { + self.show_hir_expression_id(hir_cast_expression.lhs); + self.push_str(" as "); + self.show_type(&hir_cast_expression.r#type); + } + HirExpression::If(hir_if_expression) => { + self.push_str("if "); + self.show_hir_expression_id(hir_if_expression.condition); + self.push_str(" {\n"); + self.increase_indent(); + self.write_indent(); + self.show_hir_expression_id(hir_if_expression.consequence); + self.push('\n'); + self.decrease_indent(); + self.write_indent(); + self.push('}'); + if let Some(alternative) = hir_if_expression.alternative { + self.push_str(" else {\n"); + self.increase_indent(); + self.write_indent(); + self.show_hir_expression_id(alternative); + self.push('\n'); + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } + } + HirExpression::Match(hir_match) => match hir_match { + HirMatch::Success(expr_id) => self.show_hir_expression_id(expr_id), + HirMatch::Failure { .. } => { + unreachable!("At this point code should not have errors") + } + HirMatch::Guard { .. } => todo!("hir match guard"), + HirMatch::Switch(..) => todo!("hir match switch"), + }, + HirExpression::Tuple(expr_ids) => { + let len = expr_ids.len(); + self.push('('); + self.show_hir_expression_ids_separated_by_comma(&expr_ids); + if len == 1 { + self.push(','); + } + self.push(')'); + } + HirExpression::Lambda(hir_lambda) => { + self.push('|'); + for (index, (parameter, typ)) in hir_lambda.parameters.into_iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_hir_pattern(parameter); + self.push_str(": "); + self.show_type(&typ); + } + self.push_str("| "); + if hir_lambda.return_type != Type::Unit { + self.push_str("-> "); + self.show_type(&hir_lambda.return_type); + self.push_str(" "); + } + self.show_hir_expression_id(hir_lambda.body); + } + HirExpression::Quote(tokens) => { + self.push_str("quote { "); + self.push_str(&tokens_to_string(&tokens.0, self.interner)); + self.push_str(" }"); + } + HirExpression::Comptime(hir_block_expression) => { + self.push_str("comptime "); + self.show_hir_block_expression(hir_block_expression); + } + HirExpression::Unsafe(hir_block_expression) => { + self.push_str("unsafe "); + self.show_hir_block_expression(hir_block_expression); + } + HirExpression::Error => unreachable!("error nodes should not happen"), + HirExpression::MethodCall(_) => { + todo!("method calls should not happen") + } + HirExpression::Unquote(_) => todo!("unquote should not happen"), + } + } + + fn show_hir_block_expression(&mut self, block: HirBlockExpression) { + self.push_str("{\n"); + self.increase_indent(); + for statement in block.statements { + self.write_indent(); + self.show_hir_statement_id(statement); + self.push_str("\n"); + } + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } + + fn show_hir_expression_ids_separated_by_comma(&mut self, expr_ids: &[ExprId]) { + for (index, expr_id) in expr_ids.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_hir_expression_id(*expr_id); + } + } + + fn show_hir_statement_id(&mut self, stmt_id: StmtId) { + let statement = self.interner.statement(&stmt_id); + self.show_hir_statement(statement); + } + + fn show_hir_statement(&mut self, statement: HirStatement) { + match statement { + HirStatement::Let(hir_let_statement) => { + self.push_str("let "); + self.show_hir_pattern(hir_let_statement.pattern); + self.push_str(": "); + self.show_type(&hir_let_statement.r#type); + self.push_str(" = "); + self.show_hir_expression_id(hir_let_statement.expression); + self.push(';'); + } + HirStatement::Assign(hir_assign_statement) => { + self.show_hir_lvalue(hir_assign_statement.lvalue); + self.push_str(" = "); + self.show_hir_expression_id(hir_assign_statement.expression); + } + HirStatement::For(hir_for_statement) => { + self.push_str("for "); + self.show_hir_ident(hir_for_statement.identifier); + self.push_str(" in "); + self.show_hir_expression_id(hir_for_statement.start_range); + self.push_str(".."); + self.show_hir_expression_id(hir_for_statement.end_range); + self.push(' '); + self.show_hir_expression_id(hir_for_statement.block); + } + HirStatement::Loop(expr_id) => { + self.push_str("loop "); + self.show_hir_expression_id(expr_id); + } + HirStatement::While(condition, body) => { + self.push_str("while "); + self.show_hir_expression_id(condition); + self.push(' '); + self.show_hir_expression_id(body); + } + HirStatement::Break => { + self.push_str("break;"); + } + HirStatement::Continue => { + self.push_str("continue;"); + } + HirStatement::Expression(expr_id) => { + self.show_hir_expression_id(expr_id); + } + HirStatement::Semi(expr_id) => { + self.show_hir_expression_id(expr_id); + self.push(';'); + } + HirStatement::Comptime(_) => todo!("comptime should not happen"), + HirStatement::Error => unreachable!("error should not happen"), + } + } + + fn show_hir_literal(&mut self, literal: HirLiteral) { + match literal { + HirLiteral::Array(hir_array_literal) => { + self.push_str("["); + self.show_hir_array_literal(hir_array_literal); + self.push(']'); + } + HirLiteral::Slice(hir_array_literal) => { + self.push_str("&["); + self.show_hir_array_literal(hir_array_literal); + self.push(']'); + } + HirLiteral::Bool(value) => { + self.push_str(&value.to_string()); + } + HirLiteral::Integer(signed_field) => { + self.push_str(&signed_field.to_string()); + } + HirLiteral::Str(string) => { + self.push_str(&format!("{:?}", string)); + } + HirLiteral::FmtStr(fmt_str_fragments, _expr_ids, _) => { + self.push_str("f\""); + for fragment in fmt_str_fragments { + match fragment { + FmtStrFragment::String(string) => { + // TODO: escape the string + self.push_str(&string); + } + FmtStrFragment::Interpolation(string, _) => { + // TODO: interpolate expr_id instead? + self.push('{'); + self.push_str(&string); + self.push('}'); + } + } + } + self.push('"'); + } + HirLiteral::Unit => { + self.push_str("()"); + } + } + } + + fn show_hir_array_literal(&mut self, array: HirArrayLiteral) { + match array { + HirArrayLiteral::Standard(expr_ids) => { + self.show_hir_expression_ids_separated_by_comma(&expr_ids); + } + HirArrayLiteral::Repeated { repeated_element, length } => { + self.show_hir_expression_id(repeated_element); + self.push_str("; "); + self.show_type(&length); + } + } + } + + fn show_hir_lvalue(&mut self, lvalue: HirLValue) { + match lvalue { + HirLValue::Ident(hir_ident, _) => { + self.show_hir_ident(hir_ident); + } + HirLValue::MemberAccess { object, field_name, field_index: _, typ: _, location: _ } => { + self.show_hir_lvalue(*object); + self.push('.'); + self.push_str(&field_name.to_string()); + } + HirLValue::Index { array, index, typ: _, location: _ } => { + self.show_hir_lvalue(*array); + self.push('['); + self.show_hir_expression_id(index); + self.push(']'); + } + HirLValue::Dereference { lvalue, element_type: _, location: _ } => { + self.push('*'); + self.show_hir_lvalue(*lvalue); + } + } + } + + fn show_hir_pattern(&mut self, pattern: HirPattern) { + match pattern { + HirPattern::Identifier(hir_ident) => self.show_hir_ident(hir_ident), + HirPattern::Mutable(hir_pattern, _) => { + self.push_str("mut "); + self.show_hir_pattern(*hir_pattern); + } + HirPattern::Tuple(hir_patterns, _location) => { + let len = hir_patterns.len(); + self.push('('); + for (index, pattern) in hir_patterns.into_iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_hir_pattern(pattern); + } + if len == 1 { + self.push(','); + } + self.push(')'); + } + HirPattern::Struct(typ, items, _location) => { + self.show_type(&typ); + self.push_str(" {\n"); + self.increase_indent(); + for (index, (name, pattern)) in items.into_iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.push_str(&name.to_string()); + self.push_str(": "); + self.show_hir_pattern(pattern); + } + self.push('\n'); + self.decrease_indent(); + self.write_indent(); + self.push('}'); + } + } + } + + fn show_hir_ident(&mut self, ident: HirIdent) { + // TODO: we might need to fully-qualify this name + let name = self.interner.definition_name(ident.id); + self.push_str(name); + } + fn pattern_is_self(&self, pattern: &HirPattern) -> bool { match pattern { HirPattern::Identifier(ident) => { @@ -918,28 +1325,6 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } -fn indent_lines(string: String, indent: usize) -> String { - if indent == 0 { - return string; - } - - let lines = string.lines(); - let lines_count = lines.clone().count(); - - lines - .enumerate() - .map(|(index, line)| { - if index == lines_count - 1 { - format!("{}{}", " ".repeat(indent), line) - } else if index == 0 { - format!("{}\n", line) - } else { - format!("{}{}\n", " ".repeat(indent), line) - } - }) - .collect() -} - fn gather_named_type_vars(typ: &Type, type_vars: &mut HashSet) { match typ { Type::Array(length, typ) => { From b1fdb3f37fdb6b2a2d4586c25c3a3856913f0c0c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 10:37:27 -0300 Subject: [PATCH 37/57] Better way to show if --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 16 ++-------------- 1 file changed, 2 insertions(+), 14 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index d52514206af..33f5818da92 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -931,23 +931,11 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { HirExpression::If(hir_if_expression) => { self.push_str("if "); self.show_hir_expression_id(hir_if_expression.condition); - self.push_str(" {\n"); - self.increase_indent(); - self.write_indent(); + self.push(' '); self.show_hir_expression_id(hir_if_expression.consequence); - self.push('\n'); - self.decrease_indent(); - self.write_indent(); - self.push('}'); if let Some(alternative) = hir_if_expression.alternative { - self.push_str(" else {\n"); - self.increase_indent(); - self.write_indent(); + self.push_str(" else "); self.show_hir_expression_id(alternative); - self.push('\n'); - self.decrease_indent(); - self.write_indent(); - self.push('}'); } } HirExpression::Match(hir_match) => match hir_match { From aeb3c8a1fc0c5415cb3ed06d0f4e557433649d70 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 10:38:05 -0300 Subject: [PATCH 38/57] No need for indent before mod/contract --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 33f5818da92..a24381cd26f 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -51,7 +51,6 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { let is_contract = module_data.is_contract; if let Some(name) = name { - self.write_indent(); if is_contract { self.push_str("contract "); } else { From 41e6a69997a1f4ca745f0105a471fe0034442b01 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 10:38:59 -0300 Subject: [PATCH 39/57] Remove extra spaces for `quote {}` --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index a24381cd26f..0ef8764e989 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -772,9 +772,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push(']'); } Value::Quoted(tokens) => { - self.push_str("quote { "); + self.push_str("quote {"); self.push_str(&tokens_to_string(tokens, self.interner)); - self.push_str(" }"); + self.push_str("}"); } Value::Pointer(value, ..) => { self.show_value(&value.borrow()); @@ -973,9 +973,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_hir_expression_id(hir_lambda.body); } HirExpression::Quote(tokens) => { - self.push_str("quote { "); + self.push_str("quote {"); self.push_str(&tokens_to_string(&tokens.0, self.interner)); - self.push_str(" }"); + self.push_str("}"); } HirExpression::Comptime(hir_block_expression) => { self.push_str("comptime "); From 2e3952564db1d789999e61b5bd57cc399ef1dc56 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 10:42:08 -0300 Subject: [PATCH 40/57] Show `quote {}` with correct indentation --- compiler/noirc_frontend/src/hir/comptime/display.rs | 10 +++++++++- compiler/noirc_frontend/src/hir/comptime/mod.rs | 2 +- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 10 +++++++--- 3 files changed, 17 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index 1c3698b65e5..f73d751f9d7 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -64,7 +64,15 @@ impl Display for TokensPrettyPrinter<'_, '_> { } pub fn tokens_to_string(tokens: &[LocatedToken], interner: &NodeInterner) -> String { - TokensPrettyPrinter { tokens, interner, indent: 0 }.to_string() + tokens_to_string_with_indent(tokens, 0, interner) +} + +pub fn tokens_to_string_with_indent( + tokens: &[LocatedToken], + indent: usize, + interner: &NodeInterner, +) -> String { + TokensPrettyPrinter { tokens, interner, indent }.to_string() } /// Tries to print tokens in a way that it'll be easier for the user to understand a diff --git a/compiler/noirc_frontend/src/hir/comptime/mod.rs b/compiler/noirc_frontend/src/hir/comptime/mod.rs index aff1b68dfe1..a5658ffbe2a 100644 --- a/compiler/noirc_frontend/src/hir/comptime/mod.rs +++ b/compiler/noirc_frontend/src/hir/comptime/mod.rs @@ -5,7 +5,7 @@ mod interpreter; mod tests; mod value; -pub use display::tokens_to_string; +pub use display::{tokens_to_string, tokens_to_string_with_indent}; pub use errors::{ComptimeError, InterpreterError}; pub use interpreter::Interpreter; pub use value::Value; diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 0ef8764e989..a3ac844a5bd 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -6,7 +6,7 @@ use noirc_frontend::{ DataType, Generics, Type, ast::{ItemVisibility, Visibility}, hir::{ - comptime::{Value, tokens_to_string}, + comptime::{Value, tokens_to_string_with_indent}, def_map::{CrateDefMap, ModuleDefId, ModuleId}, type_check::generics::TraitGenerics, }, @@ -773,7 +773,11 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } Value::Quoted(tokens) => { self.push_str("quote {"); - self.push_str(&tokens_to_string(tokens, self.interner)); + self.push_str(&tokens_to_string_with_indent( + tokens, + self.indent + 1, + self.interner, + )); self.push_str("}"); } Value::Pointer(value, ..) => { @@ -974,7 +978,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } HirExpression::Quote(tokens) => { self.push_str("quote {"); - self.push_str(&tokens_to_string(&tokens.0, self.interner)); + self.push_str(&tokens_to_string_with_indent(&tokens.0, self.indent, self.interner)); self.push_str("}"); } HirExpression::Comptime(hir_block_expression) => { From 7b5acba6b62e1a37e670038334be6e96550b7d28 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 10:52:46 -0300 Subject: [PATCH 41/57] Show doc comments --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 49 +++++++++++++++---- 1 file changed, 39 insertions(+), 10 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index a3ac844a5bd..b705a55ca7d 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -96,6 +96,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } fn show_module_def_id(&mut self, module_def_id: ModuleDefId, visibility: ItemVisibility) { + let reference_id = module_def_id_to_reference_id(module_def_id); + self.show_doc_comments(reference_id); + if visibility != ItemVisibility::Private { self.push_str(&visibility.to_string()); self.push(' '); @@ -116,6 +119,25 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_doc_comments(&mut self, reference_id: ReferenceId) { + let Some(doc_comments) = self.interner.doc_comments(reference_id) else { + return; + }; + + for comment in doc_comments { + if comment.contains('\n') { + self.push_str("/**"); + self.push_str(comment); + self.push_str("*/"); + } else { + self.push_str("///"); + self.push_str(comment); + } + self.push('\n'); + self.write_indent(); + } + } + fn show_data_type(&mut self, type_id: TypeId) { let shared_data_type = self.interner.get_type(type_id); let data_type = shared_data_type.borrow(); @@ -144,8 +166,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_generics(&data_type.generics); self.push_str(" {\n"); self.increase_indent(); - for field in data_type.get_fields_as_written().unwrap() { + for (index, field) in data_type.get_fields_as_written().unwrap().into_iter().enumerate() { self.write_indent(); + self.show_doc_comments(ReferenceId::StructMember(data_type.id, index)); self.push_str(&field.name.to_string()); self.push_str(": "); self.show_type(&field.typ); @@ -162,8 +185,10 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_generics(&data_type.generics); self.push_str(" {\n"); self.increase_indent(); - for variant in data_type.get_variants_as_written().unwrap() { + for (index, variant) in data_type.get_variants_as_written().unwrap().into_iter().enumerate() + { self.write_indent(); + self.show_doc_comments(ReferenceId::EnumVariant(data_type.id, index)); self.push_str(&variant.name.to_string()); if variant.is_function { self.push('('); @@ -1219,14 +1244,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { fn module_def_id_location(&self, module_def_id: ModuleDefId) -> Location { // We already have logic to go from a ReferenceId to a location, so we use that here - let reference_id = match module_def_id { - ModuleDefId::ModuleId(module_id) => ReferenceId::Module(module_id), - ModuleDefId::FunctionId(func_id) => ReferenceId::Function(func_id), - ModuleDefId::TypeId(type_id) => ReferenceId::Type(type_id), - ModuleDefId::TypeAliasId(type_alias_id) => ReferenceId::Alias(type_alias_id), - ModuleDefId::TraitId(trait_id) => ReferenceId::Trait(trait_id), - ModuleDefId::GlobalId(global_id) => ReferenceId::Global(global_id), - }; + let reference_id = module_def_id_to_reference_id(module_def_id); self.interner.reference_location(reference_id) } @@ -1431,3 +1449,14 @@ fn type_mentions_data_type(typ: &Type, data_type: &DataType) -> bool { | Type::Error => true, } } + +fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { + match module_def_id { + ModuleDefId::ModuleId(module_id) => ReferenceId::Module(module_id), + ModuleDefId::FunctionId(func_id) => ReferenceId::Function(func_id), + ModuleDefId::TypeId(type_id) => ReferenceId::Type(type_id), + ModuleDefId::TypeAliasId(type_alias_id) => ReferenceId::Alias(type_alias_id), + ModuleDefId::TraitId(trait_id) => ReferenceId::Trait(trait_id), + ModuleDefId::GlobalId(global_id) => ReferenceId::Global(global_id), + } +} From 41eed63b11249de98a59be0857eff83e522e361c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 12:38:22 -0300 Subject: [PATCH 42/57] Show imports --- compiler/noirc_frontend/src/lib.rs | 1 + compiler/noirc_frontend/src/modules.rs | 206 +++++++++++++++++ tooling/lsp/src/attribute_reference_finder.rs | 3 +- tooling/lsp/src/modules.rs | 208 +----------------- .../requests/code_action/import_or_qualify.rs | 6 +- .../src/requests/code_action/import_trait.rs | 2 +- .../src/requests/completion/auto_import.rs | 7 +- .../requests/completion/completion_items.rs | 8 +- .../lsp/src/requests/hover/from_reference.rs | 2 +- .../src/trait_impl_method_stub_generator.rs | 3 +- tooling/lsp/src/visibility.rs | 3 +- tooling/nargo_cli/src/cli/expand_cmd.rs | 7 +- .../nargo_cli/src/cli/expand_cmd/printer.rs | 143 +++++++++++- 13 files changed, 369 insertions(+), 230 deletions(-) create mode 100644 compiler/noirc_frontend/src/modules.rs diff --git a/compiler/noirc_frontend/src/lib.rs b/compiler/noirc_frontend/src/lib.rs index 88a32b2717c..164f14febec 100644 --- a/compiler/noirc_frontend/src/lib.rs +++ b/compiler/noirc_frontend/src/lib.rs @@ -18,6 +18,7 @@ pub mod elaborator; pub mod graph; pub mod lexer; pub mod locations; +pub mod modules; pub mod monomorphization; pub mod node_interner; pub mod parser; diff --git a/compiler/noirc_frontend/src/modules.rs b/compiler/noirc_frontend/src/modules.rs new file mode 100644 index 00000000000..9bb4f5e8d5b --- /dev/null +++ b/compiler/noirc_frontend/src/modules.rs @@ -0,0 +1,206 @@ +use crate::{ + ast::Ident, + graph::{CrateId, Dependency}, + hir::def_map::{ModuleDefId, ModuleId}, + node_interner::{NodeInterner, ReferenceId}, +}; + +pub fn get_parent_module(interner: &NodeInterner, module_def_id: ModuleDefId) -> Option { + let reference_id = module_def_id_to_reference_id(module_def_id); + interner.reference_module(reference_id).copied() +} + +pub fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { + match module_def_id { + ModuleDefId::ModuleId(id) => ReferenceId::Module(id), + ModuleDefId::FunctionId(id) => ReferenceId::Function(id), + ModuleDefId::TypeId(id) => ReferenceId::Type(id), + ModuleDefId::TypeAliasId(id) => ReferenceId::Alias(id), + ModuleDefId::TraitId(id) => ReferenceId::Trait(id), + ModuleDefId::GlobalId(id) => ReferenceId::Global(id), + } +} + +/// Returns the fully-qualified path of the given `ModuleDefId` relative to `current_module_id`: +/// - If `ModuleDefId` is a module, that module's path is returned +/// - Otherwise, that item's parent module's path is returned +pub fn relative_module_full_path( + module_def_id: ModuleDefId, + current_module_id: ModuleId, + current_module_parent_id: Option, + interner: &NodeInterner, +) -> Option { + let full_path; + if let ModuleDefId::ModuleId(module_id) = module_def_id { + full_path = relative_module_id_path( + module_id, + current_module_id, + current_module_parent_id, + interner, + ); + } else { + let parent_module = get_parent_module(interner, module_def_id)?; + + full_path = relative_module_id_path( + parent_module, + current_module_id, + current_module_parent_id, + interner, + ); + } + Some(full_path) +} + +/// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`. +/// Returns a relative path if possible. +pub fn relative_module_id_path( + target_module_id: ModuleId, + current_module_id: ModuleId, + current_module_parent_id: Option, + interner: &NodeInterner, +) -> String { + if Some(target_module_id) == current_module_parent_id { + return "super".to_string(); + } + + let mut segments: Vec<&str> = Vec::new(); + let mut is_relative = false; + + if let Some(module_attributes) = interner.try_module_attributes(&target_module_id) { + segments.push(&module_attributes.name); + + let mut current_attributes = module_attributes; + loop { + let Some(parent_local_id) = current_attributes.parent else { + break; + }; + + let parent_module_id = + &ModuleId { krate: target_module_id.krate, local_id: parent_local_id }; + + if current_module_id == *parent_module_id { + is_relative = true; + break; + } + + if current_module_parent_id == Some(*parent_module_id) { + segments.push("super"); + is_relative = true; + break; + } + + let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { + break; + }; + + segments.push(&parent_attributes.name); + current_attributes = parent_attributes; + } + } + + if !is_relative { + // We don't record module attributes for the root module, + // so we handle that case separately + if target_module_id.krate.is_root() { + segments.push("crate"); + } + } + + segments.reverse(); + segments.join("::") +} + +pub fn module_full_path( + module: &ModuleId, + interner: &NodeInterner, + crate_id: CrateId, + crate_name: &str, + dependencies: &Vec, +) -> String { + let mut segments: Vec = Vec::new(); + + if let Some(module_attributes) = interner.try_module_attributes(module) { + segments.push(module_attributes.name.clone()); + + let mut current_attributes = module_attributes; + loop { + let Some(parent_local_id) = current_attributes.parent else { + break; + }; + + let Some(parent_attributes) = interner.try_module_attributes(&ModuleId { + krate: module.krate, + local_id: parent_local_id, + }) else { + break; + }; + + segments.push(parent_attributes.name.clone()); + current_attributes = parent_attributes; + } + } + + // We don't record module attributes for the root module, + // so we handle that case separately + if module.krate.is_root() { + if module.krate == crate_id { + segments.push(crate_name.to_string()); + } else { + for dep in dependencies { + if dep.crate_id == crate_id { + segments.push(dep.name.to_string()); + break; + } + } + } + }; + + segments.reverse(); + segments.join("::") +} + +/// Returns the relative path to reach `module_def_id` named `name` starting from `current_module_id`. +/// +/// - `defining_module` might be `Some` if the item is reexported from another module +/// - `intermediate_name` might be `Some` if the item's parent module is reexport from another module +/// (this will be the name of the reexport) +/// +/// Returns `None` if `module_def_id` isn't visible from the current module, neither directly, neither via +/// any of its reexports (or parent module reexports). +pub fn module_def_id_relative_path( + module_def_id: ModuleDefId, + name: &str, + current_module_id: ModuleId, + current_module_parent_id: Option, + defining_module: Option, + intermediate_name: &Option, + interner: &NodeInterner, +) -> Option { + let module_path = if let Some(defining_module) = defining_module { + relative_module_id_path( + defining_module, + current_module_id, + current_module_parent_id, + interner, + ) + } else { + relative_module_full_path( + module_def_id, + current_module_id, + current_module_parent_id, + interner, + )? + }; + + let path = if defining_module.is_some() || !matches!(module_def_id, ModuleDefId::ModuleId(..)) { + if let Some(reexport_name) = &intermediate_name { + format!("{}::{}::{}", module_path, reexport_name, name) + } else { + format!("{}::{}", module_path, name) + } + } else { + module_path.clone() + }; + + Some(path) +} diff --git a/tooling/lsp/src/attribute_reference_finder.rs b/tooling/lsp/src/attribute_reference_finder.rs index 384c575f0c6..a727fe1eede 100644 --- a/tooling/lsp/src/attribute_reference_finder.rs +++ b/tooling/lsp/src/attribute_reference_finder.rs @@ -16,14 +16,13 @@ use noirc_frontend::{ def_map::{CrateDefMap, LocalModuleId, ModuleId}, resolution::import::resolve_import, }, + modules::module_def_id_to_reference_id, node_interner::ReferenceId, parser::ParsedSubModule, token::MetaAttribute, usage_tracker::UsageTracker, }; -use crate::modules::module_def_id_to_reference_id; - pub(crate) struct AttributeReferenceFinder<'a> { byte_index: usize, /// The module ID in scope. This might change as we traverse the AST diff --git a/tooling/lsp/src/modules.rs b/tooling/lsp/src/modules.rs index 4e7ef64b2c0..180e921ad9c 100644 --- a/tooling/lsp/src/modules.rs +++ b/tooling/lsp/src/modules.rs @@ -1,171 +1,15 @@ use std::collections::BTreeMap; use noirc_frontend::{ - ast::{Ident, ItemVisibility}, + ast::ItemVisibility, graph::{CrateId, Dependency}, hir::def_map::{CrateDefMap, ModuleDefId, ModuleId}, - node_interner::{NodeInterner, Reexport, ReferenceId}, + modules::get_parent_module, + node_interner::{NodeInterner, Reexport}, }; use crate::visibility::module_def_id_is_visible; -pub(crate) fn get_parent_module( - interner: &NodeInterner, - module_def_id: ModuleDefId, -) -> Option { - let reference_id = module_def_id_to_reference_id(module_def_id); - interner.reference_module(reference_id).copied() -} - -pub(crate) fn module_def_id_to_reference_id(module_def_id: ModuleDefId) -> ReferenceId { - match module_def_id { - ModuleDefId::ModuleId(id) => ReferenceId::Module(id), - ModuleDefId::FunctionId(id) => ReferenceId::Function(id), - ModuleDefId::TypeId(id) => ReferenceId::Type(id), - ModuleDefId::TypeAliasId(id) => ReferenceId::Alias(id), - ModuleDefId::TraitId(id) => ReferenceId::Trait(id), - ModuleDefId::GlobalId(id) => ReferenceId::Global(id), - } -} - -/// Returns the fully-qualified path of the given `ModuleDefId` relative to `current_module_id`: -/// - If `ModuleDefId` is a module, that module's path is returned -/// - Otherwise, that item's parent module's path is returned -pub(crate) fn relative_module_full_path( - module_def_id: ModuleDefId, - current_module_id: ModuleId, - current_module_parent_id: Option, - interner: &NodeInterner, -) -> Option { - let full_path; - if let ModuleDefId::ModuleId(module_id) = module_def_id { - full_path = relative_module_id_path( - module_id, - current_module_id, - current_module_parent_id, - interner, - ); - } else { - let parent_module = get_parent_module(interner, module_def_id)?; - - full_path = relative_module_id_path( - parent_module, - current_module_id, - current_module_parent_id, - interner, - ); - } - Some(full_path) -} - -/// Returns the path to reach an item inside `target_module_id` from inside `current_module_id`. -/// Returns a relative path if possible. -pub(crate) fn relative_module_id_path( - target_module_id: ModuleId, - current_module_id: ModuleId, - current_module_parent_id: Option, - interner: &NodeInterner, -) -> String { - if Some(target_module_id) == current_module_parent_id { - return "super".to_string(); - } - - let mut segments: Vec<&str> = Vec::new(); - let mut is_relative = false; - - if let Some(module_attributes) = interner.try_module_attributes(&target_module_id) { - segments.push(&module_attributes.name); - - let mut current_attributes = module_attributes; - loop { - let Some(parent_local_id) = current_attributes.parent else { - break; - }; - - let parent_module_id = - &ModuleId { krate: target_module_id.krate, local_id: parent_local_id }; - - if current_module_id == *parent_module_id { - is_relative = true; - break; - } - - if current_module_parent_id == Some(*parent_module_id) { - segments.push("super"); - is_relative = true; - break; - } - - let Some(parent_attributes) = interner.try_module_attributes(parent_module_id) else { - break; - }; - - segments.push(&parent_attributes.name); - current_attributes = parent_attributes; - } - } - - if !is_relative { - // We don't record module attributes for the root module, - // so we handle that case separately - if target_module_id.krate.is_root() { - segments.push("crate"); - } - } - - segments.reverse(); - segments.join("::") -} - -pub(crate) fn module_full_path( - module: &ModuleId, - interner: &NodeInterner, - crate_id: CrateId, - crate_name: &str, - dependencies: &Vec, -) -> String { - let mut segments: Vec = Vec::new(); - - if let Some(module_attributes) = interner.try_module_attributes(module) { - segments.push(module_attributes.name.clone()); - - let mut current_attributes = module_attributes; - loop { - let Some(parent_local_id) = current_attributes.parent else { - break; - }; - - let Some(parent_attributes) = interner.try_module_attributes(&ModuleId { - krate: module.krate, - local_id: parent_local_id, - }) else { - break; - }; - - segments.push(parent_attributes.name.clone()); - current_attributes = parent_attributes; - } - } - - // We don't record module attributes for the root module, - // so we handle that case separately - if module.krate.is_root() { - if module.krate == crate_id { - segments.push(crate_name.to_string()); - } else { - for dep in dependencies { - if dep.crate_id == crate_id { - segments.push(dep.name.to_string()); - break; - } - } - } - }; - - segments.reverse(); - segments.join("::") -} - /// Finds a visible reexport for any ancestor module of the given ModuleDefId, pub(crate) fn get_ancestor_module_reexport( module_def_id: ModuleDefId, @@ -222,49 +66,3 @@ pub(crate) fn get_ancestor_module_reexport( Some(grandparent_module_reexport) } - -/// Returns the relative path to reach `module_def_id` named `name` starting from `current_module_id`. -/// -/// - `defining_module` might be `Some` if the item is reexported from another module -/// - `intermediate_name` might be `Some` if the item's parent module is reexport from another module -/// (this will be the name of the reexport) -/// -/// Returns `None` if `module_def_id` isn't visible from the current module, neither directly, neither via -/// any of its reexports (or parent module reexports). -pub(crate) fn module_def_id_relative_path( - module_def_id: ModuleDefId, - name: &str, - current_module_id: ModuleId, - current_module_parent_id: Option, - defining_module: Option, - intermediate_name: &Option, - interner: &NodeInterner, -) -> Option { - let module_path = if let Some(defining_module) = defining_module { - relative_module_id_path( - defining_module, - current_module_id, - current_module_parent_id, - interner, - ) - } else { - relative_module_full_path( - module_def_id, - current_module_id, - current_module_parent_id, - interner, - )? - }; - - let path = if defining_module.is_some() || !matches!(module_def_id, ModuleDefId::ModuleId(..)) { - if let Some(reexport_name) = &intermediate_name { - format!("{}::{}::{}", module_path, reexport_name, name) - } else { - format!("{}::{}", module_path, name) - } - } else { - module_path.clone() - }; - - Some(path) -} diff --git a/tooling/lsp/src/requests/code_action/import_or_qualify.rs b/tooling/lsp/src/requests/code_action/import_or_qualify.rs index 070dcfcde90..451a200954a 100644 --- a/tooling/lsp/src/requests/code_action/import_or_qualify.rs +++ b/tooling/lsp/src/requests/code_action/import_or_qualify.rs @@ -1,10 +1,12 @@ use lsp_types::TextEdit; use noirc_errors::Location; -use noirc_frontend::ast::{Ident, Path}; +use noirc_frontend::{ + ast::{Ident, Path}, + modules::module_def_id_relative_path, +}; use crate::{ byte_span_to_range, - modules::module_def_id_relative_path, use_segment_positions::{ UseCompletionItemAdditionTextEditsRequest, use_completion_item_additional_text_edits, }, diff --git a/tooling/lsp/src/requests/code_action/import_trait.rs b/tooling/lsp/src/requests/code_action/import_trait.rs index e29558c617b..5dccef61a91 100644 --- a/tooling/lsp/src/requests/code_action/import_trait.rs +++ b/tooling/lsp/src/requests/code_action/import_trait.rs @@ -4,11 +4,11 @@ use noirc_errors::Location; use noirc_frontend::{ ast::MethodCallExpression, hir::def_map::ModuleDefId, + modules::module_def_id_relative_path, node_interner::{ReferenceId, TraitId}, }; use crate::{ - modules::module_def_id_relative_path, requests::TraitReexport, use_segment_positions::{ UseCompletionItemAdditionTextEditsRequest, use_completion_item_additional_text_edits, diff --git a/tooling/lsp/src/requests/completion/auto_import.rs b/tooling/lsp/src/requests/completion/auto_import.rs index cc80c8d8e01..8d1a4e21a5b 100644 --- a/tooling/lsp/src/requests/completion/auto_import.rs +++ b/tooling/lsp/src/requests/completion/auto_import.rs @@ -1,7 +1,10 @@ -use noirc_frontend::{ast::ItemVisibility, hir::def_map::ModuleDefId, node_interner::Reexport}; +use noirc_frontend::{ + ast::ItemVisibility, hir::def_map::ModuleDefId, modules::module_def_id_relative_path, + node_interner::Reexport, +}; use crate::{ - modules::{get_ancestor_module_reexport, module_def_id_relative_path}, + modules::get_ancestor_module_reexport, use_segment_positions::{ UseCompletionItemAdditionTextEditsRequest, use_completion_item_additional_text_edits, }, diff --git a/tooling/lsp/src/requests/completion/completion_items.rs b/tooling/lsp/src/requests/completion/completion_items.rs index 4478b975a2a..2c024b78263 100644 --- a/tooling/lsp/src/requests/completion/completion_items.rs +++ b/tooling/lsp/src/requests/completion/completion_items.rs @@ -7,14 +7,12 @@ use noirc_frontend::{ ast::AttributeTarget, hir::def_map::{ModuleDefId, ModuleId}, hir_def::{function::FuncMeta, stmt::HirPattern}, + modules::{relative_module_full_path, relative_module_id_path}, node_interner::{FuncId, GlobalId, ReferenceId, TraitId, TypeAliasId, TypeId}, }; -use crate::{ - modules::{relative_module_full_path, relative_module_id_path}, - use_segment_positions::{ - UseCompletionItemAdditionTextEditsRequest, use_completion_item_additional_text_edits, - }, +use crate::use_segment_positions::{ + UseCompletionItemAdditionTextEditsRequest, use_completion_item_additional_text_edits, }; use super::{ diff --git a/tooling/lsp/src/requests/hover/from_reference.rs b/tooling/lsp/src/requests/hover/from_reference.rs index a8ed06c0896..d0416fda879 100644 --- a/tooling/lsp/src/requests/hover/from_reference.rs +++ b/tooling/lsp/src/requests/hover/from_reference.rs @@ -11,6 +11,7 @@ use noirc_frontend::{ stmt::HirPattern, traits::Trait, }, + modules::module_full_path, node_interner::{ DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, NodeInterner, ReferenceId, TraitId, TraitImplKind, TypeAliasId, TypeId, @@ -19,7 +20,6 @@ use noirc_frontend::{ use crate::{ attribute_reference_finder::AttributeReferenceFinder, - modules::module_full_path, requests::{ProcessRequestCallbackArgs, to_lsp_location}, utils, }; diff --git a/tooling/lsp/src/trait_impl_method_stub_generator.rs b/tooling/lsp/src/trait_impl_method_stub_generator.rs index b24f4dd7d87..a0a370a0c23 100644 --- a/tooling/lsp/src/trait_impl_method_stub_generator.rs +++ b/tooling/lsp/src/trait_impl_method_stub_generator.rs @@ -9,11 +9,10 @@ use noirc_frontend::{ type_check::generics::TraitGenerics, }, hir_def::{function::FuncMeta, stmt::HirPattern, traits::Trait}, + modules::relative_module_id_path, node_interner::{FunctionModifiers, NodeInterner, ReferenceId}, }; -use crate::modules::relative_module_id_path; - pub(crate) struct TraitImplMethodStubGenerator<'a> { name: &'a str, func_meta: &'a FuncMeta, diff --git a/tooling/lsp/src/visibility.rs b/tooling/lsp/src/visibility.rs index 7366684a859..09c4f12cb34 100644 --- a/tooling/lsp/src/visibility.rs +++ b/tooling/lsp/src/visibility.rs @@ -7,11 +7,10 @@ use noirc_frontend::{ def_map::{CrateDefMap, ModuleDefId, ModuleId}, resolution::visibility::item_in_module_is_visible, }, + modules::get_parent_module, node_interner::NodeInterner, }; -use crate::modules::get_parent_module; - /// Returns true if the given ModuleDefId is visible from the current module, given its visibility. /// This will in turn check if the ModuleDefId parent modules are visible from the current module. /// If `defining_module` is Some, it will be considered as the parent of the item to check diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index 512b452b286..989184c1c29 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -55,6 +55,10 @@ fn expand_package( compile_options: &CompileOptions, ) -> Result<(), CompileError> { let (mut context, crate_id) = prepare_package(file_manager, parsed_files, package); + + // Even though this isn't LSP, we need to active this to be able to go from a ModuleDefId to its parent module + context.activate_lsp_mode(); + check_crate_and_report_errors(&mut context, crate_id, compile_options)?; let def_map = &context.def_maps[&crate_id]; @@ -62,7 +66,8 @@ fn expand_package( let module_id = ModuleId { krate: crate_id, local_id: root_module_id }; let mut string = String::new(); - let mut printer = Printer::new(crate_id, &context.def_interner, def_map, &mut string); + let mut printer = + Printer::new(crate_id, &context.def_interner, &context.def_maps, def_map, &mut string); printer.show_module(module_id); printer.show_stray_trait_impls(); println!("{}", string); diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index b705a55ca7d..de52389f2a5 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -4,10 +4,10 @@ use noirc_driver::CrateId; use noirc_errors::Location; use noirc_frontend::{ DataType, Generics, Type, - ast::{ItemVisibility, Visibility}, + ast::{Ident, ItemVisibility, Visibility}, hir::{ comptime::{Value, tokens_to_string_with_indent}, - def_map::{CrateDefMap, ModuleDefId, ModuleId}, + def_map::{CrateDefMap, DefMaps, ModuleDefId, ModuleId}, type_check::generics::TraitGenerics, }, hir_def::{ @@ -17,6 +17,7 @@ use noirc_frontend::{ stmt::{HirLValue, HirLetStatement, HirPattern, HirStatement}, traits::{ResolvedTraitBound, TraitConstraint}, }, + modules::relative_module_full_path, node_interner::{ ExprId, FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, StmtId, TraitId, TraitImplId, TypeAliasId, TypeId, @@ -27,9 +28,12 @@ use noirc_frontend::{ pub(super) struct Printer<'interner, 'def_map, 'string> { crate_id: CrateId, interner: &'interner NodeInterner, + def_maps: &'def_map DefMaps, def_map: &'def_map CrateDefMap, string: &'string mut String, indent: usize, + module_id: ModuleId, + imports: HashMap, pub(super) trait_impls: HashSet, } @@ -37,11 +41,24 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { pub(super) fn new( crate_id: CrateId, interner: &'interner NodeInterner, + def_maps: &'def_map DefMaps, def_map: &'def_map CrateDefMap, string: &'string mut String, ) -> Self { + let module_id = ModuleId { krate: crate_id, local_id: def_map.root() }; let trait_impls = interner.get_trait_implementations_in_crate(crate_id); - Self { crate_id, interner, def_map, string, indent: 0, trait_impls } + let imports = HashMap::new(); + Self { + crate_id, + interner, + def_maps, + def_map, + string, + indent: 0, + module_id, + imports, + trait_impls, + } } pub(super) fn show_module(&mut self, module_id: ModuleId) { @@ -61,6 +78,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.increase_indent(); } + let previous_module_id = self.module_id; + self.module_id = module_id; + + let previous_imports = std::mem::take(&mut self.imports); + self.imports = HashMap::new(); + let definitions = module_data.definitions(); let mut definitions = definitions @@ -77,6 +100,34 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { // Make sure definitions are sorted according to location so the output is more similar to the original code definitions.sort_by_key(|(_module_def_id, _visibility, location)| *location); + // Gather all ModuleDefId's for definitions so we can exclude them when we'll list imports now + let definitions_module_def_ids = + definitions.iter().map(|(module_def_id, ..)| *module_def_id).collect::>(); + + let scope = module_data.scope(); + let mut scope = scope + .types() + .iter() + .chain(scope.values()) + .flat_map(|(name, scope)| scope.values().map(|value| (name.clone(), value))) + .filter_map(|(name, (module_def_id, visibility, is_prelude))| { + if !definitions_module_def_ids.contains(module_def_id) { + Some((name, *module_def_id, *visibility, *is_prelude)) + } else { + None + } + }) + .collect::>(); + + scope.sort_by_key(|(name, ..)| name.location()); + + self.imports = scope + .iter() + .map(|(name, module_def_id, ..)| (*module_def_id, name.clone())) + .collect::>(); + + self.show_imports(scope); + for (index, (module_def_id, visibility, _location)) in definitions.iter().enumerate() { if index == 0 { self.push_str("\n"); @@ -87,6 +138,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_module_def_id(*module_def_id, *visibility); } + self.module_id = previous_module_id; + self.imports = previous_imports; + if name.is_some() { self.push('\n'); self.decrease_indent(); @@ -99,10 +153,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { let reference_id = module_def_id_to_reference_id(module_def_id); self.show_doc_comments(reference_id); - if visibility != ItemVisibility::Private { - self.push_str(&visibility.to_string()); - self.push(' '); - }; + self.show_item_visibility(visibility); match module_def_id { ModuleDefId::ModuleId(module_id) => { @@ -138,6 +189,13 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_item_visibility(&mut self, visibility: ItemVisibility) { + if visibility != ItemVisibility::Private { + self.push_str(&visibility.to_string()); + self.push(' '); + }; + } + fn show_data_type(&mut self, type_id: TypeId) { let shared_data_type = self.interner.get_type(type_id); let data_type = shared_data_type.borrow(); @@ -836,6 +894,48 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_imports( + &mut self, + imports: Vec<(Ident, ModuleDefId, ItemVisibility, bool /* is prelude */)>, + ) { + let current_module_parent_id = self.module_id.parent(self.def_maps); + + let mut first = true; + + for (alias, module_def_id, visibility, is_prelude) in imports { + if is_prelude { + continue; + } + + if first { + self.push('\n'); + first = false; + } + self.write_indent(); + self.show_item_visibility(visibility); + self.push_str("use "); + if let Some(full_path) = relative_module_full_path( + module_def_id, + self.module_id, + current_module_parent_id, + self.interner, + ) { + self.push_str(&full_path); + self.push_str("::"); + }; + + let name = self.module_def_id_name(module_def_id); + self.push_str(&name); + + if name != alias.0.contents { + self.push_str(" as "); + self.push_str(&alias.to_string()); + } + self.push(';'); + self.push('\n'); + } + } + fn show_type(&mut self, typ: &Type) { self.push_str(&typ.to_string()); } @@ -1248,6 +1348,35 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.interner.reference_location(reference_id) } + fn module_def_id_name(&self, module_def_id: ModuleDefId) -> String { + match module_def_id { + ModuleDefId::ModuleId(module_id) => { + let attributes = self.interner.try_module_attributes(&module_id); + let name = attributes.map(|attributes| &attributes.name); + name.cloned().unwrap_or_else(String::new) + } + ModuleDefId::FunctionId(func_id) => self.interner.function_name(&func_id).to_string(), + ModuleDefId::TypeId(type_id) => { + let data_type = self.interner.get_type(type_id); + let data_type = data_type.borrow(); + data_type.name.to_string() + } + ModuleDefId::TypeAliasId(type_alias_id) => { + let type_alias = self.interner.get_type_alias(type_alias_id); + let type_alias = type_alias.borrow(); + type_alias.name.to_string() + } + ModuleDefId::TraitId(trait_id) => { + let trait_ = self.interner.get_trait(trait_id); + trait_.name.to_string() + } + ModuleDefId::GlobalId(global_id) => { + let global_info = self.interner.get_global(global_id); + global_info.ident.to_string() + } + } + } + fn type_only_mention_types_outside_current_crate(&self, typ: &Type) -> bool { match typ { Type::Array(length, typ) => { From 71b98e1bb90c629519968941ae0ce2a351189df7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 12:57:54 -0300 Subject: [PATCH 43/57] Some fixes --- compiler/noirc_frontend/src/ast/mod.rs | 4 +- .../nargo_cli/src/cli/expand_cmd/printer.rs | 68 ++++++++++++------- 2 files changed, 44 insertions(+), 28 deletions(-) diff --git a/compiler/noirc_frontend/src/ast/mod.rs b/compiler/noirc_frontend/src/ast/mod.rs index f39832e22b3..80756de297f 100644 --- a/compiler/noirc_frontend/src/ast/mod.rs +++ b/compiler/noirc_frontend/src/ast/mod.rs @@ -607,8 +607,8 @@ impl std::fmt::Display for Visibility { match self { Self::Public => write!(f, "pub"), Self::Private => write!(f, "priv"), - Self::CallData(id) => write!(f, "calldata{id}"), - Self::ReturnData => write!(f, "returndata"), + Self::CallData(id) => write!(f, "call_data({id})"), + Self::ReturnData => write!(f, "return_data"), } } } diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index de52389f2a5..256a3ca19db 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -196,6 +196,13 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { }; } + fn show_visibility(&mut self, visibility: Visibility) { + if visibility != Visibility::Private { + self.push_str(&visibility.to_string()); + self.push(' '); + } + } + fn show_data_type(&mut self, type_id: TypeId) { let shared_data_type = self.interner.get_type(type_id); let data_type = shared_data_type.borrow(); @@ -599,6 +606,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { Type::Unit => (), _ => { self.push_str(" -> "); + self.show_visibility(func_meta.return_visibility); self.show_type(return_type); } } @@ -627,12 +635,6 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } fn show_generics(&mut self, generics: &Generics) { - self.show_generics_impl( - generics, false, // only show names - ); - } - - fn show_generics_impl(&mut self, generics: &Generics, only_show_names: bool) { if generics.is_empty() { return; } @@ -643,24 +645,20 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(", "); } - if only_show_names { - self.push_str(&generic.name); - } else { - match generic.kind() { - noirc_frontend::Kind::Any | noirc_frontend::Kind::Normal => { - self.push_str(&generic.name); - } - noirc_frontend::Kind::IntegerOrField | noirc_frontend::Kind::Integer => { - self.push_str("let "); - self.push_str(&generic.name); - self.push_str(": u32"); - } - noirc_frontend::Kind::Numeric(typ) => { - self.push_str("let "); - self.push_str(&generic.name); - self.push_str(": "); - self.show_type(&typ); - } + match generic.kind() { + noirc_frontend::Kind::Any | noirc_frontend::Kind::Normal => { + self.push_str(&generic.name); + } + noirc_frontend::Kind::IntegerOrField | noirc_frontend::Kind::Integer => { + self.push_str("let "); + self.push_str(&generic.name); + self.push_str(": u32"); + } + noirc_frontend::Kind::Numeric(typ) => { + self.push_str("let "); + self.push_str(&generic.name); + self.push_str(": "); + self.show_type(&typ); } } } @@ -967,7 +965,24 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_hir_block_expression(hir_block_expression); } HirExpression::Prefix(hir_prefix_expression) => { - self.push_str(&hir_prefix_expression.operator.to_string()); + match hir_prefix_expression.operator { + noirc_frontend::ast::UnaryOp::Minus => { + self.push('-'); + } + noirc_frontend::ast::UnaryOp::Not => { + self.push('!'); + } + noirc_frontend::ast::UnaryOp::Reference { mutable } => { + if mutable { + self.push_str("&mut "); + } else { + self.push_str("&"); + } + } + noirc_frontend::ast::UnaryOp::Dereference { .. } => { + self.push('*'); + } + } self.show_hir_expression_id(hir_prefix_expression.rhs); } HirExpression::Infix(hir_infix_expression) => { @@ -1008,7 +1023,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(", "); } self.push_str(&name.to_string()); - self.push_str(" = "); + self.push_str(": "); self.show_hir_expression_id(*value); } self.push('}'); @@ -1164,6 +1179,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_hir_lvalue(hir_assign_statement.lvalue); self.push_str(" = "); self.show_hir_expression_id(hir_assign_statement.expression); + self.push(';'); } HirStatement::For(hir_for_statement) => { self.push_str("for "); From 2e517e4a9d04c69a7fabd69c8147c67e64ec180b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 13:26:14 -0300 Subject: [PATCH 44/57] More fixes --- .../src/hir/comptime/display.rs | 5 +- .../nargo_cli/src/cli/expand_cmd/printer.rs | 51 +++++++++++++++++-- 2 files changed, 51 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_frontend/src/hir/comptime/display.rs b/compiler/noirc_frontend/src/hir/comptime/display.rs index f73d751f9d7..1a482ff352b 100644 --- a/compiler/noirc_frontend/src/hir/comptime/display.rs +++ b/compiler/noirc_frontend/src/hir/comptime/display.rs @@ -293,7 +293,6 @@ impl<'interner> TokenPrettyPrinter<'interner> { | Token::RawStr(..) | Token::FmtStr(..) | Token::Whitespace(_) - | Token::LineComment(..) | Token::BlockComment(..) | Token::AttributeStart { .. } | Token::Invalid(_) => { @@ -302,6 +301,10 @@ impl<'interner> TokenPrettyPrinter<'interner> { } write!(f, "{token}") } + Token::LineComment(..) => { + writeln!(f, "{token}")?; + self.write_indent(f) + } Token::EOF => Ok(()), } } diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 256a3ca19db..2854561c6fb 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -22,7 +22,7 @@ use noirc_frontend::{ ExprId, FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, StmtId, TraitId, TraitImplId, TypeAliasId, TypeId, }, - token::FmtStrFragment, + token::{FmtStrFragment, FunctionAttribute}, }; pub(super) struct Printer<'interner, 'def_map, 'string> { @@ -630,7 +630,24 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } } else { - self.push(';'); + match &modifiers.attributes.function { + Some((attribute, _)) => match attribute { + FunctionAttribute::Foreign(_) + | FunctionAttribute::Builtin(_) + | FunctionAttribute::Oracle(_) => { + self.push_str(" {}"); + } + FunctionAttribute::Test(..) + | FunctionAttribute::Fold + | FunctionAttribute::NoPredicates + | FunctionAttribute::InlineAlways => { + self.push(';'); + } + }, + None => { + self.push(';'); + } + } } } @@ -780,7 +797,25 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push(')'); } Value::Struct(fields, typ) => { - self.show_type(typ); + let Type::DataType(data_type, generics) = typ else { + panic!("Expected a data type"); + }; + + // TODO: we might need to fully-qualify this name + let data_type = data_type.borrow(); + self.push_str(&data_type.name.to_string()); + + if !generics.is_empty() { + self.push_str("::<"); + for (index, generic) in generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + if fields.is_empty() { self.push_str(" {}"); } else { @@ -1126,6 +1161,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_hir_block_expression(hir_block_expression); } HirExpression::Unsafe(hir_block_expression) => { + // TODO: show the original comment + self.push_str("// Safety: TODO\n"); + self.write_indent(); self.push_str("unsafe "); self.show_hir_block_expression(hir_block_expression); } @@ -1344,7 +1382,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { fn show_hir_ident(&mut self, ident: HirIdent) { // TODO: we might need to fully-qualify this name let name = self.interner.definition_name(ident.id); - self.push_str(name); + + // The compiler uses '$' for some internal identifiers. + // We replace them with "___" to make sure they have valid syntax, even though + // there's a tiny change they might collide with user code (unlikely, really). + let name = name.replace('$', "___"); + self.push_str(&name); } fn pattern_is_self(&self, pattern: &HirPattern) -> bool { From 0b2ef4325e99f0ba623ef1ed24e6a96a113aa3d8 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 13:26:19 -0300 Subject: [PATCH 45/57] Format generated code if possible --- tooling/nargo_cli/src/cli/expand_cmd.rs | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index 989184c1c29..d3c373db4b5 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -6,7 +6,10 @@ use nargo::{ }; use nargo_toml::PackageSelection; use noirc_driver::CompileOptions; -use noirc_frontend::hir::{ParsedFiles, def_map::ModuleId}; +use noirc_frontend::{ + hir::{ParsedFiles, def_map::ModuleId}, + parse_program_with_dummy_file, +}; use printer::Printer; use crate::errors::CliError; @@ -70,7 +73,14 @@ fn expand_package( Printer::new(crate_id, &context.def_interner, &context.def_maps, def_map, &mut string); printer.show_module(module_id); printer.show_stray_trait_impls(); - println!("{}", string); + + let (parsed_module, errors) = parse_program_with_dummy_file(&string); + if errors.is_empty() { + let code = nargo_fmt::format(&string, parsed_module, &nargo_fmt::Config::default()); + println!("{code}"); + } else { + println!("{string}"); + } Ok(()) } From 0a3cd78a634ad0706eea9665515e2292148d2a11 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 13:40:08 -0300 Subject: [PATCH 46/57] Fix: correctly show data type name --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 42 ++++++++++--------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 2854561c6fb..509f43db943 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -797,24 +797,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push(')'); } Value::Struct(fields, typ) => { - let Type::DataType(data_type, generics) = typ else { - panic!("Expected a data type"); - }; - - // TODO: we might need to fully-qualify this name - let data_type = data_type.borrow(); - self.push_str(&data_type.name.to_string()); - - if !generics.is_empty() { - self.push_str("::<"); - for (index, generic) in generics.iter().enumerate() { - if index != 0 { - self.push_str(", "); - } - self.show_type(generic); - } - self.push('>'); - } + self.show_type_name_as_data_type(typ); if fields.is_empty() { self.push_str(" {}"); @@ -927,6 +910,27 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_type_name_as_data_type(&mut self, typ: &Type) { + let Type::DataType(data_type, generics) = typ.follow_bindings() else { + panic!("Expected a data type, got: {typ:?}"); + }; + + // TODO: we might need to fully-qualify this name + let data_type = data_type.borrow(); + self.push_str(&data_type.name.to_string()); + + if !generics.is_empty() { + self.push_str("::<"); + for (index, generic) in generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + } + fn show_imports( &mut self, imports: Vec<(Ident, ModuleDefId, ItemVisibility, bool /* is prelude */)>, @@ -1360,7 +1364,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push(')'); } HirPattern::Struct(typ, items, _location) => { - self.show_type(&typ); + self.show_type_name_as_data_type(&typ); self.push_str(" {\n"); self.increase_indent(); for (index, (name, pattern)) in items.into_iter().enumerate() { From 139ab9a4e89d276fe21b408a1e29407488444001 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 13:56:01 -0300 Subject: [PATCH 47/57] Show attributes --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 36 +++++++++++++++++-- 1 file changed, 34 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 509f43db943..d30abd937f9 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -189,6 +189,39 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_module_def_id_attributes(&mut self, module_def_id: ModuleDefId) { + match module_def_id { + ModuleDefId::FunctionId(func_id) => { + let modifiers = self.interner.function_modifiers(&func_id); + if let Some(attribute) = modifiers.attributes.function() { + self.push_str(&attribute.to_string()); + self.push('\n'); + self.write_indent(); + } + self.show_secondary_attributes(&modifiers.attributes.secondary); + } + ModuleDefId::TypeId(type_id) => { + self.show_secondary_attributes(self.interner.type_attributes(&type_id)); + } + ModuleDefId::GlobalId(global_id) => { + self.show_secondary_attributes(self.interner.global_attributes(&global_id)); + } + ModuleDefId::ModuleId(..) | ModuleDefId::TypeAliasId(..) | ModuleDefId::TraitId(..) => { + () + } + } + } + + fn show_secondary_attributes(&mut self, attributes: &[SecondaryAttribute]) { + for attribute in attributes { + if !matches!(attribute, SecondaryAttribute::Meta(..)) { + self.push_str(&attribute.to_string()); + self.push('\n'); + self.write_indent(); + } + } + } + fn show_item_visibility(&mut self, visibility: ItemVisibility) { if visibility != ItemVisibility::Private { self.push_str(&visibility.to_string()); @@ -560,7 +593,6 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { fn show_function(&mut self, func_id: FuncId) { let modifiers = self.interner.function_modifiers(&func_id); let func_meta = self.interner.function_meta(&func_id); - let name = &modifiers.name; if modifiers.is_unconstrained { self.push_str("unconstrained "); @@ -570,7 +602,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } self.push_str("fn "); - self.push_str(name); + self.push_str(&modifiers.name); self.show_generics(&func_meta.direct_generics); From 7e1d71dad8daaeed6e0a91d15505f36f5ca3094c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 13:56:06 -0300 Subject: [PATCH 48/57] Show unsafe comment on top of let --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index d30abd937f9..3a3671f79b0 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -22,7 +22,7 @@ use noirc_frontend::{ ExprId, FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, StmtId, TraitId, TraitImplId, TypeAliasId, TypeId, }, - token::{FmtStrFragment, FunctionAttribute}, + token::{FmtStrFragment, FunctionAttribute, SecondaryAttribute}, }; pub(super) struct Printer<'interner, 'def_map, 'string> { @@ -153,6 +153,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { let reference_id = module_def_id_to_reference_id(module_def_id); self.show_doc_comments(reference_id); + self.show_module_def_id_attributes(module_def_id); + self.show_item_visibility(visibility); match module_def_id { @@ -1241,12 +1243,30 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { fn show_hir_statement(&mut self, statement: HirStatement) { match statement { HirStatement::Let(hir_let_statement) => { + // If this is `let ... = unsafe { }` then show the unsafe comment on top of `let` + if let HirExpression::Unsafe(_) = + self.interner.expression(&hir_let_statement.expression) + { + // TODO: show the original comment + self.push_str("// Safety: TODO\n"); + self.write_indent(); + } + self.push_str("let "); self.show_hir_pattern(hir_let_statement.pattern); self.push_str(": "); self.show_type(&hir_let_statement.r#type); self.push_str(" = "); - self.show_hir_expression_id(hir_let_statement.expression); + + if let HirExpression::Unsafe(block_expression) = + self.interner.expression(&hir_let_statement.expression) + { + self.push_str("unsafe "); + self.show_hir_block_expression(block_expression); + } else { + self.show_hir_expression_id(hir_let_statement.expression); + } + self.push(';'); } HirStatement::Assign(hir_assign_statement) => { From 3acf1b1989aa1ceaa86d22b5db8935c02d2ed03e Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 14:05:25 -0300 Subject: [PATCH 49/57] Merge imports --- tooling/nargo_cli/src/cli/expand_cmd.rs | 9 ++++++++- tooling/nargo_fmt/src/lib.rs | 2 +- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index d3c373db4b5..2f489b85b9d 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -4,6 +4,7 @@ use nargo::{ errors::CompileError, insert_all_files_for_workspace_into_file_manager, package::Package, parse_all, prepare_package, workspace::Workspace, }; +use nargo_fmt::ImportsGranularity; use nargo_toml::PackageSelection; use noirc_driver::CompileOptions; use noirc_frontend::{ @@ -76,10 +77,16 @@ fn expand_package( let (parsed_module, errors) = parse_program_with_dummy_file(&string); if errors.is_empty() { - let code = nargo_fmt::format(&string, parsed_module, &nargo_fmt::Config::default()); + let mut config = nargo_fmt::Config::default(); + config.reorder_imports = true; + config.imports_granularity = ImportsGranularity::Crate; + + let code = nargo_fmt::format(&string, parsed_module, &config); println!("{code}"); } else { println!("{string}"); + println!(); + println!("// Warning: the generated code has syntax errors") } Ok(()) diff --git a/tooling/nargo_fmt/src/lib.rs b/tooling/nargo_fmt/src/lib.rs index bf3cf48184d..6492f0e1ad6 100644 --- a/tooling/nargo_fmt/src/lib.rs +++ b/tooling/nargo_fmt/src/lib.rs @@ -42,7 +42,7 @@ mod formatter; use formatter::Formatter; use noirc_frontend::ParsedModule; -pub use config::Config; +pub use config::{Config, ImportsGranularity}; pub fn format(source: &str, parsed_module: ParsedModule, config: &Config) -> String { let mut formatter = Formatter::new(source, config); From 8c072a58a3cb4ed1d903fe65a12733669c969a50 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 14:05:32 -0300 Subject: [PATCH 50/57] Fix showing parent traits --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 3a3671f79b0..89f4fa16833 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -419,7 +419,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(": "); for (index, trait_bound) in trait_.trait_bounds.iter().enumerate() { if index != 0 { - self.push_str(", "); + self.push_str(" + "); } self.show_trait_bound(trait_bound); } From f1ccb41d5941fba45a1a216b25ba0c385b35b3b1 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 14:54:14 -0300 Subject: [PATCH 51/57] Fully-qualify hir idents if needed --- compiler/noirc_frontend/src/modules.rs | 5 + tooling/nargo_cli/src/cli/expand_cmd.rs | 11 +- .../nargo_cli/src/cli/expand_cmd/printer.rs | 126 ++++++++++++++---- 3 files changed, 113 insertions(+), 29 deletions(-) diff --git a/compiler/noirc_frontend/src/modules.rs b/compiler/noirc_frontend/src/modules.rs index 9bb4f5e8d5b..b5b76714b00 100644 --- a/compiler/noirc_frontend/src/modules.rs +++ b/compiler/noirc_frontend/src/modules.rs @@ -41,6 +41,11 @@ pub fn relative_module_full_path( } else { let parent_module = get_parent_module(interner, module_def_id)?; + // If module_def_id is contained in the current module, the relative path is empty + if current_module_id == parent_module { + return None; + } + full_path = relative_module_id_path( parent_module, current_module_id, diff --git a/tooling/nargo_cli/src/cli/expand_cmd.rs b/tooling/nargo_cli/src/cli/expand_cmd.rs index 2f489b85b9d..29f5825009b 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd.rs @@ -77,16 +77,17 @@ fn expand_package( let (parsed_module, errors) = parse_program_with_dummy_file(&string); if errors.is_empty() { - let mut config = nargo_fmt::Config::default(); - config.reorder_imports = true; - config.imports_granularity = ImportsGranularity::Crate; - + let config = nargo_fmt::Config { + reorder_imports: true, + imports_granularity: ImportsGranularity::Crate, + ..Default::default() + }; let code = nargo_fmt::format(&string, parsed_module, &config); println!("{code}"); } else { println!("{string}"); println!(); - println!("// Warning: the generated code has syntax errors") + println!("// Warning: the generated code has syntax errors"); } Ok(()) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 89f4fa16833..016a078c3cf 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -19,8 +19,8 @@ use noirc_frontend::{ }, modules::relative_module_full_path, node_interner::{ - ExprId, FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, ReferenceId, - StmtId, TraitId, TraitImplId, TypeAliasId, TypeId, + DefinitionKind, ExprId, FuncId, GlobalId, GlobalValue, ImplMethod, Methods, NodeInterner, + ReferenceId, StmtId, TraitId, TraitImplId, TypeAliasId, TypeId, }, token::{FmtStrFragment, FunctionAttribute, SecondaryAttribute}, }; @@ -209,7 +209,6 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_secondary_attributes(self.interner.global_attributes(&global_id)); } ModuleDefId::ModuleId(..) | ModuleDefId::TypeAliasId(..) | ModuleDefId::TraitId(..) => { - () } } } @@ -969,8 +968,6 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { &mut self, imports: Vec<(Ident, ModuleDefId, ItemVisibility, bool /* is prelude */)>, ) { - let current_module_parent_id = self.module_id.parent(self.def_maps); - let mut first = true; for (alias, module_def_id, visibility, is_prelude) in imports { @@ -985,18 +982,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.write_indent(); self.show_item_visibility(visibility); self.push_str("use "); - if let Some(full_path) = relative_module_full_path( - module_def_id, - self.module_id, - current_module_parent_id, - self.interner, - ) { - self.push_str(&full_path); - self.push_str("::"); - }; - - let name = self.module_def_id_name(module_def_id); - self.push_str(&name); + let use_import = false; + let name = self.show_reference_to_module_def_id(module_def_id, use_import); if name != alias.0.contents { self.push_str(" as "); @@ -1007,6 +994,84 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_reference_to_module_def_id( + &mut self, + module_def_id: ModuleDefId, + use_import: bool, + ) -> String { + if let ModuleDefId::FunctionId(func_id) = module_def_id { + let func_meta = self.interner.function_meta(&func_id); + + if let Some(trait_impl_id) = func_meta.trait_impl { + let trait_impl = self.interner.get_trait_implementation(trait_impl_id); + let trait_impl = trait_impl.borrow(); + self.show_reference_to_module_def_id( + ModuleDefId::TraitId(trait_impl.trait_id), + use_import, + ); + if !trait_impl.trait_generics.is_empty() { + self.push_str("::<"); + for (index, generic) in trait_impl.trait_generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + + self.push_str("::"); + + let name = self.interner.function_name(&func_id).to_string(); + self.push_str(&name); + return name; + } + + if let Some(trait_id) = func_meta.trait_id { + self.show_reference_to_module_def_id(ModuleDefId::TraitId(trait_id), use_import); + self.push_str("::"); + + let name = self.interner.function_name(&func_id).to_string(); + self.push_str(&name); + return name; + } + + if let Some(type_id) = func_meta.type_id { + self.show_reference_to_module_def_id(ModuleDefId::TypeId(type_id), use_import); + self.push_str("::"); + + let name = self.interner.function_name(&func_id).to_string(); + self.push_str(&name); + return name; + } + } + + if use_import { + if let Some(name) = self.imports.get(&module_def_id) { + let name = name.to_string(); + self.push_str(&name); + return name; + } + } + + let current_module_parent_id = self.module_id.parent(self.def_maps); + if let Some(full_path) = relative_module_full_path( + module_def_id, + self.module_id, + current_module_parent_id, + self.interner, + ) { + if !full_path.is_empty() { + self.push_str(&full_path); + self.push_str("::"); + } + }; + + let name = self.module_def_id_name(module_def_id); + self.push_str(&name); + name + } + fn show_type(&mut self, typ: &Type) { self.push_str(&typ.to_string()); } @@ -1436,14 +1501,27 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } fn show_hir_ident(&mut self, ident: HirIdent) { - // TODO: we might need to fully-qualify this name - let name = self.interner.definition_name(ident.id); + let definition_id = self.interner.definition(ident.id); + match definition_id.kind { + DefinitionKind::Function(func_id) => { + let use_import = true; + self.show_reference_to_module_def_id(ModuleDefId::FunctionId(func_id), use_import); + } + DefinitionKind::Global(global_id) => { + let use_import = true; + self.show_reference_to_module_def_id(ModuleDefId::GlobalId(global_id), use_import); + } + DefinitionKind::Local(..) | DefinitionKind::NumericGeneric(..) => { + let name = self.interner.definition_name(ident.id); - // The compiler uses '$' for some internal identifiers. - // We replace them with "___" to make sure they have valid syntax, even though - // there's a tiny change they might collide with user code (unlikely, really). - let name = name.replace('$', "___"); - self.push_str(&name); + // The compiler uses '$' for some internal identifiers. + // We replace them with "___" to make sure they have valid syntax, even though + // there's a tiny change they might collide with user code (unlikely, really). + let name = name.replace('$', "___"); + + self.push_str(&name); + } + } } fn pattern_is_self(&self, pattern: &HirPattern) -> bool { From 549ca30c843a0942e8f1921d4cc58fbe92517779 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 16:37:50 -0300 Subject: [PATCH 52/57] Show calls as methods if possible --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 100 ++++++++++++++++-- 1 file changed, 90 insertions(+), 10 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 016a078c3cf..f92f75812e2 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -3,8 +3,8 @@ use std::collections::{HashMap, HashSet}; use noirc_driver::CrateId; use noirc_errors::Location; use noirc_frontend::{ - DataType, Generics, Type, - ast::{Ident, ItemVisibility, Visibility}, + DataType, Generics, Type, TypeBindings, + ast::{Ident, ItemVisibility, UnaryOp, Visibility}, hir::{ comptime::{Value, tokens_to_string_with_indent}, def_map::{CrateDefMap, DefMaps, ModuleDefId, ModuleId}, @@ -12,7 +12,8 @@ use noirc_frontend::{ }, hir_def::{ expr::{ - HirArrayLiteral, HirBlockExpression, HirExpression, HirIdent, HirLiteral, HirMatch, + HirArrayLiteral, HirBlockExpression, HirCallExpression, HirExpression, HirIdent, + HirLiteral, HirMatch, }, stmt::{HirLValue, HirLetStatement, HirPattern, HirStatement}, traits::{ResolvedTraitBound, TraitConstraint}, @@ -1081,6 +1082,23 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.show_hir_expression(hir_expr); } + fn show_hir_expression_id_dereferencing(&mut self, expr_id: ExprId) { + let hir_expr = self.interner.expression(&expr_id); + let HirExpression::Prefix(prefix) = &hir_expr else { + self.show_hir_expression(hir_expr); + return; + }; + + match prefix.operator { + UnaryOp::Reference { .. } | UnaryOp::Dereference { implicitly_added: true } => { + self.show_hir_expression_id_dereferencing(prefix.rhs); + } + UnaryOp::Minus | UnaryOp::Not | UnaryOp::Dereference { implicitly_added: false } => { + self.show_hir_expression(hir_expr); + } + } + } + fn show_hir_expression(&mut self, hir_expr: HirExpression) { match hir_expr { HirExpression::Ident(hir_ident, generics) => { @@ -1104,21 +1122,23 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } HirExpression::Prefix(hir_prefix_expression) => { match hir_prefix_expression.operator { - noirc_frontend::ast::UnaryOp::Minus => { + UnaryOp::Minus => { self.push('-'); } - noirc_frontend::ast::UnaryOp::Not => { + UnaryOp::Not => { self.push('!'); } - noirc_frontend::ast::UnaryOp::Reference { mutable } => { + UnaryOp::Reference { mutable } => { if mutable { self.push_str("&mut "); } else { self.push_str("&"); } } - noirc_frontend::ast::UnaryOp::Dereference { .. } => { - self.push('*'); + UnaryOp::Dereference { implicitly_added } => { + if !implicitly_added { + self.push('*'); + } } } self.show_hir_expression_id(hir_prefix_expression.rhs); @@ -1187,6 +1207,10 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(&hir_member_access.rhs.to_string()); } HirExpression::Call(hir_call_expression) => { + if self.try_show_hir_call_as_method(&hir_call_expression) { + return; + } + self.show_hir_expression_id(hir_call_expression.func); if hir_call_expression.is_macro_call { self.push('!'); @@ -1278,6 +1302,62 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn try_show_hir_call_as_method(&mut self, hir_call_expression: &HirCallExpression) -> bool { + let arguments = &hir_call_expression.arguments; + + // If there are no arguments this is definitely not a method call + if arguments.is_empty() { + return false; + } + + // A method call must have `func` be a HirIdent + let HirExpression::Ident(hir_ident, _generics) = + self.interner.expression(&hir_call_expression.func) + else { + return false; + }; + + // That HirIdent must be a function reference + let definition = self.interner.definition(hir_ident.id); + let DefinitionKind::Function(func_id) = definition.kind else { + return false; + }; + + // The function must have a self type + let func_meta = self.interner.function_meta(&func_id); + let Some(self_type) = &func_meta.self_type else { + return false; + }; + + // And it must have parameters + if func_meta.parameters.is_empty() { + return false; + } + + // The first parameter must unify with the self type (as-is or after removing `&mut`) + let param_type = func_meta.parameters.0[0].1.follow_bindings(); + let param_type = if let Type::Reference(typ, ..) = param_type { *typ } else { param_type }; + + let mut bindings = TypeBindings::new(); + if self_type.try_unify(¶m_type, &mut bindings).is_err() { + return false; + } + + self.show_hir_expression_id_dereferencing(arguments[0]); + self.push('.'); + self.push_str(self.interner.function_name(&func_id)); + self.push('('); + for (index, argument) in arguments[1..].iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_hir_expression_id(*argument); + } + self.push(')'); + + true + } + fn show_hir_block_expression(&mut self, block: HirBlockExpression) { self.push_str("{\n"); self.increase_indent(); @@ -1501,8 +1581,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } fn show_hir_ident(&mut self, ident: HirIdent) { - let definition_id = self.interner.definition(ident.id); - match definition_id.kind { + let definition = self.interner.definition(ident.id); + match definition.kind { DefinitionKind::Function(func_id) => { let use_import = true; self.show_reference_to_module_def_id(ModuleDefId::FunctionId(func_id), use_import); From 67e2c1d88151507a0af881e24b27d3f96f4fdcb4 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 17:02:34 -0300 Subject: [PATCH 53/57] Allow method calls after statement block (like Rust) --- .../src/parser/parser/expression.rs | 19 +++++++++++++++++++ .../src/parser/parser/function.rs | 12 ++++++++++++ .../src/parser/parser/statement.rs | 19 +++++++++++++------ 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/compiler/noirc_frontend/src/parser/parser/expression.rs b/compiler/noirc_frontend/src/parser/parser/expression.rs index 672d9428d9a..4b51c6219ff 100644 --- a/compiler/noirc_frontend/src/parser/parser/expression.rs +++ b/compiler/noirc_frontend/src/parser/parser/expression.rs @@ -147,6 +147,25 @@ impl Parser<'_> { self.parse_index(atom, start_location) } + pub(super) fn parse_member_accesses_or_method_calls_after_expression( + &mut self, + mut atom: Expression, + start_location: Location, + ) -> Expression { + let mut parsed; + + loop { + (atom, parsed) = self.parse_member_access_or_method_call(atom, start_location); + if parsed { + continue; + } else { + break; + } + } + + atom + } + /// CallExpression = Atom CallArguments fn parse_call(&mut self, atom: Expression, start_location: Location) -> (Expression, bool) { if let Some(call_arguments) = self.parse_call_arguments() { diff --git a/compiler/noirc_frontend/src/parser/parser/function.rs b/compiler/noirc_frontend/src/parser/parser/function.rs index caf2cdeb1c3..b9271f36d5c 100644 --- a/compiler/noirc_frontend/src/parser/parser/function.rs +++ b/compiler/noirc_frontend/src/parser/parser/function.rs @@ -570,4 +570,16 @@ mod tests { UnresolvedTypeData::Integer(Signedness::Signed, IntegerBitSize::SixtyFour) ); } + + #[test] + fn parses_block_followed_by_call() { + let src = "fn foo() { { 1 }.bar() }"; + let _ = parse_function_no_error(src); + } + + #[test] + fn parses_if_followed_by_call() { + let src = "fn foo() { if 1 { 2 } else { 3 }.bar() }"; + let _ = parse_function_no_error(src); + } } diff --git a/compiler/noirc_frontend/src/parser/parser/statement.rs b/compiler/noirc_frontend/src/parser/parser/statement.rs index 600ddec43c9..873f921f12f 100644 --- a/compiler/noirc_frontend/src/parser/parser/statement.rs +++ b/compiler/noirc_frontend/src/parser/parser/statement.rs @@ -148,19 +148,26 @@ impl Parser<'_> { if let Some(kind) = self.parse_if_expr() { let location = self.location_since(start_location); - return Some(StatementKind::Expression(Expression { kind, location })); + let expression = Expression { kind, location }; + let expression = self + .parse_member_accesses_or_method_calls_after_expression(expression, start_location); + return Some(StatementKind::Expression(expression)); } if let Some(kind) = self.parse_match_expr() { let location = self.location_since(start_location); - return Some(StatementKind::Expression(Expression { kind, location })); + let expression = Expression { kind, location }; + let expression = self + .parse_member_accesses_or_method_calls_after_expression(expression, start_location); + return Some(StatementKind::Expression(expression)); } if let Some(block) = self.parse_block() { - return Some(StatementKind::Expression(Expression { - kind: ExpressionKind::Block(block), - location: self.location_since(start_location), - })); + let location = self.location_since(start_location); + let expression = Expression { kind: ExpressionKind::Block(block), location }; + let expression = self + .parse_member_accesses_or_method_calls_after_expression(expression, start_location); + return Some(StatementKind::Expression(expression)); } if let Some(token) = self.eat_kind(TokenKind::InternedLValue) { From ef8140563d77ff50155594baa8fa851c2b7d769b Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 17:04:47 -0300 Subject: [PATCH 54/57] Use a different kind of comment because of a formatter bug --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index f92f75812e2..caa8bd83521 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1289,7 +1289,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } HirExpression::Unsafe(hir_block_expression) => { // TODO: show the original comment - self.push_str("// Safety: TODO\n"); + self.push_str("/* Safety: TODO */\n"); self.write_indent(); self.push_str("unsafe "); self.show_hir_block_expression(hir_block_expression); @@ -1393,7 +1393,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.interner.expression(&hir_let_statement.expression) { // TODO: show the original comment - self.push_str("// Safety: TODO\n"); + self.push_str("/* Safety: TODO */\n"); self.write_indent(); } From 10654f0dc61cb23b80431e2a8e17fe85bdbb539c Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 17:38:40 -0300 Subject: [PATCH 55/57] Fully-qualify types if needed --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 266 ++++++++++++------ 1 file changed, 185 insertions(+), 81 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index caa8bd83521..e22cde1dd5e 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -3,7 +3,7 @@ use std::collections::{HashMap, HashSet}; use noirc_driver::CrateId; use noirc_errors::Location; use noirc_frontend::{ - DataType, Generics, Type, TypeBindings, + DataType, Generics, Type, TypeBinding, TypeBindings, ast::{Ident, ItemVisibility, UnaryOp, Visibility}, hir::{ comptime::{Value, tokens_to_string_with_indent}, @@ -539,16 +539,10 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push(' '); self.push_str(&trait_.name.to_string()); - if !trait_impl.trait_generics.is_empty() { - self.push('<'); - for (index, generic) in trait_impl.trait_generics.iter().enumerate() { - if index != 0 { - self.push_str(", "); - } - self.show_type(generic); - } - self.push('>'); - } + + let use_colons = false; + self.show_generic_types(&trait_impl.trait_generics, use_colons); + self.push_str(" for "); self.show_type(&trait_impl.typ); self.show_where_clause(&trait_impl.where_clause); @@ -685,6 +679,18 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } + fn show_generic_types(&mut self, types: &[Type], use_colons: bool) { + if types.is_empty() { + return; + } + if use_colons { + self.push_str("::"); + } + self.push('<'); + self.show_types_separated_by_comma(types); + self.push('>'); + } + fn show_generics(&mut self, generics: &Generics) { if generics.is_empty() { return; @@ -816,9 +822,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { )); } Value::Function(func_id, ..) => { - // TODO: the name might need to be fully-qualified - let name = self.interner.function_name(func_id); - self.push_str(name); + let use_import = true; + self.show_reference_to_module_def_id(ModuleDefId::FunctionId(*func_id), use_import); } Value::Tuple(values) => { self.push('('); @@ -851,25 +856,13 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } } Value::Enum(index, args, typ) => { - let Type::DataType(data_type, generics) = typ else { + self.show_type_name_as_data_type(typ); + + let Type::DataType(data_type, _generics) = typ.follow_bindings() else { panic!("Expected typ to be a data type"); }; let data_type = data_type.borrow(); - // TODO: we might need to fully-qualify this enum - self.push_str(&data_type.name.to_string()); - - if !generics.is_empty() { - self.push_str("::<"); - for (index, generic) in generics.iter().enumerate() { - if index != 0 { - self.push_str(", "); - } - self.show_type(generic); - } - self.push('>'); - } - let variant = data_type.variant_at(*index); self.push_str("::"); self.push_str(&variant.name.to_string()); @@ -949,20 +942,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { panic!("Expected a data type, got: {typ:?}"); }; - // TODO: we might need to fully-qualify this name let data_type = data_type.borrow(); - self.push_str(&data_type.name.to_string()); + let use_import = true; + self.show_reference_to_module_def_id(ModuleDefId::TypeId(data_type.id), use_import); - if !generics.is_empty() { - self.push_str("::<"); - for (index, generic) in generics.iter().enumerate() { - if index != 0 { - self.push_str(", "); - } - self.show_type(generic); - } - self.push('>'); - } + let use_colons = true; + self.show_generic_types(&generics, use_colons); } fn show_imports( @@ -1010,16 +995,9 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { ModuleDefId::TraitId(trait_impl.trait_id), use_import, ); - if !trait_impl.trait_generics.is_empty() { - self.push_str("::<"); - for (index, generic) in trait_impl.trait_generics.iter().enumerate() { - if index != 0 { - self.push_str(", "); - } - self.show_type(generic); - } - self.push('>'); - } + + let use_colons = true; + self.show_generic_types(&trait_impl.trait_generics, use_colons); self.push_str("::"); @@ -1073,8 +1051,152 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { name } + fn show_types_separated_by_comma(&mut self, types: &[Type]) { + for (index, typ) in types.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(typ); + } + } + fn show_type(&mut self, typ: &Type) { - self.push_str(&typ.to_string()); + match typ { + Type::Array(length, typ) => { + self.push('['); + self.show_type(typ); + self.push_str("; "); + self.show_type(length); + self.push(']'); + } + Type::Slice(typ) => { + self.push('['); + self.show_type(typ); + self.push(']'); + } + Type::FmtString(length, typ) => { + self.push_str("fmtstr<"); + self.show_type(length); + self.push_str(", "); + self.show_type(typ); + self.push('>'); + } + Type::Tuple(types) => { + let len = types.len(); + self.push('('); + for (index, typ) in types.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(typ); + } + if len == 1 { + self.push(','); + } + self.push(')'); + } + Type::DataType(data_type, generics) => { + let data_type = data_type.borrow(); + let use_import = true; + self.show_reference_to_module_def_id(ModuleDefId::TypeId(data_type.id), use_import); + if !generics.is_empty() { + self.push_str("<"); + for (index, generic) in generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + } + Type::Alias(type_alias, generics) => { + let type_alias = type_alias.borrow(); + let use_import = true; + self.show_reference_to_module_def_id( + ModuleDefId::TypeAliasId(type_alias.id), + use_import, + ); + if !generics.is_empty() { + self.push_str("<"); + for (index, generic) in generics.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(generic); + } + self.push('>'); + } + } + Type::TypeVariable(type_variable) => match &*type_variable.borrow() { + TypeBinding::Bound(typ) => { + self.show_type(&typ); + } + TypeBinding::Unbound(..) => { + self.push('_'); + } + }, + Type::TraitAsType(..) => { + panic!("Trait as type should not happen") + } + Type::NamedGeneric(_type_variable, name) => { + self.push_str(name); + } + Type::CheckedCast { from: _, to } => { + self.show_type(to); + } + Type::Function(args, ret, env, unconstrained) => { + if *unconstrained { + self.push_str("unconstrained "); + } + self.push_str("fn"); + if **env != Type::Unit { + self.push('['); + self.show_type(env); + self.push(']'); + } + self.push('('); + for (index, arg) in args.iter().enumerate() { + if index != 0 { + self.push_str(", "); + } + self.show_type(arg); + } + self.push_str(")"); + if **ret != Type::Unit { + self.push_str(" -> "); + self.show_type(ret); + } + } + Type::Reference(typ, mutable) => { + if *mutable { + self.push_str("&mut "); + } else { + self.push('&'); + } + self.show_type(typ); + } + Type::Forall(..) => { + panic!("Should not need to print Type::Forall") + } + Type::Constant(field_element, _) => { + self.push_str(&field_element.to_string()); + } + Type::InfixExpr(lhs, op, rhs, _) => { + self.show_type(lhs); + self.push(' '); + self.push_str(&op.to_string()); + self.push(' '); + self.show_type(rhs); + } + Type::Unit + | Type::Bool + | Type::Integer(..) + | Type::FieldElement + | Type::String(_) + | Type::Quoted(..) + | Type::Error => self.push_str(&typ.to_string()), + } } fn show_hir_expression_id(&mut self, expr_id: ExprId) { @@ -1104,14 +1226,8 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { HirExpression::Ident(hir_ident, generics) => { self.show_hir_ident(hir_ident); if let Some(generics) = generics { - self.push_str("::<"); - for (index, generic) in generics.iter().enumerate() { - if index != 0 { - self.push_str(", "); - } - self.show_type(generic); - } - self.push('>'); + let use_colons = true; + self.show_generic_types(&generics, use_colons); } } HirExpression::Literal(hir_literal) => { @@ -1157,23 +1273,12 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push(']'); } HirExpression::Constructor(hir_constructor_expression) => { - // TODO: we might need to fully-qualify this name - let typ = hir_constructor_expression.r#type.borrow(); - let name = typ.name.to_string(); - self.push_str(&name); + let data_type = hir_constructor_expression.r#type.borrow(); + let use_import = true; + self.show_reference_to_module_def_id(ModuleDefId::TypeId(data_type.id), use_import); - if !hir_constructor_expression.struct_generics.is_empty() { - self.push_str("::<"); - for (index, typ) in - hir_constructor_expression.struct_generics.iter().enumerate() - { - if index != 0 { - self.push_str(", "); - } - self.show_type(typ); - } - self.push('>'); - } + let use_colons = true; + self.show_generic_types(&hir_constructor_expression.struct_generics, use_colons); self.push_str(" { "); for (index, (name, value)) in hir_constructor_expression.fields.iter().enumerate() { @@ -1187,12 +1292,11 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push('}'); } HirExpression::EnumConstructor(constructor) => { - // TODO: we might need to fully-qualify this name - let typ = constructor.r#type.borrow(); - let name = typ.name.to_string(); - self.push_str(&name); + let data_type = constructor.r#type.borrow(); + let use_import = true; + self.show_reference_to_module_def_id(ModuleDefId::TypeId(data_type.id), use_import); - let variant = typ.variant_at(constructor.variant_index); + let variant = data_type.variant_at(constructor.variant_index); self.push_str("::"); self.push_str(&variant.name.to_string()); if variant.is_function { From b1cd85259a20676abd54540a165bde16b8bcd7a7 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 17:40:51 -0300 Subject: [PATCH 56/57] Surround with parentheses just in case --- .../nargo_cli/src/cli/expand_cmd/printer.rs | 47 +++++++++++-------- 1 file changed, 27 insertions(+), 20 deletions(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index e22cde1dd5e..2871fbd521f 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1183,11 +1183,13 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { self.push_str(&field_element.to_string()); } Type::InfixExpr(lhs, op, rhs, _) => { + self.push('('); self.show_type(lhs); self.push(' '); self.push_str(&op.to_string()); self.push(' '); self.show_type(rhs); + self.push(')'); } Type::Unit | Type::Bool @@ -1236,35 +1238,40 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { HirExpression::Block(hir_block_expression) => { self.show_hir_block_expression(hir_block_expression); } - HirExpression::Prefix(hir_prefix_expression) => { - match hir_prefix_expression.operator { - UnaryOp::Minus => { - self.push('-'); - } - UnaryOp::Not => { - self.push('!'); - } - UnaryOp::Reference { mutable } => { - if mutable { - self.push_str("&mut "); - } else { - self.push_str("&"); - } + HirExpression::Prefix(hir_prefix_expression) => match hir_prefix_expression.operator { + UnaryOp::Minus => { + self.push_str("-("); + self.show_hir_expression_id(hir_prefix_expression.rhs); + self.push(')'); + } + UnaryOp::Not => { + self.push_str("!("); + self.show_hir_expression_id(hir_prefix_expression.rhs); + self.push(')'); + } + UnaryOp::Reference { mutable } => { + if mutable { + self.push_str("&mut "); + } else { + self.push_str("&"); } - UnaryOp::Dereference { implicitly_added } => { - if !implicitly_added { - self.push('*'); - } + self.show_hir_expression_id(hir_prefix_expression.rhs); + } + UnaryOp::Dereference { implicitly_added } => { + if !implicitly_added { + self.push('*'); } + self.show_hir_expression_id(hir_prefix_expression.rhs); } - self.show_hir_expression_id(hir_prefix_expression.rhs); - } + }, HirExpression::Infix(hir_infix_expression) => { + self.push('('); self.show_hir_expression_id(hir_infix_expression.lhs); self.push(' '); self.push_str(&hir_infix_expression.operator.kind.to_string()); self.push(' '); self.show_hir_expression_id(hir_infix_expression.rhs); + self.push(')'); } HirExpression::Index(hir_index_expression) => { self.show_hir_expression_id(hir_index_expression.collection); From 31937e78941f6425e5bd32106a6f66c0f83ce217 Mon Sep 17 00:00:00 2001 From: Ary Borenszweig Date: Mon, 10 Mar 2025 17:42:45 -0300 Subject: [PATCH 57/57] clippy --- tooling/nargo_cli/src/cli/expand_cmd/printer.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs index 2871fbd521f..fd2bb68e7e4 100644 --- a/tooling/nargo_cli/src/cli/expand_cmd/printer.rs +++ b/tooling/nargo_cli/src/cli/expand_cmd/printer.rs @@ -1130,7 +1130,7 @@ impl<'interner, 'def_map, 'string> Printer<'interner, 'def_map, 'string> { } Type::TypeVariable(type_variable) => match &*type_variable.borrow() { TypeBinding::Bound(typ) => { - self.show_type(&typ); + self.show_type(typ); } TypeBinding::Unbound(..) => { self.push('_');