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
105 changes: 105 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,108 @@ 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);
}

#[test]
fn errors_if_accessing_private_struct_member_inside_comptime_context() {
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() {
comptime {
let foo = Foo::new(5);
let _ = foo.inner;
};
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::PathResolutionError(
PathResolutionError::Private(ident),
)) = &errors[0].0
else {
panic!("Expected a private error");
};

assert_eq!(ident.to_string(), "inner");
}

#[test]
fn errors_if_accessing_private_struct_member_inside_function_generated_at_comptime() {
let src = r#"
mod foo {
pub struct Foo {
foo_inner: Field,
}
}

use foo::Foo;

#[generate_inner_accessor]
struct Bar {
bar_inner: Foo,
}

comptime fn generate_inner_accessor(_s: StructDefinition) -> Quoted {
quote {
fn bar_get_foo_inner(x: Bar) -> Field {
x.bar_inner.foo_inner
}
}
}

fn main(x: Bar) {
let _ = bar_get_foo_inner(x);
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 1);

let CompilationError::ResolverError(ResolverError::PathResolutionError(
PathResolutionError::Private(ident),
)) = &errors[0].0
else {
panic!("Expected a private error");
};

assert_eq!(ident.to_string(), "foo_inner");
}
Loading