Skip to content

Commit

Permalink
chore(abi)!: Replace struct name with fully qualified struct path (#2374
Browse files Browse the repository at this point in the history
)
  • Loading branch information
phated authored Aug 30, 2023
1 parent 4554287 commit 0920dd0
Show file tree
Hide file tree
Showing 9 changed files with 64 additions and 25 deletions.
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
(
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_abi/src/input_parser/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }),
(
Expand Down
20 changes: 11 additions & 9 deletions crates/noirc_abi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ 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::{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.
Expand Down Expand Up @@ -53,7 +53,7 @@ pub enum AbiType {
},
Boolean,
Struct {
name: String,
path: String,
#[serde(
serialize_with = "serialization::serialize_struct_fields",
deserialize_with = "serialization::deserialize_struct_fields"
Expand Down Expand Up @@ -116,8 +116,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, 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 {
Expand All @@ -127,7 +126,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, typ)) }
}
Type::Integer(sign, bit_width) => {
let sign = match sign {
Expand All @@ -139,8 +138,8 @@ 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, typ),
TypeBinding::Unbound(_) => Self::from_type(context, &Type::default_int_type()),
}
}
Type::Bool => Self::Boolean,
Expand All @@ -157,8 +156,11 @@ 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, &typ)));
// For the ABI, we always want to resolve the struct paths from the root crate
let path =
context.fully_qualified_struct_path(context.root_crate_id(), struct_type.id);
Self::Struct { fields, path }
}
Type::Tuple(_) => todo!("AbiType::from_type not yet implemented for tuple types"),
Type::TypeVariable(_, _) => unreachable!(),
Expand Down
6 changes: 3 additions & 3 deletions crates/noirc_abi/src/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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\",
Expand Down Expand Up @@ -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 }),
(
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,8 +127,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, parameters);
let return_type = return_type.map(|typ| AbiType::from_type(context, &typ));
Some((parameters, return_type))
}

Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ pub fn create_circuit(
current_witness_index, return_witnesses, locations, input_witnesses, ..
} = generated_acir;

let abi = gen_abi(&context.def_interner, func_sig, &input_witnesses, return_witnesses.clone());
let abi = gen_abi(context, func_sig, &input_witnesses, return_witnesses.clone());
let public_abi = abi.clone().public_abi();

let public_parameters =
Expand Down
13 changes: 7 additions & 6 deletions crates/noirc_evaluator/src/ssa/abi_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use acvm::acir::native_types::Witness;
use iter_extended::{btree_map, vecmap};
use noirc_abi::{Abi, AbiParameter, AbiType};
use noirc_frontend::{
hir::Context,
hir_def::{
function::{FunctionSignature, Param},
stmt::HirPattern,
Expand All @@ -22,27 +23,27 @@ 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, 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, &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,
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, parameters);
let return_type = return_type.map(|typ| AbiType::from_type(context, &typ));
let param_witnesses = param_witnesses_from_abi_param(&parameters, input_witnesses);
Abi { parameters, return_type, param_witnesses, return_witnesses }
}
Expand Down
6 changes: 6 additions & 0 deletions crates/noirc_frontend/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,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
Expand Down
34 changes: 32 additions & 2 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -92,6 +92,36 @@ impl Context {
}
}

/// Returns a fully-qualified path to the given [StructId] from the given [CrateId]. This function also
/// account for the crate names of dependencies.
///
/// For example, if you project contains a `main.nr` and `foo.nr` and you provide the `main_crate_id` and the
/// `bar_struct_id` where the `Bar` struct is inside `foo.nr`, this function would return `foo::Bar` as a [String].
pub fn fully_qualified_struct_path(&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 module_path = def_map.get_module_path_with_separator(child_id, module.parent, "::");

if &module_id.krate == crate_id {
module_path
} else {
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,
})
.expect("The Struct was supposed to be defined in a dependency");
format!("{crate_name}::{module_path}")
}
}

pub fn function_meta(&self, func_id: &FuncId) -> FuncMeta {
self.def_interner.function_meta(func_id)
}
Expand Down

0 comments on commit 0920dd0

Please sign in to comment.