WIP Add fully qualified paths for structs in ABI

phated · phated · commit e8022779d816 · 2023-08-18T16:11:37.000-07:00
diff --git a/crates/nargo_cli/src/cli/check_cmd.rs b/crates/nargo_cli/src/cli/check_cmd.rs
@@ -135,7 +135,7 @@ mod tests {
             typed_param(
                 "d",
                 AbiType::Struct {
-                    name: String::from("MyStruct"),
+                    path: String::from("MyStruct"),
                     fields: vec![
                         (String::from("d1"), AbiType::Field),
                         (
diff --git a/crates/nargo_cli/src/cli/test_cmd.rs b/crates/nargo_cli/src/cli/test_cmd.rs
@@ -6,7 +6,7 @@ use nargo::{ops::execute_circuit, package::Package, prepare_package};
 use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
 use noirc_driver::{compile_no_check, CompileOptions};
 use noirc_frontend::{
-    graph::CrateName,
+    graph::{CrateId, CrateName},
     hir::{Context, FunctionNameMatch},
     node_interner::FuncId,
 };
@@ -94,7 +94,15 @@ fn run_tests<B: Backend>(
             .expect("Failed to write to stdout");
         writer.flush().expect("Failed to flush writer");
 
-        match run_test(backend, &test_name, test_function, &context, show_output, compile_options) {
+        match run_test(
+            backend,
+            &crate_id,
+            &test_name,
+            test_function,
+            &context,
+            show_output,
+            compile_options,
+        ) {
             Ok(_) => {
                 writer
                     .set_color(ColorSpec::new().set_fg(Some(Color::Green)))
@@ -122,13 +130,14 @@ fn run_tests<B: Backend>(
 
 fn run_test<B: Backend>(
     backend: &B,
+    crate_id: &CrateId,
     test_name: &str,
     main: FuncId,
     context: &Context,
     show_output: bool,
     config: &CompileOptions,
 ) -> Result<(), CliError<B>> {
-    let mut program = compile_no_check(context, config, main).map_err(|err| {
+    let mut program = compile_no_check(context, crate_id, config, main).map_err(|err| {
         noirc_errors::reporter::report_all(&context.file_manager, &[err], config.deny_warnings);
         CliError::Generic(format!("Test '{test_name}' failed to compile"))
     })?;
diff --git a/crates/noirc_abi/src/input_parser/mod.rs b/crates/noirc_abi/src/input_parser/mod.rs
@@ -133,7 +133,7 @@ mod serialization_tests {
                 AbiParameter {
                     name: "bar".into(),
                     typ: AbiType::Struct {
-                        name: "MyStruct".into(),
+                        path: "MyStruct".into(),
                         fields: vec![
                             ("field1".into(), AbiType::Integer { sign: Sign::Unsigned, width: 8 }),
                             (
diff --git a/crates/noirc_abi/src/lib.rs b/crates/noirc_abi/src/lib.rs
@@ -12,7 +12,9 @@ use acvm::{
 use errors::AbiError;
 use input_parser::InputValue;
 use iter_extended::{try_btree_map, try_vecmap, vecmap};
-use noirc_frontend::{Signedness, Type, TypeBinding, TypeVariableKind, Visibility};
+use noirc_frontend::{
+    graph::CrateId, hir::Context, Signedness, Type, TypeBinding, TypeVariableKind, Visibility,
+};
 use serde::{Deserialize, Serialize};
 // This is the ABI used to bridge the different TOML formats for the initial
 // witness, the partial witness generator and the interpreter.
@@ -53,7 +55,7 @@ pub enum AbiType {
     },
     Boolean,
     Struct {
-        name: String,
+        path: String,
         #[serde(
             serialize_with = "serialization::serialize_struct_fields",
             deserialize_with = "serialization::deserialize_struct_fields"
@@ -117,7 +119,7 @@ pub enum Sign {
 
 impl AbiType {
     // TODO: Add `Context` argument for resolving fully qualified struct paths
-    pub fn from_type(typ: &Type) -> Self {
+    pub fn from_type(context: &Context, crate_id: &CrateId, typ: &Type) -> Self {
         // Note; use strict_eq instead of partial_eq when comparing field types
         // in this method, you most likely want to distinguish between public and private
         match typ {
@@ -127,7 +129,7 @@ impl AbiType {
                     .evaluate_to_u64()
                     .expect("Cannot have variable sized arrays as a parameter to main");
                 let typ = typ.as_ref();
-                Self::Array { length, typ: Box::new(Self::from_type(typ)) }
+                Self::Array { length, typ: Box::new(Self::from_type(context, crate_id, typ)) }
             }
             Type::Integer(sign, bit_width) => {
                 let sign = match sign {
@@ -139,8 +141,10 @@ impl AbiType {
             }
             Type::TypeVariable(binding, TypeVariableKind::IntegerOrField) => {
                 match &*binding.borrow() {
-                    TypeBinding::Bound(typ) => Self::from_type(typ),
-                    TypeBinding::Unbound(_) => Self::from_type(&Type::default_int_type()),
+                    TypeBinding::Bound(typ) => Self::from_type(context, crate_id, typ),
+                    TypeBinding::Unbound(_) => {
+                        Self::from_type(context, crate_id, &Type::default_int_type())
+                    }
                 }
             }
             Type::Bool => Self::Boolean,
@@ -157,8 +161,10 @@ impl AbiType {
             Type::Struct(def, ref args) => {
                 let struct_type = def.borrow();
                 let fields = struct_type.get_fields(args);
-                let fields = vecmap(fields, |(name, typ)| (name, Self::from_type(&typ)));
-                Self::Struct { fields, name: struct_type.name.to_string() }
+                let fields =
+                    vecmap(fields, |(name, typ)| (name, Self::from_type(context, crate_id, &typ)));
+                let path = context.fully_qualified_struct_name(crate_id, struct_type.id);
+                Self::Struct { fields, path }
             }
             Type::Tuple(_) => todo!("as_abi_type not yet implemented for tuple types"),
             Type::TypeVariable(_, _) => unreachable!(),
diff --git a/crates/noirc_abi/src/serialization.rs b/crates/noirc_abi/src/serialization.rs
@@ -88,11 +88,11 @@ mod tests {
         let deserialized_array: AbiParameter = serde_json::from_str(serialized_array).unwrap();
         assert_eq!(deserialized_array, expected_array);
 
-        let serialized_struct = "{   
+        let serialized_struct = "{
             \"name\":\"thing3\",
             \"type\": {
                 \"kind\":\"struct\",
-                \"name\": \"MyStruct\",
+                \"path\": \"MyStruct\",
                 \"fields\": [
                     {
                         \"name\": \"field1\",
@@ -120,7 +120,7 @@ mod tests {
         let expected_struct = AbiParameter {
             name: "thing3".to_string(),
             typ: AbiType::Struct {
-                name: "MyStruct".to_string(),
+                path: "MyStruct".to_string(),
                 fields: vec![
                     ("field1".to_string(), AbiType::Integer { sign: Sign::Unsigned, width: 3 }),
                     (
diff --git a/crates/noirc_driver/src/lib.rs b/crates/noirc_driver/src/lib.rs
@@ -134,8 +134,8 @@ pub fn compute_function_abi(
     let func_meta = context.def_interner.function_meta(&main_function);
 
     let (parameters, return_type) = func_meta.into_function_signature();
-    let parameters = into_abi_params(parameters, &context.def_interner);
-    let return_type = return_type.map(|typ| AbiType::from_type(&typ));
+    let parameters = into_abi_params(context, crate_id, parameters);
+    let return_type = return_type.map(|typ| AbiType::from_type(context, crate_id, &typ));
     Some((parameters, return_type))
 }
 
@@ -162,7 +162,7 @@ pub fn compile_main(
         }
     };
 
-    let compiled_program = compile_no_check(context, options, main)?;
+    let compiled_program = compile_no_check(context, &crate_id, options, main)?;
 
     if options.print_acir {
         println!("Compiled ACIR for main (unoptimized):");
@@ -186,7 +186,7 @@ pub fn compile_contracts(
     let mut errors = warnings;
 
     for contract in contracts {
-        match compile_contract(context, contract, options) {
+        match compile_contract(context, &crate_id, contract, options) {
             Ok(contract) => compiled_contracts.push(contract),
             Err(mut more_errors) => errors.append(&mut more_errors),
         }
@@ -223,14 +223,15 @@ fn has_errors(errors: &[FileDiagnostic], deny_warnings: bool) -> bool {
 /// Compile all of the functions associated with a Noir contract.
 fn compile_contract(
     context: &Context,
+    crate_id: &CrateId,
     contract: Contract,
     options: &CompileOptions,
 ) -> Result<CompiledContract, Vec<FileDiagnostic>> {
     let mut functions = Vec::new();
     let mut errors = Vec::new();
     for function_id in &contract.functions {
         let name = context.function_name(function_id).to_owned();
-        let function = match compile_no_check(context, options, *function_id) {
+        let function = match compile_no_check(context, crate_id, options, *function_id) {
             Ok(function) => function,
             Err(new_error) => {
                 errors.push(new_error);
@@ -268,13 +269,14 @@ fn compile_contract(
 #[allow(deprecated)]
 pub fn compile_no_check(
     context: &Context,
+    crate_id: &CrateId,
     options: &CompileOptions,
     main_function: FuncId,
 ) -> Result<CompiledProgram, FileDiagnostic> {
     let program = monomorphize(main_function, &context.def_interner);
 
     let (circuit, debug, abi) =
-        create_circuit(context, program, options.show_ssa, options.show_brillig)?;
+        create_circuit(context, crate_id, program, options.show_ssa, options.show_brillig)?;
 
     Ok(CompiledProgram { circuit, debug, abi })
 }
diff --git a/crates/noirc_evaluator/src/ssa.rs b/crates/noirc_evaluator/src/ssa.rs
@@ -19,7 +19,7 @@ use noirc_errors::debug_info::DebugInfo;
 
 use noirc_abi::Abi;
 
-use noirc_frontend::{hir::Context, monomorphization::ast::Program};
+use noirc_frontend::{graph::CrateId, hir::Context, monomorphization::ast::Program};
 
 use self::{abi_gen::gen_abi, acir_gen::GeneratedAcir, ir::function::RuntimeType, ssa_gen::Ssa};
 
@@ -82,6 +82,7 @@ pub(crate) fn optimize_into_acir(
 /// The output ACIR is is backend-agnostic and so must go through a transformation pass before usage in proof generation.
 pub fn create_circuit(
     context: &Context,
+    crate_id: &CrateId,
     program: Program,
     enable_ssa_logging: bool,
     enable_brillig_logging: bool,
@@ -96,7 +97,7 @@ pub fn create_circuit(
         ..
     } = optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?;
 
-    let abi = gen_abi(&context.def_interner, func_sig, &input_witnesses, return_witnesses.clone());
+    let abi = gen_abi(context, crate_id, func_sig, &input_witnesses, return_witnesses.clone());
     let public_abi = abi.clone().public_abi();
 
     let public_parameters =
diff --git a/crates/noirc_evaluator/src/ssa/abi_gen/mod.rs b/crates/noirc_evaluator/src/ssa/abi_gen/mod.rs
@@ -4,6 +4,8 @@ use acvm::acir::native_types::Witness;
 use iter_extended::{btree_map, vecmap};
 use noirc_abi::{Abi, AbiParameter, AbiType};
 use noirc_frontend::{
+    graph::CrateId,
+    hir::Context,
     hir_def::{
         function::{FunctionSignature, Param},
         stmt::HirPattern,
@@ -22,27 +24,32 @@ fn get_param_name<'a>(pattern: &HirPattern, interner: &'a NodeInterner) -> Optio
     }
 }
 
-pub fn into_abi_params(params: Vec<Param>, interner: &NodeInterner) -> Vec<AbiParameter> {
+pub fn into_abi_params(
+    context: &Context,
+    crate_id: &CrateId,
+    params: Vec<Param>,
+) -> Vec<AbiParameter> {
     vecmap(params, |(pattern, typ, vis)| {
-        let param_name = get_param_name(&pattern, interner)
+        let param_name = get_param_name(&pattern, &context.def_interner)
             .expect("Abi for tuple and struct parameters is unimplemented")
             .to_owned();
-        let as_abi = AbiType::from_type(&typ);
+        let as_abi = AbiType::from_type(context, crate_id, &typ);
         AbiParameter { name: param_name, typ: as_abi, visibility: vis.into() }
     })
 }
 
 /// Arranges a function signature and a generated circuit's return witnesses into a
 /// `noirc_abi::Abi`.
 pub(crate) fn gen_abi(
-    interner: &NodeInterner,
+    context: &Context,
+    crate_id: &CrateId,
     func_sig: FunctionSignature,
     input_witnesses: &[Witness],
     return_witnesses: Vec<Witness>,
 ) -> Abi {
     let (parameters, return_type) = func_sig;
-    let parameters = into_abi_params(parameters, interner);
-    let return_type = return_type.map(|typ| AbiType::from_type(&typ));
+    let parameters = into_abi_params(context, crate_id, parameters);
+    let return_type = return_type.map(|typ| AbiType::from_type(context, crate_id, &typ));
     let param_witnesses = param_witnesses_from_abi_param(&parameters, input_witnesses);
     Abi { parameters, return_type, param_witnesses, return_witnesses }
 }
diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs
@@ -225,6 +225,12 @@ impl std::ops::Index<CrateId> for CrateGraph {
         &self.arena[&crate_id]
     }
 }
+impl std::ops::Index<&CrateId> for CrateGraph {
+    type Output = CrateData;
+    fn index(&self, crate_id: &CrateId) -> &CrateData {
+        &self.arena[crate_id]
+    }
+}
 
 /// XXX: This is bare-bone for two reasons:
 // There are no display names currently
diff --git a/crates/noirc_frontend/src/hir/mod.rs b/crates/noirc_frontend/src/hir/mod.rs
@@ -4,9 +4,9 @@ pub mod resolution;
 pub mod scope;
 pub mod type_check;
 
-use crate::graph::{CrateGraph, CrateId};
+use crate::graph::{CrateGraph, CrateId, Dependency};
 use crate::hir_def::function::FuncMeta;
-use crate::node_interner::{FuncId, NodeInterner};
+use crate::node_interner::{FuncId, NodeInterner, StructId};
 use def_map::{Contract, CrateDefMap};
 use fm::FileManager;
 use std::collections::HashMap;
@@ -82,6 +82,29 @@ impl Context {
         }
     }
 
+    // TODO: This still feels hacky, need to fix
+    pub fn fully_qualified_struct_name(&self, crate_id: &CrateId, id: StructId) -> String {
+        let module_id = id.0;
+        let child_id = module_id.local_id.0;
+        let def_map =
+            self.def_map(&module_id.krate).expect("The local crate should be analyzed already");
+
+        let module = self.module(module_id);
+
+        let crate_name = &self.crate_graph[crate_id]
+            .dependencies
+            .iter()
+            .find_map(|dep| match dep {
+                Dependency { name, crate_id } if crate_id == &module_id.krate => Some(name),
+                _ => None,
+            })
+            .unwrap();
+
+        let module_path = def_map.get_module_path_with_separator(child_id, module.parent, "::");
+
+        format!("{crate_name}::{module_path}")
+    }
+
     pub fn function_meta(&self, func_id: &FuncId) -> FuncMeta {
         self.def_interner.function_meta(func_id)
     }
diff --git a/crates/wasm/src/compile.rs b/crates/wasm/src/compile.rs
@@ -105,7 +105,8 @@ pub fn compile(args: JsValue) -> JsValue {
     } else {
         let main = context.get_main_function(&crate_id).expect("Could not find main function!");
         let mut compiled_program =
-            compile_no_check(&context, &options.compile_options, main).expect("Compilation failed");
+            compile_no_check(&context, &crate_id, &options.compile_options, main)
+                .expect("Compilation failed");
 
         compiled_program.circuit = optimize_circuit(compiled_program.circuit);
 

Original file line number	Diff line number	Diff line change
`@@ -135,7 +135,7 @@ mod tests {`
`135`	`135`	`typed_param(`
`136`	`136`	`"d",`
`137`	`137`	`AbiType::Struct {`
`138`		`- name: String::from("MyStruct"),`
	`138`	`+ path: String::from("MyStruct"),`
`139`	`139`	`fields: vec![`
`140`	`140`	`(String::from("d1"), AbiType::Field),`
`141`	`141`	`(`
Original file line number	Diff line number	Diff line change
`@@ -133,7 +133,7 @@ mod serialization_tests {`
`133`	`133`	`AbiParameter {`
`134`	`134`	`name: "bar".into(),`
`135`	`135`	`typ: AbiType::Struct {`
`136`		`- name: "MyStruct".into(),`
	`136`	`+ path: "MyStruct".into(),`
`137`	`137`	`fields: vec![`
`138`	`138`	`("field1".into(), AbiType::Integer { sign: Sign::Unsigned, width: 8 }),`
`139`	`139`	`(`
Original file line number	Diff line number	Diff line change
`@@ -225,6 +225,12 @@ impl std::ops::Index<CrateId> for CrateGraph {`
`225`	`225`	`&self.arena[&crate_id]`
`226`	`226`	`}`
`227`	`227`	`}`
	`228`	`+impl std::ops::Index<&CrateId> for CrateGraph {`
	`229`	`+ type Output = CrateData;`
	`230`	`+ fn index(&self, crate_id: &CrateId) -> &CrateData {`
	`231`	`+ &self.arena[crate_id]`
	`232`	`+ }`
	`233`	`+}`
`228`	`234`
`229`	`235`	`/// XXX: This is bare-bone for two reasons:`
`230`	`236`	`// There are no display names currently`