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.

Sign up for GitHub

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

Jump to bottom

chore: check for references to private functions during path resolution #4622

Merged
merged 2 commits into from
Apr 2, 2024
Merged

chore: check for references to private functions during path resolution #4622

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0f06ac6
feat: check for references to private paths during path resolution
TomAFrench Mar 23, 2024
445ff3c
chore: remove redundant errors
TomAFrench Mar 25, 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
23 changes: 17 additions & 6 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use crate::graph::CrateId;
use crate::hir::def_map::{CrateDefMap, LocalModuleId, ModuleId};
use crate::hir::resolution::errors::ResolverError;

use crate::hir::resolution::import::{resolve_import, ImportDirective};
use crate::hir::resolution::import::{resolve_import, ImportDirective, PathResolution};
use crate::hir::resolution::{
collect_impls, collect_trait_impls, path_resolver, resolve_free_functions, resolve_globals,
resolve_impls, resolve_structs, resolve_trait_by_path, resolve_trait_impls, resolve_traits,
Expand Down Expand Up @@ -56,8 +56,11 @@ impl UnresolvedFunctions {
for bound in &mut func.def.where_clause {
match resolve_trait_by_path(def_maps, module, bound.trait_bound.trait_path.clone())
{
Ok(trait_id) => {
Ok((trait_id, warning)) => {
bound.trait_bound.trait_id = Some(trait_id);
if let Some(warning) = warning {
errors.push(DefCollectorErrorKind::PathResolutionError(warning));
}
}
Err(err) => {
errors.push(err);
Expand Down Expand Up @@ -281,6 +284,13 @@ impl DefCollector {
for collected_import in def_collector.collected_imports {
match resolve_import(crate_id, &collected_import, &context.def_maps) {
Ok(resolved_import) => {
if let Some(error) = resolved_import.error {
errors.push((
DefCollectorErrorKind::PathResolutionError(error).into(),
root_file_id,
));
}

// Populate module namespaces according to the imports used
let current_def_map = context.def_maps.get_mut(&crate_id).unwrap();

Expand All @@ -299,9 +309,9 @@ impl DefCollector {
}
}
}
Err((error, module_id)) => {
Err(error) => {
let current_def_map = context.def_maps.get(&crate_id).unwrap();
let file_id = current_def_map.file_id(module_id);
let file_id = current_def_map.file_id(collected_import.module_id);
let error = DefCollectorErrorKind::PathResolutionError(error);
errors.push((error.into(), file_id));
}
Expand Down Expand Up @@ -409,12 +419,13 @@ fn inject_prelude(
Path { segments: segments.clone(), kind: crate::PathKind::Dep, span: Span::default() };

if !crate_id.is_stdlib() {
if let Ok(module_def) = path_resolver::resolve_path(
if let Ok(PathResolution { module_def_id, error }) = path_resolver::resolve_path(
&context.def_maps,
ModuleId { krate: crate_id, local_id: crate_root },
path,
) {
let module_id = module_def.as_module().expect("std::prelude should be a module");
assert!(error.is_none(), "Tried to add private item to prelude");
let module_id = module_def_id.as_module().expect("std::prelude should be a module");
let prelude = context.module(module_id).scope().names();

for path in prelude {
Expand Down
13 changes: 1 addition & 12 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub enum ResolverError {
#[error("path is not an identifier")]
PathIsNotIdent { span: Span },
#[error("could not resolve path")]
PathResolutionError(PathResolutionError),
PathResolutionError(#[from] PathResolutionError),
#[error("Expected")]
Expected { span: Span, expected: String, got: String },
#[error("Duplicate field in constructor")]
Expand Down Expand Up @@ -72,10 +72,6 @@ pub enum ResolverError {
NumericConstantInFormatString { name: String, span: Span },
#[error("Closure environment must be a tuple or unit type")]
InvalidClosureEnvironment { typ: Type, span: Span },
#[error("{name} is private and not visible from the current module")]
PrivateFunctionCalled { name: String, span: Span },
#[error("{name} is not visible from the current crate")]
NonCrateFunctionCalled { name: String, span: Span },
#[error("Nested slices are not supported")]
NestedSlices { span: Span },
#[error("#[recursive] attribute is only allowed on entry points to a program")]
Expand Down Expand Up @@ -290,13 +286,6 @@ impl From<ResolverError> for Diagnostic {
ResolverError::InvalidClosureEnvironment { span, typ } => Diagnostic::simple_error(
format!("{typ} is not a valid closure environment type"),
"Closure environment must be a tuple or unit type".to_string(), span),
// This will be upgraded to an error in future versions
ResolverError::PrivateFunctionCalled { span, name } => Diagnostic::simple_warning(
format!("{name} is private and not visible from the current module"),
format!("{name} is private"), span),
ResolverError::NonCrateFunctionCalled { span, name } => Diagnostic::simple_warning(
format!("{name} is not visible from the current crate"),
format!("{name} is only visible within its crate"), span),
ResolverError::NestedSlices { span } => Diagnostic::simple_error(
"Nested slices are not supported".into(),
"Try to use a constant sized array instead".into(),
Expand Down
Loading
Loading