Skip to content

Commit

Permalink
fix: add missing visibility for auto-import names (noir-lang/noir#6205)
Browse files Browse the repository at this point in the history
fix: don't warn twice when referring to private item (noir-lang/noir#6216)
feat: visibility for impl functions (noir-lang/noir#6179)
feat: Inclusive for loop (noir-lang/noir#6200)
chore: Release Noir(0.35.0) (noir-lang/noir#6030)
chore: revert mistaken stack size change (noir-lang/noir#6212)
chore: fix typo in code snippet (noir-lang/noir#6211)
feat: Sync from aztec-packages (noir-lang/noir#6210)
fix: ignore compression of blocks after msg.len in sha256_var (noir-lang/noir#6206)
feat(perf): Optimize array set from get (noir-lang/noir#6207)
chore(refactor): Array set optimization context struct for analysis (noir-lang/noir#6204)
fix: type variables by default should have Any kind (noir-lang/noir#6203)
feat: remove orphaned blocks from cfg to improve `simplify_cfg` pass. (noir-lang/noir#6198)
fix(ssa): Check if result of array set is used in value of another array set (noir-lang/noir#6197)
fix(docs): Rename recursion.md to recursion.mdx (noir-lang/noir#6195)
feat: skip `remove_enable_side_effects` pass on brillig functions (noir-lang/noir#6199)
feat!: Syncing TypeVariableKind with Kind (noir-lang/noir#6094)
feat(perf): Simplify the cfg after DIE (noir-lang/noir#6184)
feat: refactor SSA passes to run on individual functions (noir-lang/noir#6072)
chore: Remove macros_api module (noir-lang/noir#6190)
fix: ensure to_bytes returns the canonical decomposition (noir-lang/noir#6084)
chore: rename `DefinitionKind::GenericType` (noir-lang/noir#6182)
  • Loading branch information
AztecBot committed Oct 4, 2024
2 parents c0650d9 + dfbc6ac commit e19b8c1
Show file tree
Hide file tree
Showing 14 changed files with 153 additions and 59 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1b26440889379f491315cd9d088537b1898d57c5
c3cb38a7c4de6fc321b367eda3fca6d06e76b77a
Original file line number Diff line number Diff line change
Expand Up @@ -1385,7 +1385,7 @@ impl<'context> Elaborator<'context> {
}

if let Some(name) = name {
self.interner.register_global(global_id, name, self.module_id());
self.interner.register_global(global_id, name, global.visibility, self.module_id());
}

self.local_module = old_module;
Expand Down
34 changes: 21 additions & 13 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use crate::{
type_check::{Source, TypeCheckError},
},
hir_def::{
expr::{HirExpression, HirIdent, HirMethodReference, ImplKind},
expr::{HirExpression, HirIdent, HirMethodReference, ImplKind, TraitMethod},
stmt::HirPattern,
},
node_interner::{DefinitionId, DefinitionKind, ExprId, FuncId, GlobalId, TraitImplKind},
Expand Down Expand Up @@ -525,11 +525,15 @@ impl<'context> Elaborator<'context> {
}

fn resolve_variable(&mut self, path: Path) -> HirIdent {
if let Some((method, constraint, assumed)) = self.resolve_trait_generic_path(&path) {
if let Some(trait_path_resolution) = self.resolve_trait_generic_path(&path) {
if let Some(error) = trait_path_resolution.error {
self.push_err(error);
}

HirIdent {
location: Location::new(path.span, self.file),
id: self.interner.trait_method_id(method),
impl_kind: ImplKind::TraitMethod(method, constraint, assumed),
id: self.interner.trait_method_id(trait_path_resolution.method.method_id),
impl_kind: ImplKind::TraitMethod(trait_path_resolution.method),
}
} else {
// If the Path is being used as an Expression, then it is referring to a global from a separate module
Expand Down Expand Up @@ -595,8 +599,12 @@ impl<'context> Elaborator<'context> {
// We need to do this first since otherwise instantiating the type below
// will replace each trait generic with a fresh type variable, rather than
// the type used in the trait constraint (if it exists). See #4088.
if let ImplKind::TraitMethod(_, constraint, assumed) = &ident.impl_kind {
self.bind_generics_from_trait_constraint(constraint, *assumed, &mut bindings);
if let ImplKind::TraitMethod(method) = &ident.impl_kind {
self.bind_generics_from_trait_constraint(
&method.constraint,
method.assumed,
&mut bindings,
);
}

// An identifiers type may be forall-quantified in the case of generic functions.
Expand Down Expand Up @@ -634,18 +642,18 @@ impl<'context> Elaborator<'context> {
}
}

if let ImplKind::TraitMethod(_, mut constraint, assumed) = ident.impl_kind {
constraint.apply_bindings(&bindings);
if assumed {
let trait_generics = constraint.trait_generics.clone();
let object_type = constraint.typ;
if let ImplKind::TraitMethod(mut method) = ident.impl_kind {
method.constraint.apply_bindings(&bindings);
if method.assumed {
let trait_generics = method.constraint.trait_generics.clone();
let object_type = method.constraint.typ;
let trait_impl = TraitImplKind::Assumed { object_type, trait_generics };
self.interner.select_impl_for_expression(expr_id, trait_impl);
} else {
// Currently only one impl can be selected per expr_id, so this
// constraint needs to be pushed after any other constraints so
// that monomorphization can resolve this trait method to the correct impl.
self.push_trait_constraint(constraint, expr_id);
self.push_trait_constraint(method.constraint, expr_id);
}
}

Expand Down Expand Up @@ -731,7 +739,7 @@ impl<'context> Elaborator<'context> {
let mut constraint =
self.interner.get_trait(method_id.trait_id).as_constraint(span);
constraint.trait_generics = generics;
ImplKind::TraitMethod(method_id, constraint, false)
ImplKind::TraitMethod(TraitMethod { method_id, constraint, assumed: false })
}
};

Expand Down
46 changes: 28 additions & 18 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use crate::{
comptime::{Interpreter, Value},
def_collector::dc_crate::CompilationError,
def_map::ModuleDefId,
resolution::errors::ResolverError,
resolution::{errors::ResolverError, import::PathResolutionError},
type_check::{
generics::{Generic, TraitGenerics},
NoMatchingImplFoundError, Source, TypeCheckError,
Expand All @@ -24,15 +24,15 @@ use crate::{
hir_def::{
expr::{
HirBinaryOp, HirCallExpression, HirExpression, HirLiteral, HirMemberAccess,
HirMethodReference, HirPrefixExpression,
HirMethodReference, HirPrefixExpression, TraitMethod,
},
function::{FuncMeta, Parameters},
stmt::HirStatement,
traits::{NamedType, TraitConstraint},
},
node_interner::{
DefinitionKind, DependencyId, ExprId, FuncId, GlobalId, ImplSearchErrorKind, NodeInterner,
TraitId, TraitImplKind, TraitMethodId,
DefinitionKind, DependencyId, ExprId, GlobalId, ImplSearchErrorKind, NodeInterner, TraitId,
TraitImplKind, TraitMethodId,
},
token::SecondaryAttribute,
Generics, Kind, ResolvedGeneric, Type, TypeBinding, TypeBindings, TypeVariable,
Expand All @@ -44,6 +44,11 @@ use super::{lints, Elaborator};
pub const SELF_TYPE_NAME: &str = "Self";
pub const WILDCARD_TYPE: &str = "_";

pub(super) struct TraitPathResolution {
pub(super) method: TraitMethod,
pub(super) error: Option<PathResolutionError>,
}

impl<'context> Elaborator<'context> {
/// Translates an UnresolvedType to a Type with a `TypeKind::Normal`
pub(crate) fn resolve_type(&mut self, typ: UnresolvedType) -> Type {
Expand Down Expand Up @@ -522,10 +527,7 @@ impl<'context> Elaborator<'context> {
//
// Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not
// E.g. `t.method()` with `where T: Foo<Bar>` in scope will return `(Foo::method, T, vec![Bar])`
fn resolve_trait_static_method_by_self(
&mut self,
path: &Path,
) -> Option<(TraitMethodId, TraitConstraint, bool)> {
fn resolve_trait_static_method_by_self(&mut self, path: &Path) -> Option<TraitPathResolution> {
let trait_impl = self.current_trait_impl?;
let trait_id = self.interner.try_get_trait_implementation(trait_impl)?.borrow().trait_id;

Expand All @@ -537,7 +539,10 @@ impl<'context> Elaborator<'context> {
let the_trait = self.interner.get_trait(trait_id);
let method = the_trait.find_method(method.0.contents.as_str())?;
let constraint = the_trait.as_constraint(path.span);
return Some((method, constraint, true));
return Some(TraitPathResolution {
method: TraitMethod { method_id: method, constraint, assumed: true },
error: None,
});
}
}
None
Expand All @@ -547,16 +552,18 @@ impl<'context> Elaborator<'context> {
//
// Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not
// E.g. `t.method()` with `where T: Foo<Bar>` in scope will return `(Foo::method, T, vec![Bar])`
fn resolve_trait_static_method(
&mut self,
path: &Path,
) -> Option<(TraitMethodId, TraitConstraint, bool)> {
let func_id: FuncId = self.lookup(path.clone()).ok()?;
fn resolve_trait_static_method(&mut self, path: &Path) -> Option<TraitPathResolution> {
let path_resolution = self.resolve_path(path.clone()).ok()?;
let ModuleDefId::FunctionId(func_id) = path_resolution.module_def_id else { return None };

let meta = self.interner.function_meta(&func_id);
let the_trait = self.interner.get_trait(meta.trait_id?);
let method = the_trait.find_method(path.last_name())?;
let constraint = the_trait.as_constraint(path.span);
Some((method, constraint, false))
Some(TraitPathResolution {
method: TraitMethod { method_id: method, constraint, assumed: false },
error: path_resolution.error,
})
}

// This resolves a static trait method T::trait_method by iterating over the where clause
Expand All @@ -567,7 +574,7 @@ impl<'context> Elaborator<'context> {
fn resolve_trait_method_by_named_generic(
&mut self,
path: &Path,
) -> Option<(TraitMethodId, TraitConstraint, bool)> {
) -> Option<TraitPathResolution> {
if path.segments.len() != 2 {
return None;
}
Expand All @@ -581,7 +588,10 @@ impl<'context> Elaborator<'context> {

let the_trait = self.interner.get_trait(constraint.trait_id);
if let Some(method) = the_trait.find_method(path.last_name()) {
return Some((method, constraint, true));
return Some(TraitPathResolution {
method: TraitMethod { method_id: method, constraint, assumed: true },
error: None,
});
}
}
}
Expand All @@ -595,7 +605,7 @@ impl<'context> Elaborator<'context> {
pub(super) fn resolve_trait_generic_path(
&mut self,
path: &Path,
) -> Option<(TraitMethodId, TraitConstraint, bool)> {
) -> Option<TraitPathResolution> {
self.resolve_trait_static_method_by_self(path)
.or_else(|| self.resolve_trait_static_method(path))
.or_else(|| self.resolve_trait_method_by_named_generic(path))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -544,8 +544,8 @@ impl<'local, 'interner> Interpreter<'local, 'interner> {
InterpreterError::VariableNotInScope { location }
})?;

if let ImplKind::TraitMethod(method, _, _) = ident.impl_kind {
let method_id = resolve_trait_method(self.elaborator.interner, method, id)?;
if let ImplKind::TraitMethod(method) = ident.impl_kind {
let method_id = resolve_trait_method(self.elaborator.interner, method.method_id, id)?;
let typ = self.elaborator.interner.id_type(id).follow_bindings();
let bindings = self.elaborator.interner.get_instantiation_bindings(id).clone();
return Ok(Value::Function(method_id, typ, Rc::new(bindings)));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,7 @@ pub struct UnresolvedGlobal {
pub module_id: LocalModuleId,
pub global_id: GlobalId,
pub stmt_def: LetStatement,
pub visibility: ItemVisibility,
}

pub struct ModuleAttribute {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -357,7 +357,12 @@ impl<'a> ModCollector<'a> {
if context.def_interner.is_in_lsp_mode() {
let parent_module_id = ModuleId { krate, local_id: self.module_id };
let name = name.to_string();
context.def_interner.register_type_alias(type_alias_id, name, parent_module_id);
context.def_interner.register_type_alias(
type_alias_id,
name,
visibility,
parent_module_id,
);
}
}
errors
Expand Down Expand Up @@ -591,7 +596,12 @@ impl<'a> ModCollector<'a> {

if context.def_interner.is_in_lsp_mode() {
let parent_module_id = ModuleId { krate, local_id: self.module_id };
context.def_interner.register_trait(trait_id, name.to_string(), parent_module_id);
context.def_interner.register_trait(
trait_id,
name.to_string(),
visibility,
parent_module_id,
);
}

self.def_collector.items.traits.insert(trait_id, unresolved);
Expand Down Expand Up @@ -879,7 +889,7 @@ fn push_child_module(
);

if interner.is_in_lsp_mode() {
interner.register_module(mod_id, mod_name.0.contents.clone());
interner.register_module(mod_id, visibility, mod_name.0.contents.clone());
}
}

Expand Down Expand Up @@ -1221,7 +1231,7 @@ pub(crate) fn collect_global(

interner.set_doc_comments(ReferenceId::Global(global_id), doc_comments);

let global = UnresolvedGlobal { file_id, module_id, global_id, stmt_def: global };
let global = UnresolvedGlobal { file_id, module_id, global_id, stmt_def: global, visibility };
(global, error)
}

Expand Down
11 changes: 9 additions & 2 deletions noir/noir-repo/compiler/noirc_frontend/src/hir_def/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,14 @@ pub enum ImplKind {
/// and eventually linked to this id. The boolean indicates whether the impl
/// is already assumed to exist - e.g. when resolving a path such as `T::default`
/// when there is a corresponding `T: Default` constraint in scope.
TraitMethod(TraitMethodId, TraitConstraint, bool),
TraitMethod(TraitMethod),
}

#[derive(Debug, Clone)]
pub struct TraitMethod {
pub method_id: TraitMethodId,
pub constraint: TraitConstraint,
pub assumed: bool,
}

impl Eq for HirIdent {}
Expand Down Expand Up @@ -247,7 +254,7 @@ impl HirMethodCallExpression {
trait_generics,
span: location.span,
};
(id, ImplKind::TraitMethod(method_id, constraint, false))
(id, ImplKind::TraitMethod(TraitMethod { method_id, constraint, assumed: false }))
}
};
let func_var = HirIdent { location, id, impl_kind };
Expand Down
24 changes: 15 additions & 9 deletions noir/noir-repo/compiler/noirc_frontend/src/locations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -278,20 +278,23 @@ impl NodeInterner {
.next()
}

pub(crate) fn register_module(&mut self, id: ModuleId, name: String) {
let visibility = ItemVisibility::Public;
pub(crate) fn register_module(
&mut self,
id: ModuleId,
visibility: ItemVisibility,
name: String,
) {
self.register_name_for_auto_import(name, ModuleDefId::ModuleId(id), visibility, None);
}

pub(crate) fn register_global(
&mut self,
id: GlobalId,
name: String,
visibility: ItemVisibility,
parent_module_id: ModuleId,
) {
self.add_definition_location(ReferenceId::Global(id), Some(parent_module_id));

let visibility = ItemVisibility::Public;
self.register_name_for_auto_import(name, ModuleDefId::GlobalId(id), visibility, None);
}

Expand All @@ -306,22 +309,25 @@ impl NodeInterner {
self.register_name_for_auto_import(name, ModuleDefId::TypeId(id), visibility, None);
}

pub(crate) fn register_trait(&mut self, id: TraitId, name: String, parent_module_id: ModuleId) {
pub(crate) fn register_trait(
&mut self,
id: TraitId,
name: String,
visibility: ItemVisibility,
parent_module_id: ModuleId,
) {
self.add_definition_location(ReferenceId::Trait(id), Some(parent_module_id));

let visibility = ItemVisibility::Public;
self.register_name_for_auto_import(name, ModuleDefId::TraitId(id), visibility, None);
}

pub(crate) fn register_type_alias(
&mut self,
id: TypeAliasId,
name: String,
visibility: ItemVisibility,
parent_module_id: ModuleId,
) {
self.add_definition_location(ReferenceId::Alias(id), Some(parent_module_id));

let visibility = ItemVisibility::Public;
self.register_name_for_auto_import(name, ModuleDefId::TypeAliasId(id), visibility, None);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -850,8 +850,8 @@ impl<'interner> Monomorphizer<'interner> {
) -> Result<ast::Expression, MonomorphizationError> {
let typ = self.interner.id_type(expr_id);

if let ImplKind::TraitMethod(method, _, _) = ident.impl_kind {
return self.resolve_trait_method_expr(expr_id, typ, method);
if let ImplKind::TraitMethod(method) = ident.impl_kind {
return self.resolve_trait_method_expr(expr_id, typ, method.method_id);
}

// Ensure all instantiation bindings are bound.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ fn warns_on_use_of_private_exported_item() {
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 2); // An existing bug causes this error to be duplicated
assert_eq!(errors.len(), 1);

assert!(matches!(
&errors[0].0,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ fn errors_if_trying_to_access_public_function_inside_private_module() {
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 2); // There's a bug that duplicates this error
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::PathResolutionError(
PathResolutionError::Private(ident),
Expand Down
Loading

0 comments on commit e19b8c1

Please sign in to comment.