Skip to content

Commit

Permalink
fix: don't report visibility errors when elaborating comptime value (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
asterite authored Nov 22, 2024
1 parent d94eb08 commit 3c361c9
Show file tree
Hide file tree
Showing 4 changed files with 127 additions and 1 deletion.
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 {
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() {
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");
}

0 comments on commit 3c361c9

Please sign in to comment.