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!: remove dep:: prefix #4946

Merged
merged 37 commits into from
Jun 17, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
167e153
use PathKind::Plain over Dep, remove PathKind::Dep, resolve both poss…
michaeljklein Apr 30, 2024
425116f
removing from aztec_macros, expected-to-be failing test_programs, etc
michaeljklein Apr 30, 2024
ec7eb40
only resolve path externally when unresolved internally, i.e. not whe…
michaeljklein Apr 30, 2024
91e003e
add PathKind::Dep back in 1) to ease distinguishing deprecated case, …
michaeljklein Apr 30, 2024
22ec8c7
cargo fmt/clippy
michaeljklein Apr 30, 2024
7ad8d05
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein Apr 30, 2024
552b29b
fix tests failing from deprecation warning
michaeljklein Apr 30, 2024
c8ba6f5
Update compiler/noirc_frontend/src/parser/errors.rs
michaeljklein May 1, 2024
51bbb0e
Update compiler/noirc_frontend/src/hir/resolution/import.rs
michaeljklein May 1, 2024
3493069
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein May 8, 2024
0da34b2
wip responding to review comments, distinguish deprecated path kind f…
michaeljklein May 9, 2024
a00b775
remove LitDep and associated warning, revert parser
michaeljklein May 9, 2024
2c6b20b
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein May 9, 2024
97bfe72
update previous-approach '::path' tests, simplify dep/mod overlap test
michaeljklein May 9, 2024
b47f8b7
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein May 10, 2024
8a4d138
add test for external vs local import without 'dep::'
michaeljklein May 10, 2024
085d1f6
convert overlapping_dep_and_mod to a compile failure by moving expect…
michaeljklein May 10, 2024
11f6235
add 'overlapping_dep_and_mod*' tests to excluded_dirs in formatting t…
michaeljklein May 10, 2024
d374c04
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein May 10, 2024
268490f
nargo fmt
michaeljklein May 10, 2024
0842de8
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein May 14, 2024
a4437f5
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein Jun 10, 2024
fb82de4
nargo fmt
michaeljklein Jun 10, 2024
3d36751
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein Jun 11, 2024
cea5f46
Merge branch 'master' into michaeljklein/combine-dep-crate
michaeljklein Jun 14, 2024
fd1b873
Merge branch 'master' into michaeljklein/combine-dep-crate
TomAFrench Jun 17, 2024
7b3eb42
chore: fmt
TomAFrench Jun 17, 2024
9111c2a
chore: update new paths
TomAFrench Jun 17, 2024
0a876b3
chore: fix naming of new test
TomAFrench Jun 17, 2024
407b22a
chore: restrict parallelism in `rebuild.sh`
TomAFrench Jun 17, 2024
ec2b819
chore: remove useless `use std` statements
TomAFrench Jun 17, 2024
c51cdd6
chore: clean up straggler
TomAFrench Jun 17, 2024
74526c4
chore: revert change to formatter test
TomAFrench Jun 17, 2024
a53fb5c
Update compiler/noirc_frontend/src/parser/errors.rs
TomAFrench Jun 17, 2024
ac9cdbc
Update docs/docs/noir/concepts/data_types/slices.mdx
TomAFrench Jun 17, 2024
8816095
chore: revert changes to formatter
TomAFrench Jun 17, 2024
787e020
Merge branch 'master' into michaeljklein/combine-dep-crate
TomAFrench Jun 17, 2024
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
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ fn generate_compute_note_hash_and_nullifier_source(
format!(
"
unconstrained fn compute_note_hash_and_nullifier(
contract_address: dep::aztec::protocol_types::address::AztecAddress,
contract_address: aztec::protocol_types::address::AztecAddress,
nonce: Field,
storage_slot: Field,
note_type_id: Field,
Expand All @@ -191,7 +191,7 @@ fn generate_compute_note_hash_and_nullifier_source(

let if_statements: Vec<String> = note_types.iter().map(|note_type| format!(
"if (note_type_id == {0}::get_note_type_id()) {{
dep::aztec::note::utils::compute_note_hash_and_nullifier({0}::deserialize_content, note_header, serialized_note)
aztec::note::utils::compute_note_hash_and_nullifier({0}::deserialize_content, note_header, serialized_note)
}}"
, note_type)).collect();

Expand All @@ -205,13 +205,13 @@ fn generate_compute_note_hash_and_nullifier_source(
format!(
"
unconstrained fn compute_note_hash_and_nullifier(
contract_address: dep::aztec::protocol_types::address::AztecAddress,
contract_address: aztec::protocol_types::address::AztecAddress,
nonce: Field,
storage_slot: Field,
note_type_id: Field,
serialized_note: [Field; {}]
) -> pub [Field; 4] {{
let note_header = dep::aztec::prelude::NoteHeader::new(contract_address, nonce, storage_slot);
let note_header = aztec::prelude::NoteHeader::new(contract_address, nonce, storage_slot);

{}
}}",
Expand Down
27 changes: 15 additions & 12 deletions aztec_macros/src/transforms/contract_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ use crate::utils::{
// for i in 0..third_arg.len() {
// args_acc = args_acc.append(third_arg[i].serialize().as_slice());
// }
// let args_hash = dep::aztec::hash::hash_args(args_acc);
// assert(args_hash == dep::aztec::oracle::arguments::pack_arguments(args_acc));
// let args_hash = aztec::hash::hash_args(args_acc);
// assert(args_hash == aztec::oracle::arguments::pack_arguments(args_acc));
// PublicCallInterface {
// target_contract: self.target_contract,
// selector: FunctionSelector::from_signature("SELECTOR_PLACEHOLDER"),
Expand All @@ -55,7 +55,10 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction) -> String {
.join(", ");
let fn_return_type: noirc_frontend::ast::UnresolvedType = func.return_type();

let fn_selector = format!("dep::aztec::protocol_types::abis::function_selector::FunctionSelector::from_signature(\"{}\")", SELECTOR_PLACEHOLDER);
let fn_selector = format!(
"aztec::protocol_types::abis::function_selector::FunctionSelector::from_signature(\"{}\")",
SELECTOR_PLACEHOLDER
);

let parameters = func.parameters();
let is_void = if matches!(fn_return_type.typ, UnresolvedTypeData::Unit) { "Void" } else { "" };
Expand Down Expand Up @@ -90,8 +93,8 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction) -> String {
format!(
"let mut args_acc: [Field] = &[];
{}
let args_hash = dep::aztec::hash::hash_args(args_acc);
assert(args_hash == dep::aztec::oracle::arguments::pack_arguments(args_acc));",
let args_hash = aztec::hash::hash_args(args_acc);
assert(args_hash == aztec::oracle::arguments::pack_arguments(args_acc));",
call_args
)
} else {
Expand All @@ -100,15 +103,15 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction) -> String {

let fn_body = format!(
"{}
dep::aztec::context::{}{}CallInterface {{
aztec::context::{}{}CallInterface {{
target_contract: self.target_contract,
selector: {},
args_hash,
}}",
args_hash, aztec_visibility, is_void, fn_selector,
);
format!(
"pub fn {}(self, {}) -> dep::aztec::context::{}{}CallInterface{} {{
"pub fn {}(self, {}) -> aztec::context::{}{}CallInterface{} {{
{}
}}",
fn_name, fn_parameters, aztec_visibility, is_void, return_type_hint, fn_body
Expand All @@ -122,7 +125,7 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction) -> String {
);
let fn_body = format!(
"{}
dep::aztec::context::Avm{}CallInterface {{
aztec::context::Avm{}CallInterface {{
target_contract: self.target_contract,
selector: {},
args: args_acc,
Expand All @@ -131,7 +134,7 @@ pub fn stub_function(aztec_visibility: &str, func: &NoirFunction) -> String {
args, is_void, fn_selector,
);
format!(
"pub fn {}(self, {}) -> dep::aztec::context::Avm{}CallInterface{} {{
"pub fn {}(self, {}) -> aztec::context::Avm{}CallInterface{} {{
{}
}}",
fn_name, fn_parameters, is_void, return_type_hint, fn_body
Expand All @@ -150,22 +153,22 @@ pub fn generate_contract_interface(
let contract_interface = format!(
"
struct {0} {{
target_contract: dep::aztec::protocol_types::address::AztecAddress
target_contract: aztec::protocol_types::address::AztecAddress
}}

impl {0} {{
{1}

pub fn at(
target_contract: dep::aztec::protocol_types::address::AztecAddress
target_contract: aztec::protocol_types::address::AztecAddress
) -> Self {{
Self {{ target_contract }}
}}
}}

#[contract_library_method]
pub fn at(
target_contract: dep::aztec::protocol_types::address::AztecAddress
target_contract: aztec::protocol_types::address::AztecAddress
) -> {0} {{
{0} {{ target_contract }}
}}
Expand Down
20 changes: 7 additions & 13 deletions aztec_macros/src/transforms/note_interface.rs
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ fn generate_note_get_header(
) -> Result<NoirFunction, AztecMacroError> {
let function_source = format!(
"
fn get_header(note: {}) -> dep::aztec::note::note_header::NoteHeader {{
fn get_header(note: {}) -> aztec::note::note_header::NoteHeader {{
note.{}
}}
",
Expand Down Expand Up @@ -229,7 +229,7 @@ fn generate_note_set_header(
) -> Result<NoirFunction, AztecMacroError> {
let function_source = format!(
"
fn set_header(self: &mut {}, header: dep::aztec::note::note_header::NoteHeader) {{
fn set_header(self: &mut {}, header: aztec::note::note_header::NoteHeader) {{
self.{} = header;
}}
",
Expand Down Expand Up @@ -418,7 +418,7 @@ fn generate_note_properties_fn(

// Automatically generate the method to compute the note's content hash as:
// fn compute_note_content_hash(self: NoteType) -> Field {
// dep::aztec::hash::pedersen_hash(self.serialize_content(), dep::aztec::protocol_types::constants::GENERATOR_INDEX__NOTE_CONTENT_HASH)
// aztec::hash::pedersen_hash(self.serialize_content(), aztec::protocol_types::constants::GENERATOR_INDEX__NOTE_CONTENT_HASH)
// }
//
fn generate_compute_note_content_hash(
Expand All @@ -428,7 +428,7 @@ fn generate_compute_note_content_hash(
let function_source = format!(
"
fn compute_note_content_hash(self: {}) -> Field {{
dep::aztec::hash::pedersen_hash(self.serialize_content(), dep::aztec::protocol_types::constants::GENERATOR_INDEX__NOTE_CONTENT_HASH)
aztec::hash::pedersen_hash(self.serialize_content(), aztec::protocol_types::constants::GENERATOR_INDEX__NOTE_CONTENT_HASH)
}}
",
note_type
Expand Down Expand Up @@ -487,10 +487,7 @@ fn generate_note_properties_struct_source(
.iter()
.filter_map(|(field_name, _)| {
if field_name != note_header_field_name {
Some(format!(
"{}: dep::aztec::note::note_getter_options::PropertySelector",
field_name
))
Some(format!("{}: aztec::note::note_getter_options::PropertySelector", field_name))
} else {
None
}
Expand Down Expand Up @@ -518,7 +515,7 @@ fn generate_note_properties_fn_source(
.filter_map(|(index, (field_name, _))| {
if field_name != note_header_field_name {
Some(format!(
"{}: dep::aztec::note::note_getter_options::PropertySelector {{ index: {}, offset: 0, length: 32 }}",
"{}: aztec::note::note_getter_options::PropertySelector {{ index: {}, offset: 0, length: 32 }}",
field_name,
index
))
Expand Down Expand Up @@ -595,10 +592,7 @@ fn generate_note_deserialize_content_source(
)
}
} else {
format!(
"{}: dep::aztec::note::note_header::NoteHeader::empty()",
note_header_field_name
)
format!("{}: aztec::note::note_header::NoteHeader::empty()", note_header_field_name)
}
})
.collect::<Vec<String>>()
Expand Down
4 changes: 2 additions & 2 deletions aztec_macros/src/transforms/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -452,10 +452,10 @@ pub fn generate_storage_layout(
let mut storable_fields_impl = vec![];

definition.fields.iter().enumerate().for_each(|(index, (field_ident, field_type))| {
storable_fields.push(format!("{}: dep::aztec::prelude::Storable<N{}>", field_ident, index));
storable_fields.push(format!("{}: aztec::prelude::Storable<N{}>", field_ident, index));
generic_args.push(format!("N{}", index));
storable_fields_impl.push(format!(
"{}: dep::aztec::prelude::Storable {{ slot: 0, typ: \"{}\" }}",
"{}: aztec::prelude::Storable {{ slot: 0, typ: \"{}\" }}",
field_ident,
field_type.to_string().replace("plain::", "")
));
Expand Down
2 changes: 1 addition & 1 deletion aztec_macros/src/utils/ast_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ macro_rules! chained_dep {
( $base:expr $(, $tail:expr)* ) => {
{
let mut base_path = ident_path($base);
base_path.kind = PathKind::Dep;
base_path.kind = PathKind::Plain;
$(
base_path.segments.push(ident($tail));
)*
Expand Down
2 changes: 1 addition & 1 deletion compiler/integration-tests/circuits/recursion/src/main.nr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use dep::std;
use std;

fn main(
verification_key: [Field; 114],
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/debug/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ impl DebugInstrumenter {
.join(",\n");
let (program, errors) = parse_program(&format!(
r#"
use dep::__debug::{{
use __debug::{{
__debug_var_assign,
__debug_var_drop,
__debug_fn_enter,
Expand Down
9 changes: 6 additions & 3 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -436,8 +436,11 @@ fn inject_prelude(
.map(|segment| crate::ast::Ident::new(segment.into(), Span::default()))
.collect();

let path =
Path { segments: segments.clone(), kind: crate::ast::PathKind::Dep, span: Span::default() };
let path = Path {
segments: segments.clone(),
kind: crate::ast::PathKind::Plain,
span: Span::default(),
};

if !crate_id.is_stdlib() {
if let Ok(PathResolution { module_def_id, error }) = path_resolver::resolve_path(
Expand All @@ -457,7 +460,7 @@ fn inject_prelude(
0,
ImportDirective {
module_id: crate_root,
path: Path { segments, kind: PathKind::Dep, span: Span::default() },
path: Path { segments, kind: PathKind::Plain, span: Span::default() },
alias: None,
is_prelude: true,
},
Expand Down
48 changes: 38 additions & 10 deletions compiler/noirc_frontend/src/hir/resolution/import.rs
Original file line number Diff line number Diff line change
Expand Up @@ -158,16 +158,34 @@ fn resolve_path_to_ns(
allow_contracts,
)
}
crate::ast::PathKind::Dep => resolve_external_dep(
def_map,
import_directive,
def_maps,
allow_contracts,
importing_crate,
),
crate::ast::PathKind::Plain => {
// Plain paths are only used to import children modules. It's possible to allow import of external deps, but maybe this distinction is better?
// In Rust they can also point to external Dependencies, if no children can be found with the specified name
// There is a possibility that the import path is empty
// In that case, early return
if import_path.is_empty() {
return resolve_name_in_module(
crate_id,
importing_crate,
import_path,
import_directive.module_id,
def_maps,
allow_contracts,
);
}

let current_mod_id = ModuleId { krate: crate_id, local_id: import_directive.module_id };
let current_mod = &def_map.modules[current_mod_id.local_id.0];
let first_segment = import_path.first().expect("ice: could not fetch first segment");
if current_mod.find_name(first_segment).is_none() {
// Resolve externally when first segment is unresolved
return resolve_external_dep(
def_map,
import_directive,
def_maps,
allow_contracts,
importing_crate,
);
}

resolve_name_in_module(
crate_id,
importing_crate,
Expand All @@ -177,6 +195,14 @@ fn resolve_path_to_ns(
allow_contracts,
)
}

crate::ast::PathKind::Dep => resolve_external_dep(
def_map,
import_directive,
def_maps,
allow_contracts,
importing_crate,
),
}
}

Expand Down Expand Up @@ -302,7 +328,9 @@ fn resolve_external_dep(
.ok_or_else(|| PathResolutionError::Unresolved(crate_name.to_owned()))?;

// Create an import directive for the dependency crate
let path_without_crate_name = &path[1..]; // XXX: This will panic if the path is of the form `use dep::std` Ideal algorithm will not distinguish between crate and module
// XXX: This will panic if the path is of the form `use std`. Ideal algorithm will not distinguish between crate and module
// See `singleton_import.nr` test case for a check that such cases are handled elsewhere.
let path_without_crate_name = &path[1..];

let path = Path {
segments: path_without_crate_name.to_vec(),
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/noir_parser.lalrpop
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ pub(crate) Path: Path = {
},

<lo:@L> "dep" "::" <segments:PathSegments> <hi:@R> => {
let kind = PathKind::Dep;
let kind = PathKind::Plain;
let span = Span::from(lo as u32..hi as u32);
Path { segments, kind, span }
},
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub enum ParserErrorReason {
MissingSeparatingSemi,
#[error("constrain keyword is deprecated")]
ConstrainDeprecated,
#[error("Expression is invalid in an array-length type: '{0}'. Only unsigned integer constants, globals, generics, +, -, *, /, and % may be used in this context.")]
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
#[error("'dep::' prefix in paths is deprecated")]
TomAFrench marked this conversation as resolved.
Show resolved Hide resolved
InvalidArrayLengthExpression(Expression),
#[error("Early 'return' is unsupported")]
EarlyReturn,
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1623,7 +1623,7 @@ mod test {
"use foo::{bar, hello}",
"use foo::{bar as bar2, hello}",
"use foo::{bar as bar2, hello::{foo}, nested::{foo, bar}}",
"use dep::{std::println, bar::baz}",
"use std::{println, bar::baz}",
];

let invalid_use_statements = [
Expand Down
9 changes: 5 additions & 4 deletions compiler/noirc_frontend/src/parser/parser/path.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn empty_path() -> impl NoirParser<Path> {
let make_path = |kind| move |_, span| Path { segments: Vec::new(), kind, span };
let path_kind = |key, kind| keyword(key).map_with_span(make_path(kind));

choice((path_kind(Keyword::Crate, PathKind::Crate), path_kind(Keyword::Dep, PathKind::Dep)))
choice((path_kind(Keyword::Crate, PathKind::Crate), path_kind(Keyword::Dep, PathKind::Plain)))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On line 19 there's also path_kind(Keyword::Dep, PathKind::Dep)... maybe that should have also been changed? Or maybe PathKind::Dep should be removed completely?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IIRC, this was kept both to ensure

  1. We can parse the old-style use statements
  2. We can distinguish which was which when formatting

}

pub(super) fn maybe_empty_path() -> impl NoirParser<Path> {
Expand All @@ -43,7 +43,8 @@ mod test {
("std", vec!["std"]),
("std::hash", vec!["std", "hash"]),
("std::hash::collections", vec!["std", "hash", "collections"]),
("dep::foo::bar", vec!["foo", "bar"]),
("foo::bar", vec!["foo", "bar"]),
("foo::bar", vec!["foo", "bar"]),
("crate::std::hash", vec!["std", "hash"]),
];

Expand All @@ -61,7 +62,7 @@ mod test {
fn parse_path_kinds() {
let cases = vec![
("std", PathKind::Plain),
("dep::hash::collections", PathKind::Dep),
("hash::collections", PathKind::Plain),
("crate::std::hash", PathKind::Crate),
];

Expand All @@ -72,7 +73,7 @@ mod test {

parse_all_failing(
path(),
vec!["dep", "crate", "crate::std::crate", "foo::bar::crate", "foo::dep"],
vec!["crate", "crate::std::crate", "foo::bar::crate", "foo::dep"],
);
}
}
2 changes: 1 addition & 1 deletion compiler/wasm/test/fixtures/deps/lib-a/src/lib.nr
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use dep::lib_b::assert_non_zero;
use lib_b::assert_non_zero;

pub fn divide(a: u64, b: u64) -> u64 {
assert_non_zero(b);
Expand Down
Loading
Loading