Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(abi)!: Replace struct name with fully qualified struct path #2374

Merged
merged 2 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/check_cmd.rs
phated marked this conversation as resolved.
Show resolved Hide resolved
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 @@ -141,8 +141,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 @@ -94,7 +94,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 @@ -275,6 +275,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 @@ -86,6 +86,36 @@ impl Context {
}
}

phated marked this conversation as resolved.
Show resolved Hide resolved
/// 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