Skip to content

Commit

Permalink
fix: visibility for impl methods (noir-lang/noir#6261)
Browse files Browse the repository at this point in the history
feat: Add `checked_transmute` (noir-lang/noir#6262)
feat!: kind size checks (noir-lang/noir#6137)
feat: don't crash LSP when there are errors resolving the workspace (noir-lang/noir#6257)
fix: don't warn on unuse global if it has an abi annotation (noir-lang/noir#6258)
fix: don't warn on unused struct that has an abi annotation (noir-lang/noir#6254)
feat: don't suggest private struct fields in LSP (noir-lang/noir#6256)
feat: visibility for struct fields (noir-lang/noir#6221)
fix: handle dfg databus in SSA normalization (noir-lang/noir#6249)
fix: homogeneous input points for EC ADD (noir-lang/noir#6241)
chore: add regression test for #5756 (noir-lang/noir#5770)
feat: allow `unconstrained` after visibility (noir-lang/noir#6246)
feat: optimize `Quoted::as_expr` by parsing just once (noir-lang/noir#6237)
fix(frontend): Do not warn when a nested struct is provided as input to main (noir-lang/noir#6239)
fix!: Change tag attributes to require a ' prefix (noir-lang/noir#6235)
feat: recover from '=' instead of ':' in struct constructor/pattern (noir-lang/noir#6236)
  • Loading branch information
AztecBot committed Oct 10, 2024
2 parents 8ebe3c2 + 7c635e6 commit caab224
Show file tree
Hide file tree
Showing 116 changed files with 1,962 additions and 635 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
f6a7306436ea1a37ec7f3b884721b50467e9a063
70cbeb4322a0b11c1c167ab27bf0408d04fe7b7d
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,11 @@ impl Context {
terminator.mutate_blocks(|old_block| self.new_ids.blocks[&old_block]);
new_function.dfg.set_block_terminator(new_block_id, terminator);
}

// Also map the values in the databus
let old_databus = &old_function.dfg.data_bus;
new_function.dfg.data_bus = old_databus
.map_values(|old_value| self.new_ids.map_value(new_function, old_function, old_value));
}
}

Expand Down
8 changes: 5 additions & 3 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub use visitor::Visitor;
pub use expression::*;
pub use function::*;

use acvm::FieldElement;
pub use docs::*;
use noirc_errors::Span;
use serde::{Deserialize, Serialize};
Expand Down Expand Up @@ -219,7 +220,7 @@ pub struct UnaryRhsMethodCall {
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum UnresolvedTypeExpression {
Variable(Path),
Constant(u32, Span),
Constant(FieldElement, Span),
BinaryOperation(
Box<UnresolvedTypeExpression>,
BinaryTypeOperator,
Expand Down Expand Up @@ -421,12 +422,13 @@ impl UnresolvedTypeExpression {
fn from_expr_helper(expr: Expression) -> Result<UnresolvedTypeExpression, Expression> {
match expr.kind {
ExpressionKind::Literal(Literal::Integer(int, _)) => match int.try_to_u32() {
Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)),
Some(int) => Ok(UnresolvedTypeExpression::Constant(int.into(), expr.span)),
None => Err(expr),
},
ExpressionKind::Variable(path) => Ok(UnresolvedTypeExpression::Variable(path)),
ExpressionKind::Prefix(prefix) if prefix.operator == UnaryOp::Minus => {
let lhs = Box::new(UnresolvedTypeExpression::Constant(0, expr.span));
let lhs =
Box::new(UnresolvedTypeExpression::Constant(FieldElement::zero(), expr.span));
let rhs = Box::new(UnresolvedTypeExpression::from_expr_helper(prefix.rhs)?);
let op = BinaryTypeOperator::Subtraction;
Ok(UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, expr.span))
Expand Down
7 changes: 7 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,15 @@ pub struct NoirStruct {
pub span: Span,
}

impl NoirStruct {
pub fn is_abi(&self) -> bool {
self.attributes.iter().any(|attr| attr.is_abi())
}
}

#[derive(Clone, Debug, PartialEq, Eq)]
pub struct StructField {
pub visibility: ItemVisibility,
pub name: Ident,
pub typ: UnresolvedType,
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
use acvm::{AcirField, FieldElement};
use iter_extended::vecmap;
use noirc_errors::{Location, Span};
use regex::Regex;
Expand All @@ -6,13 +7,15 @@ use rustc_hash::FxHashSet as HashSet;
use crate::{
ast::{
ArrayLiteral, BlockExpression, CallExpression, CastExpression, ConstructorExpression,
Expression, ExpressionKind, Ident, IfExpression, IndexExpression, InfixExpression, Lambda,
Literal, MemberAccessExpression, MethodCallExpression, PrefixExpression, StatementKind,
UnaryOp, UnresolvedTypeData, UnresolvedTypeExpression,
Expression, ExpressionKind, Ident, IfExpression, IndexExpression, InfixExpression,
ItemVisibility, Lambda, Literal, MemberAccessExpression, MethodCallExpression,
PrefixExpression, StatementKind, UnaryOp, UnresolvedTypeData, UnresolvedTypeExpression,
},
hir::{
comptime::{self, InterpreterError},
resolution::errors::ResolverError,
resolution::{
errors::ResolverError, import::PathResolutionError, visibility::method_call_is_visible,
},
type_check::{generics::TraitGenerics, TypeCheckError},
},
hir_def::{
Expand Down Expand Up @@ -161,7 +164,7 @@ impl<'context> Elaborator<'context> {
(Lit(int), self.polymorphic_integer_or_field())
}
Literal::Str(str) | Literal::RawStr(str, _) => {
let len = Type::Constant(str.len() as u32, Kind::u32());
let len = Type::Constant(str.len().into(), Kind::u32());
(Lit(HirLiteral::Str(str)), Type::String(Box::new(len)))
}
Literal::FmtStr(str) => self.elaborate_fmt_string(str, span),
Expand Down Expand Up @@ -203,15 +206,15 @@ impl<'context> Elaborator<'context> {
elem_id
});

let length = Type::Constant(elements.len() as u32, Kind::u32());
let length = Type::Constant(elements.len().into(), Kind::u32());
(HirArrayLiteral::Standard(elements), first_elem_type, length)
}
ArrayLiteral::Repeated { repeated_element, length } => {
let span = length.span;
let length =
UnresolvedTypeExpression::from_expr(*length, span).unwrap_or_else(|error| {
self.push_err(ResolverError::ParserError(Box::new(error)));
UnresolvedTypeExpression::Constant(0, span)
UnresolvedTypeExpression::Constant(FieldElement::zero(), span)
});

let length = self.convert_expression_type(length, &Kind::u32(), span);
Expand Down Expand Up @@ -267,7 +270,7 @@ impl<'context> Elaborator<'context> {
}
}

let len = Type::Constant(str.len() as u32, Kind::u32());
let len = Type::Constant(str.len().into(), Kind::u32());
let typ = Type::FmtString(Box::new(len), Box::new(Type::Tuple(capture_types)));
(HirExpression::Literal(HirLiteral::FmtStr(str, fmt_str_idents)), typ)
}
Expand Down Expand Up @@ -448,6 +451,8 @@ impl<'context> Elaborator<'context> {
let method_call =
HirMethodCallExpression { method, object, arguments, location, generics };

self.check_method_call_visibility(func_id, &object_type, &method_call.method);

// Desugar the method call into a normal, resolved function call
// so that the backend doesn't need to worry about methods
// TODO: update object_type here?
Expand Down Expand Up @@ -486,6 +491,20 @@ impl<'context> Elaborator<'context> {
}
}

fn check_method_call_visibility(&mut self, func_id: FuncId, object_type: &Type, name: &Ident) {
if !method_call_is_visible(
object_type,
func_id,
self.module_id(),
self.interner,
self.def_maps,
) {
self.push_err(ResolverError::PathResolutionError(PathResolutionError::Private(
name.clone(),
)));
}
}

fn elaborate_constructor(
&mut self,
constructor: ConstructorExpression,
Expand Down Expand Up @@ -548,7 +567,7 @@ impl<'context> Elaborator<'context> {
let generics = struct_generics.clone();

let fields = constructor.fields;
let field_types = r#type.borrow().get_fields(&struct_generics);
let field_types = r#type.borrow().get_fields_with_visibility(&struct_generics);
let fields =
self.resolve_constructor_expr_fields(struct_type.clone(), field_types, fields, span);
let expr = HirExpression::Constructor(HirConstructorExpression {
Expand Down Expand Up @@ -576,7 +595,7 @@ impl<'context> Elaborator<'context> {
fn resolve_constructor_expr_fields(
&mut self,
struct_type: Shared<StructType>,
field_types: Vec<(String, Type)>,
field_types: Vec<(String, ItemVisibility, Type)>,
fields: Vec<(Ident, Expression)>,
span: Span,
) -> Vec<(Ident, ExprId)> {
Expand All @@ -588,10 +607,11 @@ impl<'context> Elaborator<'context> {
let expected_field_with_index = field_types
.iter()
.enumerate()
.find(|(_, (name, _))| name == &field_name.0.contents);
let expected_index = expected_field_with_index.map(|(index, _)| index);
.find(|(_, (name, _, _))| name == &field_name.0.contents);
let expected_index_and_visibility =
expected_field_with_index.map(|(index, (_, visibility, _))| (index, visibility));
let expected_type =
expected_field_with_index.map(|(_, (_, typ))| typ).unwrap_or(&Type::Error);
expected_field_with_index.map(|(_, (_, _, typ))| typ).unwrap_or(&Type::Error);

let field_span = field.span;
let (resolved, field_type) = self.elaborate_expression(field);
Expand All @@ -618,11 +638,21 @@ impl<'context> Elaborator<'context> {
});
}

if let Some(expected_index) = expected_index {
if let Some((index, visibility)) = expected_index_and_visibility {
let struct_type = struct_type.borrow();
let field_span = field_name.span();
let field_name = &field_name.0.contents;
self.check_struct_field_visibility(
&struct_type,
field_name,
*visibility,
field_span,
);

self.interner.add_struct_member_reference(
struct_type.borrow().id,
expected_index,
Location::new(field_name.span(), self.file),
struct_type.id,
index,
Location::new(field_span, self.file),
);
}

Expand Down Expand Up @@ -665,7 +695,7 @@ impl<'context> Elaborator<'context> {
fn elaborate_cast(&mut self, cast: CastExpression, span: Span) -> (HirExpression, Type) {
let (lhs, lhs_type) = self.elaborate_expression(cast.lhs);
let r#type = self.resolve_type(cast.r#type);
let result = self.check_cast(&lhs_type, &r#type, span);
let result = self.check_cast(&lhs, &lhs_type, &r#type, span);
let expr = HirExpression::Cast(HirCastExpression { lhs, r#type });
(expr, result)
}
Expand Down
37 changes: 24 additions & 13 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::{
rc::Rc,
};

use crate::ast::ItemVisibility;
use crate::{ast::ItemVisibility, StructField};
use crate::{
ast::{
BlockExpression, FunctionKind, GenericTypeArgs, Ident, NoirFunction, NoirStruct, Param,
Expand Down Expand Up @@ -751,7 +751,7 @@ impl<'context> Elaborator<'context> {
);

if is_entry_point {
self.mark_parameter_type_as_used(&typ);
self.mark_type_as_used(&typ);
}

let pattern = self.elaborate_pattern_and_store_ids(
Expand Down Expand Up @@ -832,33 +832,36 @@ impl<'context> Elaborator<'context> {
self.current_item = None;
}

fn mark_parameter_type_as_used(&mut self, typ: &Type) {
fn mark_type_as_used(&mut self, typ: &Type) {
match typ {
Type::Array(_n, typ) => self.mark_parameter_type_as_used(typ),
Type::Slice(typ) => self.mark_parameter_type_as_used(typ),
Type::Array(_n, typ) => self.mark_type_as_used(typ),
Type::Slice(typ) => self.mark_type_as_used(typ),
Type::Tuple(types) => {
for typ in types {
self.mark_parameter_type_as_used(typ);
self.mark_type_as_used(typ);
}
}
Type::Struct(struct_type, generics) => {
self.mark_struct_as_constructed(struct_type.clone());
for generic in generics {
self.mark_parameter_type_as_used(generic);
self.mark_type_as_used(generic);
}
for (_, typ) in struct_type.borrow().get_fields(generics) {
self.mark_type_as_used(&typ);
}
for (_, typ) in struct_type.borrow().get_fields(generics) {
self.mark_parameter_type_as_used(&typ);
}
}
Type::Alias(alias_type, generics) => {
self.mark_parameter_type_as_used(&alias_type.borrow().get_type(generics));
self.mark_type_as_used(&alias_type.borrow().get_type(generics));
}
Type::MutableReference(typ) => {
self.mark_parameter_type_as_used(typ);
self.mark_type_as_used(typ);
}
Type::InfixExpr(left, _op, right) => {
self.mark_parameter_type_as_used(left);
self.mark_parameter_type_as_used(right);
self.mark_type_as_used(left);
self.mark_type_as_used(right);
}
Type::FieldElement
| Type::Integer(..)
Expand Down Expand Up @@ -1277,6 +1280,13 @@ impl<'context> Elaborator<'context> {
self.local_module = typ.module_id;

let fields = self.resolve_struct_fields(&typ.struct_def, *type_id);

if typ.struct_def.is_abi() {
for field in &fields {
self.mark_type_as_used(&field.typ);
}
}

let fields_len = fields.len();
self.interner.update_struct(*type_id, |struct_def| {
struct_def.set_fields(fields);
Expand Down Expand Up @@ -1315,7 +1325,7 @@ impl<'context> Elaborator<'context> {
&mut self,
unresolved: &NoirStruct,
struct_id: StructId,
) -> Vec<(Ident, Type)> {
) -> Vec<StructField> {
self.recover_generics(|this| {
this.current_item = Some(DependencyId::Struct(struct_id));

Expand All @@ -1327,7 +1337,8 @@ impl<'context> Elaborator<'context> {
let fields = vecmap(&unresolved.fields, |field| {
let ident = &field.item.name;
let typ = &field.item.typ;
(ident.clone(), this.resolve_type(typ.clone()))
let visibility = field.item.visibility;
StructField { visibility, name: ident.clone(), typ: this.resolve_type(typ.clone()) }
});

this.resolving_ids.remove(&struct_id);
Expand Down
14 changes: 12 additions & 2 deletions noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ use rustc_hash::FxHashSet as HashSet;

use crate::{
ast::{
Expression, ExpressionKind, Ident, Path, Pattern, TypePath, UnresolvedType, ERROR_IDENT,
Expression, ExpressionKind, Ident, ItemVisibility, Path, Pattern, TypePath, UnresolvedType,
ERROR_IDENT,
},
hir::{
def_collector::dc_crate::CompilationError,
Expand Down Expand Up @@ -272,7 +273,9 @@ impl<'context> Elaborator<'context> {
let mut unseen_fields = struct_type.borrow().field_names();

for (field, pattern) in fields {
let field_type = expected_type.get_field_type(&field.0.contents).unwrap_or(Type::Error);
let (field_type, visibility) = expected_type
.get_field_type_and_visibility(&field.0.contents)
.unwrap_or((Type::Error, ItemVisibility::Public));
let resolved = self.elaborate_pattern_mut(
pattern,
field_type,
Expand All @@ -286,6 +289,13 @@ impl<'context> Elaborator<'context> {
if unseen_fields.contains(&field) {
unseen_fields.remove(&field);
seen_fields.insert(field.clone());

self.check_struct_field_visibility(
&struct_type.borrow(),
&field.0.contents,
visibility,
field.span(),
);
} else if seen_fields.contains(&field) {
// duplicate field
self.push_err(ResolverError::DuplicateField { field: field.clone() });
Expand Down
Loading

0 comments on commit caab224

Please sign in to comment.