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

feat: Include full module path to structs in abi #2296

Closed
Closed
Show file tree
Hide file tree
Changes from 1 commit
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
5 changes: 3 additions & 2 deletions crates/nargo/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ pub fn prepare_dependencies(
for (dep_name, dep) in dependencies.iter() {
match dep {
Dependency::Remote { package } | Dependency::Local { package } => {
let crate_id = prepare_crate(context, &package.entry_path);
let crate_id =
prepare_crate(context, &package.entry_path, Some(package.name.to_owned()));
add_dep(context, parent_crate, crate_id, dep_name.clone());
prepare_dependencies(context, crate_id, &package.dependencies);
}
Expand All @@ -48,7 +49,7 @@ pub fn prepare_package(package: &Package) -> (Context, CrateId) {
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);

let crate_id = prepare_crate(&mut context, &package.entry_path);
let crate_id = prepare_crate(&mut context, &package.entry_path, Some(package.name.to_owned()));

prepare_dependencies(&mut context, crate_id, &package.dependencies);

Expand Down
12 changes: 8 additions & 4 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,15 +52,19 @@ pub fn compile_file(
context: &mut Context,
root_file: &Path,
) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
let crate_id = prepare_crate(context, root_file);
let crate_id = prepare_crate(context, root_file, None);
compile_main(context, crate_id, &CompileOptions::default())
}

/// Adds the file from the file system at `Path` to the crate graph
pub fn prepare_crate(context: &mut Context, file_name: &Path) -> CrateId {
pub fn prepare_crate(
context: &mut Context,
file_name: &Path,
package_name: Option<CrateName>,
) -> CrateId {
let root_file_id = context.file_manager.add_file(file_name).unwrap();

context.crate_graph.add_crate_root(root_file_id)
context.crate_graph.add_crate_root(root_file_id, package_name)
}

/// Adds a edge in the crate graph for two crates
Expand Down Expand Up @@ -132,7 +136,7 @@ pub fn compute_function_signature(
let main_function = context.get_main_function(crate_id)?;

let func_meta = context.def_interner.function_meta(&main_function);

println!("In compute_function_signature");
Some(func_meta.into_function_signature(&context.def_interner))
}

Expand Down
38 changes: 24 additions & 14 deletions crates/noirc_frontend/src/graph/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
// This version is also simpler due to not having macro_defs or proc_macros
// XXX: Edition may be reintroduced or some sort of versioning

use std::{fmt::Display, str::FromStr};
use std::{fmt::Display, option::Option, str::FromStr};

use fm::FileId;
use rustc_hash::{FxHashMap, FxHashSet};
Expand Down Expand Up @@ -76,6 +76,7 @@ pub const CHARACTER_BLACK_LIST: [char; 1] = ['-'];
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CrateData {
pub root_file_id: FileId,
pub name: Option<CrateName>,
pub dependencies: Vec<Dependency>,
}

Expand All @@ -94,7 +95,11 @@ impl Dependency {
}

impl CrateGraph {
pub fn add_crate_root(&mut self, file_id: FileId) -> CrateId {
pub fn get_crate(&self, crate_id: CrateId) -> Option<&CrateData> {
self.arena.get(&crate_id)
}

pub fn add_crate_root(&mut self, file_id: FileId, package_name: Option<CrateName>) -> CrateId {
let mut roots_with_file_id =
self.arena.iter().filter(|(_, crate_data)| crate_data.root_file_id == file_id);

Expand All @@ -103,7 +108,8 @@ impl CrateGraph {
return *file_id.0;
}

let data = CrateData { root_file_id: file_id, dependencies: Vec::new() };
let data =
CrateData { name: package_name, root_file_id: file_id, dependencies: Vec::new() };
let crate_id = CrateId::Crate(self.arena.len());
let prev = self.arena.insert(crate_id, data);
assert!(prev.is_none());
Expand All @@ -119,7 +125,11 @@ impl CrateGraph {
return *file_id.0;
}

let data = CrateData { root_file_id: file_id, dependencies: Vec::new() };
let data = CrateData {
name: Some(CrateName::from_str("std").unwrap()),
root_file_id: file_id,
dependencies: Vec::new(),
};
let crate_id = CrateId::Stdlib(self.arena.len());
let prev = self.arena.insert(crate_id, data);
assert!(prev.is_none());
Expand Down Expand Up @@ -239,9 +249,9 @@ mod tests {
let file_ids = dummy_file_ids(3);

let mut graph = CrateGraph::default();
let crate1 = graph.add_crate_root(file_ids[0]);
let crate2 = graph.add_crate_root(file_ids[1]);
let crate3 = graph.add_crate_root(file_ids[2]);
let crate1 = graph.add_crate_root(file_ids[0], None);
let crate2 = graph.add_crate_root(file_ids[1], None);
let crate3 = graph.add_crate_root(file_ids[2], None);

assert!(graph.add_dep(crate1, "crate2".parse().unwrap(), crate2).is_ok());
assert!(graph.add_dep(crate2, "crate3".parse().unwrap(), crate3).is_ok());
Expand All @@ -255,9 +265,9 @@ mod tests {
let file_id_1 = file_ids[1];
let file_id_2 = file_ids[2];
let mut graph = CrateGraph::default();
let crate1 = graph.add_crate_root(file_id_0);
let crate2 = graph.add_crate_root(file_id_1);
let crate3 = graph.add_crate_root(file_id_2);
let crate1 = graph.add_crate_root(file_id_0, None);
let crate2 = graph.add_crate_root(file_id_1, None);
let crate3 = graph.add_crate_root(file_id_2, None);
assert!(graph.add_dep(crate1, "crate2".parse().unwrap(), crate2).is_ok());
assert!(graph.add_dep(crate2, "crate3".parse().unwrap(), crate3).is_ok());
}
Expand All @@ -268,12 +278,12 @@ mod tests {
let file_id_1 = file_ids[1];
let file_id_2 = file_ids[2];
let mut graph = CrateGraph::default();
let _crate1 = graph.add_crate_root(file_id_0);
let _crate2 = graph.add_crate_root(file_id_1);
let _crate1 = graph.add_crate_root(file_id_0, None);
let _crate2 = graph.add_crate_root(file_id_1, None);

// Adding the same file, so the crate should be the same.
let crate3 = graph.add_crate_root(file_id_2);
let crate3_2 = graph.add_crate_root(file_id_2);
let crate3 = graph.add_crate_root(file_id_2, None);
let crate3_2 = graph.add_crate_root(file_id_2, None);
assert_eq!(crate3, crate3_2);
}
}
14 changes: 13 additions & 1 deletion crates/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,7 +360,19 @@ fn resolve_structs(
// without resolve_struct_fields non-deterministically unwrapping a value
// that isn't in the HashMap.
for (type_id, typ) in &structs {
context.def_interner.push_empty_struct(*type_id, typ);
let type_index = type_id.0.local_id.0;
let module_path = context.def_map(&crate_id).unwrap().get_module_path_with_separator(
type_index,
Some(typ.module_id),
"::",
);
let crate_name = context
.crate_graph
.get_crate(crate_id)
.and_then(|c| c.name.to_owned())
.map_or(String::new(), |n| n.to_string());
let full_path = format!("{crate_name}::{module_path}");
context.def_interner.push_empty_struct(*type_id, full_path, typ);
}

for (type_id, typ) in structs {
Expand Down
7 changes: 5 additions & 2 deletions crates/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,8 @@ pub struct StructType {

pub generics: Generics,
pub span: Span,

pub module_path: String,
}

/// Corresponds to generic lists such as `<T, U>` in the source
Expand Down Expand Up @@ -149,8 +151,9 @@ impl StructType {
span: Span,
fields: Vec<(Ident, Type)>,
generics: Generics,
module_path: String,
) -> StructType {
StructType { id, fields, name, span, generics }
StructType { id, fields, name, span, generics, module_path }
}

/// To account for cyclic references between structs, a struct's
Expand Down Expand Up @@ -986,7 +989,7 @@ impl Type {
let struct_type = def.borrow();
let fields = struct_type.get_fields(args);
let fields = vecmap(fields, |(name, typ)| (name, typ.as_abi_type()));
AbiType::Struct { fields, name: struct_type.name.to_string() }
AbiType::Struct { fields, name: struct_type.module_path.to_string() }
}
Type::Tuple(_) => todo!("as_abi_type not yet implemented for tuple types"),
Type::TypeVariable(_, _) => unreachable!(),
Expand Down
8 changes: 7 additions & 1 deletion crates/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,12 @@ impl NodeInterner {
self.id_to_type.insert(expr_id.into(), typ);
}

pub fn push_empty_struct(&mut self, type_id: StructId, typ: &UnresolvedStruct) {
pub fn push_empty_struct(
&mut self,
type_id: StructId,
module_path: String,
typ: &UnresolvedStruct,
) {
self.structs.insert(
type_id,
Shared::new(StructType::new(
Expand All @@ -320,6 +325,7 @@ impl NodeInterner {
let id = TypeVariableId(0);
(id, Shared::new(TypeBinding::Unbound(id)))
}),
module_path,
)),
);
}
Expand Down
11 changes: 7 additions & 4 deletions crates/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,12 @@ use noirc_driver::{
check_crate, compile_contracts, compile_no_check, prepare_crate, propagate_dep, CompileOptions,
CompiledContract,
};
use noirc_frontend::{graph::CrateGraph, hir::Context};
use noirc_frontend::{
graph::{CrateGraph, CrateName},
hir::Context,
};
use serde::{Deserialize, Serialize};
use std::path::Path;
use std::{path::Path, str::FromStr};
use wasm_bindgen::prelude::*;

#[derive(Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -60,7 +63,7 @@ impl Default for WASMCompileOptions {

fn add_noir_lib(context: &mut Context, crate_name: &str) {
let path_to_lib = Path::new(&crate_name).join("lib.nr");
let library_crate = prepare_crate(context, &path_to_lib);
let library_crate = prepare_crate(context, &path_to_lib, CrateName::from_str(crate_name).ok());

propagate_dep(context, library_crate, &crate_name.parse().unwrap());
}
Expand All @@ -84,7 +87,7 @@ pub fn compile(args: JsValue) -> JsValue {
let mut context = Context::new(fm, graph);

let path = Path::new(&options.entry_point);
let crate_id = prepare_crate(&mut context, path);
let crate_id = prepare_crate(&mut context, path, None);

for dependency in options.optional_dependencies_set {
add_noir_lib(&mut context, dependency.as_str());
Expand Down