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

fix: don't report visibility errors when elaborating comptime value #6498

Merged
merged 7 commits into from
Nov 22, 2024
12 changes: 11 additions & 1 deletion compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -907,7 +907,17 @@ impl<'context> Elaborator<'context> {

let location = Location::new(span, self.file);
match value.into_expression(self.interner, location) {
Ok(new_expr) => self.elaborate_expression(new_expr),
Ok(new_expr) => {
// At this point the Expression was already elaborated and we got a Value.
// We'll elaborate this value turned into Expression to inline it and get
// an ExprId and Type, but we don't want any visibility errors to happen
// here (they could if we have `Foo { inner: 5 }` and `inner` is not
// accessible from where this expression is being elaborated).
self.silence_field_visibility_errors += 1;
let value = self.elaborate_expression(new_expr);
self.silence_field_visibility_errors -= 1;
value
}
Err(error) => make_error(self, error),
}
}
Expand Down
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -164,6 +164,12 @@ pub struct Elaborator<'context> {
unresolved_globals: BTreeMap<GlobalId, UnresolvedGlobal>,

pub(crate) interpreter_call_stack: im::Vector<Location>,

/// If greater than 0, field visibility errors won't be reported.
/// This is used when elaborating a comptime expression that is a struct constructor
/// like `Foo { inner: 5 }`: in that case we already elaborated the code that led to
/// that comptime value and any visibility errors were already reported.
silence_field_visibility_errors: usize,
}

#[derive(Default)]
Expand Down Expand Up @@ -213,6 +219,7 @@ impl<'context> Elaborator<'context> {
current_trait: None,
interpreter_call_stack,
in_comptime_context: false,
silence_field_visibility_errors: 0,
}
}

Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -509,6 +509,10 @@ impl<'context> Elaborator<'context> {
visibility: ItemVisibility,
span: Span,
) {
if self.silence_field_visibility_errors > 0 {
asterite marked this conversation as resolved.
Show resolved Hide resolved
return;
}

if !struct_member_is_visible(struct_type.id, visibility, self.module_id(), self.def_maps) {
self.push_err(ResolverError::PathResolutionError(PathResolutionError::Private(
Ident::new(field_name.to_string(), span),
Expand Down
25 changes: 25 additions & 0 deletions compiler/noirc_frontend/src/tests/visibility.rs
Original file line number Diff line number Diff line change
Expand Up @@ -493,3 +493,28 @@ fn does_not_error_if_referring_to_top_level_private_module_via_crate() {
"#;
assert_no_errors(src);
}

#[test]
fn visibility_bug_inside_comptime() {
asterite marked this conversation as resolved.
Show resolved Hide resolved
let src = r#"
mod foo {
pub struct Foo {
inner: Field,
}

impl Foo {
pub fn new(inner: Field) -> Self {
Self { inner }
}
}
}

use foo::Foo;

fn main() {
let _ = Foo::new(5);
let _ = comptime { Foo::new(5) };
}
"#;
assert_no_errors(src);
}
Loading