Skip to content
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
6 changes: 4 additions & 2 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1008,7 +1008,7 @@ impl<'f> Context<'f> {

#[cfg(test)]
mod test {
use acvm::{FieldElement, acir::AcirField};
use acvm::acir::AcirField;

use crate::{
assert_ssa_snapshot,
Expand All @@ -1019,7 +1019,6 @@ mod test {
instruction::{Instruction, TerminatorInstruction},
value::{Value, ValueId},
},
opt::flatten_cfg::Context,
},
};

Expand Down Expand Up @@ -1754,6 +1753,9 @@ mod test {
#[test]
#[cfg(feature = "bn254")]
fn test_grumpkin_points() {
use crate::ssa::opt::flatten_cfg::Context;
use acvm::acir::FieldElement;

let generators = Context::grumpkin_generators();
let len = generators.len();
for i in (0..len).step_by(2) {
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_frontend/src/elaborator/enums.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,7 @@ impl Elaborator<'_> {
Vec::new(),
false,
false,
ItemVisibility::Public,
);

let mut typ = self_type.clone();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,7 @@ pub(crate) fn get_struct_fields(
Ident::new(name.to_string(), location),
location,
Vec::new(),
ItemVisibility::Public,
);
let expected = Type::DataType(Shared::new(expected), Vec::new());
type_mismatch(value, expected, location)
Expand Down
26 changes: 24 additions & 2 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -609,6 +609,7 @@ impl ModCollector<'_> {
vec![],
false,
false,
ItemVisibility::Public,
);

if let Err((first_def, second_def)) = self.def_collector.def_map
Expand Down Expand Up @@ -1131,7 +1132,17 @@ pub fn collect_struct(
let span = unresolved.struct_def.location.span;
let attributes = unresolved.struct_def.attributes.clone();
let local_id = module_id.local_id;
interner.new_type(name, span, attributes, resolved_generics, krate, local_id, file_id)
let visibility = unresolved.struct_def.visibility;
interner.new_type(
name,
span,
attributes,
resolved_generics,
visibility,
krate,
local_id,
file_id,
)
}
Err(error) => {
definition_errors.push(error.into());
Expand Down Expand Up @@ -1225,7 +1236,17 @@ pub fn collect_enum(
let span = unresolved.enum_def.location.span;
let attributes = unresolved.enum_def.attributes.clone();
let local_id = module_id.local_id;
interner.new_type(name, span, attributes, resolved_generics, krate, local_id, file_id)
let visibility = unresolved.enum_def.visibility;
interner.new_type(
name,
span,
attributes,
resolved_generics,
visibility,
krate,
local_id,
file_id,
)
}
Err(error) => {
definition_errors.push(error.into());
Expand Down Expand Up @@ -1475,6 +1496,7 @@ pub(crate) fn collect_global(
global.attributes.clone(),
matches!(global.pattern, Pattern::Mutable { .. }),
global.comptime,
visibility,
);

// Add the statement to the scope so its path can be looked up later
Expand Down
15 changes: 12 additions & 3 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ pub struct DataType {
pub id: TypeId,

pub name: Ident,
pub visibility: ItemVisibility,

/// A type's body is private to force struct fields or enum variants to only be
/// accessed through get_field(), get_fields(), instantiate(), or similar functions
Expand Down Expand Up @@ -451,8 +452,14 @@ impl Ord for DataType {
}

impl DataType {
pub fn new(id: TypeId, name: Ident, location: Location, generics: Generics) -> DataType {
DataType { id, name, location, generics, body: TypeBody::None }
pub fn new(
id: TypeId,
name: Ident,
location: Location,
generics: Generics,
visibility: ItemVisibility,
) -> DataType {
DataType { id, name, location, generics, body: TypeBody::None, visibility }
}

/// To account for cyclic references between structs, a struct's
Expand Down Expand Up @@ -693,6 +700,7 @@ pub struct TypeAlias {
pub id: TypeAliasId,
pub typ: Type,
pub generics: Generics,
pub visibility: ItemVisibility,
pub location: Location,
}

Expand Down Expand Up @@ -733,8 +741,9 @@ impl TypeAlias {
location: Location,
typ: Type,
generics: Generics,
visibility: ItemVisibility,
) -> TypeAlias {
TypeAlias { id, typ, name, location, generics }
TypeAlias { id, typ, name, location, generics, visibility }
}

pub fn set_type_and_generics(&mut self, new_typ: Type, new_generics: Generics) {
Expand Down
61 changes: 59 additions & 2 deletions compiler/noirc_frontend/src/modules.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,12 @@
use std::collections::BTreeMap;

use crate::{
ast::Ident,
ast::{Ident, ItemVisibility},
graph::{CrateId, Dependency},
hir::def_map::{ModuleDefId, ModuleId},
hir::{
def_map::{CrateDefMap, ModuleDefId, ModuleId},
resolution::visibility::item_in_module_is_visible,
},
node_interner::{NodeInterner, ReferenceId},
};

Expand Down Expand Up @@ -209,3 +214,55 @@ pub fn module_def_id_relative_path(

Some(path)
}

/// Returns true if the given ModuleDefId is visible from the current module, given its visibility.
///
/// This will in turn check if the ModuleDefId parent modules are visible from the current module.
Comment thread
TomAFrench marked this conversation as resolved.
/// If `defining_module` is Some, it will be considered as the parent of the item to check
/// (this is the case when the item is re-exported with `pub use` or similar).
pub fn module_def_id_is_visible(
module_def_id: ModuleDefId,
current_module_id: ModuleId,
mut visibility: ItemVisibility,
mut defining_module: Option<ModuleId>,
interner: &NodeInterner,
def_maps: &BTreeMap<CrateId, CrateDefMap>,
dependencies: &[Dependency],
) -> bool {
// First find out which module we need to check.
// If a module is trying to be referenced, it's that module. Otherwise it's the module that contains the item.
let mut target_module_id = if let ModuleDefId::ModuleId(module_id) = module_def_id {
Some(module_id)
} else {
std::mem::take(&mut defining_module).or_else(|| get_parent_module(interner, module_def_id))
};

// Then check if it's visible, and upwards
while let Some(module_id) = target_module_id {
if !item_in_module_is_visible(def_maps, current_module_id, module_id, visibility) {
return false;
}

// If the target module isn't in the same crate as `module_id` or isn't in one of its
// dependencies, then it's not visible.
if module_id.krate != current_module_id.krate
&& dependencies.iter().all(|dep| dep.crate_id != module_id.krate)
{
return false;
}

target_module_id = std::mem::take(&mut defining_module).or_else(|| {
let module_data = &def_maps[&module_id.krate][module_id.local_id];
let parent_local_id = module_data.parent;
parent_local_id.map(|local_id| ModuleId { krate: module_id.krate, local_id })
});

// This is a bit strange, but the visibility is always that of the item inside another module,
// so the visibility we update here is for the next loop check.
visibility = interner
.try_module_attributes(&module_id)
.map_or(ItemVisibility::Public, |attributes| attributes.visibility);
}

true
}
15 changes: 11 additions & 4 deletions compiler/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -277,7 +277,7 @@ pub struct NodeInterner {
/// Only for LSP: a map of ModuleDefId to each module that pub or pub(crate) exports it.
/// In LSP this is used to offer importing the item via one of these exports if
/// the item is not visible where it's defined.
reexports: HashMap<ModuleDefId, Vec<Reexport>>,
pub reexports: HashMap<ModuleDefId, Vec<Reexport>>,
}

/// A dependency in the dependency graph may be a type or a definition.
Expand Down Expand Up @@ -621,6 +621,7 @@ pub struct GlobalInfo {
pub id: GlobalId,
pub definition_id: DefinitionId,
pub ident: Ident,
pub visibility: ItemVisibility,
pub local_id: LocalModuleId,
pub crate_id: CrateId,
pub location: Location,
Expand Down Expand Up @@ -799,14 +800,15 @@ impl NodeInterner {
span: Span,
attributes: Vec<SecondaryAttribute>,
generics: Generics,
visibility: ItemVisibility,
krate: CrateId,
local_id: LocalModuleId,
file_id: FileId,
) -> TypeId {
let type_id = TypeId(ModuleId { krate, local_id });

let location = Location::new(span, file_id);
let new_type = DataType::new(type_id, name, location, generics);
let new_type = DataType::new(type_id, name, location, generics, visibility);
self.data_types.insert(type_id, Shared::new(new_type));
self.type_attributes.insert(type_id, attributes);
type_id
Expand All @@ -825,6 +827,7 @@ impl NodeInterner {
typ.type_alias_def.location,
Type::Error,
generics,
typ.type_alias_def.visibility,
)));

type_id
Expand Down Expand Up @@ -900,6 +903,7 @@ impl NodeInterner {
attributes: Vec<SecondaryAttribute>,
mutable: bool,
comptime: bool,
visibility: ItemVisibility,
) -> GlobalId {
let id = GlobalId(self.globals.len());
let location = Location::new(ident.span(), file);
Expand All @@ -916,6 +920,7 @@ impl NodeInterner {
crate_id,
let_statement,
location,
visibility,
value: GlobalValue::Unresolved,
});
self.global_attributes.insert(id, attributes);
Expand All @@ -937,12 +942,14 @@ impl NodeInterner {
attributes: Vec<SecondaryAttribute>,
mutable: bool,
comptime: bool,
visibility: ItemVisibility,
) -> GlobalId {
let statement = self.push_stmt(HirStatement::Error);
let location = name.location();

let id = self
.push_global(name, local_id, crate_id, statement, file, attributes, mutable, comptime);
let id = self.push_global(
name, local_id, crate_id, statement, file, attributes, mutable, comptime, visibility,
);
self.push_stmt_location(statement, location);
id
}
Expand Down
1 change: 0 additions & 1 deletion tooling/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,6 @@ mod trait_impl_method_stub_generator;
mod types;
mod use_segment_positions;
mod utils;
mod visibility;
mod with_file;

#[cfg(test)]
Expand Down
4 changes: 1 addition & 3 deletions tooling/lsp/src/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ use noirc_frontend::{
ast::{Ident, ItemVisibility},
graph::{CrateId, Dependency},
hir::def_map::{CrateDefMap, ModuleDefId, ModuleId},
modules::get_parent_module,
modules::{get_parent_module, module_def_id_is_visible},
node_interner::{NodeInterner, Reexport},
};

use crate::visibility::module_def_id_is_visible;

/// Finds a visible reexport for any ancestor module of the given ModuleDefId,
pub(crate) fn get_ancestor_module_reexport(
module_def_id: ModuleDefId,
Expand Down
3 changes: 2 additions & 1 deletion tooling/lsp/src/requests/code_action.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ use fm::{FileId, FileMap, PathString};
use noirc_errors::Span;
use noirc_frontend::{
ParsedModule,
modules::module_def_id_is_visible,
parser::{Item, ItemKind, ParsedSubModule},
};
use noirc_frontend::{
Expand All @@ -28,7 +29,7 @@ use noirc_frontend::{

use crate::{
LspState, modules::get_ancestor_module_reexport, use_segment_positions::UseSegmentPositions,
utils, visibility::module_def_id_is_visible,
utils,
};

use super::{process_request, to_lsp_location};
Expand Down
3 changes: 2 additions & 1 deletion tooling/lsp/src/requests/completion.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,7 @@ use noirc_frontend::{
},
},
hir_def::traits::Trait,
modules::module_def_id_is_visible,
node_interner::{FuncId, NodeInterner, ReferenceId, TypeId},
parser::{Item, ItemKind, ParsedSubModule},
token::{MetaAttribute, MetaAttributeName, Token, Tokens},
Expand All @@ -44,7 +45,7 @@ use sort_text::underscore_sort_text;
use crate::{
LspState, requests::to_lsp_location,
trait_impl_method_stub_generator::TraitImplMethodStubGenerator,
use_segment_positions::UseSegmentPositions, utils, visibility::module_def_id_is_visible,
use_segment_positions::UseSegmentPositions, utils,
};

use super::{TraitReexport, process_request};
Expand Down
Loading
Loading