Skip to content

Commit

Permalink
feat: format trait impl functions (#6016)
Browse files Browse the repository at this point in the history
# Description

## Problem

Trait impl functions weren't formatted. I think it's because trait impl
items don't have an associated span.

## Summary

This PR changes `TraitImplItem` to be `TraitImplItemKind`, and
introduces `TraitImplItem` as the usual struct that holds `kind` and
`span`.

With that, we can at least have `nargo fmt` format trait impl functions
(I left the other ones unchanged, maybe they could be done later, but
functions are the most common items).

## Additional Context

None.

## Documentation

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
asterite authored Sep 12, 2024
1 parent 4d8fe28 commit da32bd8
Show file tree
Hide file tree
Showing 48 changed files with 1,222 additions and 617 deletions.
36 changes: 23 additions & 13 deletions aztec_macros/src/transforms/events.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use noirc_frontend::ast::{Documented, ItemVisibility, NoirFunction, NoirTraitImpl, TraitImplItem};
use noirc_errors::Span;
use noirc_frontend::ast::{
Documented, ItemVisibility, NoirFunction, NoirTraitImpl, TraitImplItem, TraitImplItemKind,
};
use noirc_frontend::macros_api::{NodeInterner, StructId};
use noirc_frontend::token::SecondaryAttribute;
use noirc_frontend::{
Expand Down Expand Up @@ -67,30 +70,37 @@ pub fn generate_event_impls(
event_byte_len,
empty_spans,
)?;
event_interface_trait_impl.items.push(Documented::not_documented(
TraitImplItem::Function(generate_fn_get_event_type_id(
event_interface_trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(generate_fn_get_event_type_id(
event_type.as_str(),
event_len,
empty_spans,
)?),
));
event_interface_trait_impl.items.push(Documented::not_documented(
TraitImplItem::Function(generate_fn_private_to_be_bytes(
span: Span::default(),
}));
event_interface_trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(generate_fn_private_to_be_bytes(
event_type.as_str(),
event_byte_len,
empty_spans,
)?),
));
event_interface_trait_impl.items.push(Documented::not_documented(
TraitImplItem::Function(generate_fn_to_be_bytes(
span: Span::default(),
}));
event_interface_trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(generate_fn_to_be_bytes(
event_type.as_str(),
event_byte_len,
empty_spans,
)?),
));
event_interface_trait_impl.items.push(Documented::not_documented(
TraitImplItem::Function(generate_fn_emit(event_type.as_str(), empty_spans)?),
));
span: Span::default(),
}));
event_interface_trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(generate_fn_emit(
event_type.as_str(),
empty_spans,
)?),
span: Span::default(),
}));
submodule.contents.trait_impls.push(event_interface_trait_impl);

let serialize_trait_impl = generate_trait_impl_serialize(
Expand Down
51 changes: 29 additions & 22 deletions aztec_macros/src/transforms/note_interface.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
use noirc_errors::Span;
use noirc_frontend::ast::{
Documented, ItemVisibility, LetStatement, NoirFunction, NoirStruct, PathKind, StructField,
TraitImplItem, TypeImpl, UnresolvedTypeData, UnresolvedTypeExpression,
TraitImplItem, TraitImplItemKind, TypeImpl, UnresolvedTypeData, UnresolvedTypeExpression,
};
use noirc_frontend::{
graph::CrateId,
Expand Down Expand Up @@ -153,9 +153,10 @@ pub fn generate_note_interface_impl(
note_interface_impl_span,
empty_spans,
)?;
trait_impl.items.push(Documented::not_documented(TraitImplItem::Function(
note_serialize_content_fn,
)));
trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(note_serialize_content_fn),
span: note_interface_impl_span,
}));

let note_deserialize_content_fn = generate_note_deserialize_content(
&note_type,
Expand All @@ -165,9 +166,10 @@ pub fn generate_note_interface_impl(
note_interface_impl_span,
empty_spans,
)?;
trait_impl.items.push(Documented::not_documented(TraitImplItem::Function(
note_deserialize_content_fn,
)));
trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(note_deserialize_content_fn),
span: note_interface_impl_span,
}));

let note_properties_struct = generate_note_properties_struct(
&note_type,
Expand Down Expand Up @@ -196,9 +198,10 @@ pub fn generate_note_interface_impl(
note_interface_impl_span,
empty_spans,
)?;
trait_impl
.items
.push(Documented::not_documented(TraitImplItem::Function(get_header_fn)));
trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(get_header_fn),
span: note_interface_impl_span,
}));
}
if !check_trait_method_implemented(trait_impl, "set_header") {
let set_header_fn = generate_note_set_header(
Expand All @@ -207,18 +210,20 @@ pub fn generate_note_interface_impl(
note_interface_impl_span,
empty_spans,
)?;
trait_impl
.items
.push(Documented::not_documented(TraitImplItem::Function(set_header_fn)));
trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(set_header_fn),
span: note_interface_impl_span,
}));
}

if !check_trait_method_implemented(trait_impl, "get_note_type_id") {
let note_type_id = compute_note_type_id(&note_type);
let get_note_type_id_fn =
generate_get_note_type_id(note_type_id, note_interface_impl_span, empty_spans)?;
trait_impl
.items
.push(Documented::not_documented(TraitImplItem::Function(get_note_type_id_fn)));
trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(get_note_type_id_fn),
span: note_interface_impl_span,
}));
}

if !check_trait_method_implemented(trait_impl, "compute_note_hiding_point") {
Expand All @@ -227,9 +232,10 @@ pub fn generate_note_interface_impl(
note_interface_impl_span,
empty_spans,
)?;
trait_impl.items.push(Documented::not_documented(TraitImplItem::Function(
compute_note_hiding_point_fn,
)));
trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(compute_note_hiding_point_fn),
span: note_interface_impl_span,
}));
}

if !check_trait_method_implemented(trait_impl, "to_be_bytes") {
Expand All @@ -240,9 +246,10 @@ pub fn generate_note_interface_impl(
note_interface_impl_span,
empty_spans,
)?;
trait_impl
.items
.push(Documented::not_documented(TraitImplItem::Function(to_be_bytes_fn)));
trait_impl.items.push(Documented::not_documented(TraitImplItem {
kind: TraitImplItemKind::Function(to_be_bytes_fn),
span: note_interface_impl_span,
}));
}
}

Expand Down
6 changes: 3 additions & 3 deletions aztec_macros/src/utils/ast_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use noirc_frontend::ast::{
BinaryOpKind, CallExpression, CastExpression, Expression, ExpressionKind, FunctionReturnType,
Ident, IndexExpression, InfixExpression, Lambda, MemberAccessExpression, MethodCallExpression,
NoirTraitImpl, Path, PathSegment, Pattern, PrefixExpression, Statement, StatementKind,
TraitImplItem, UnaryOp, UnresolvedType, UnresolvedTypeData,
TraitImplItemKind, UnaryOp, UnresolvedType, UnresolvedTypeData,
};
use noirc_frontend::token::SecondaryAttribute;

Expand Down Expand Up @@ -179,8 +179,8 @@ pub fn index_array(array: Ident, index: &str) -> Expression {
}

pub fn check_trait_method_implemented(trait_impl: &NoirTraitImpl, method_name: &str) -> bool {
trait_impl.items.iter().any(|item| match &item.item {
TraitImplItem::Function(func) => func.def.name.0.contents == method_name,
trait_impl.items.iter().any(|item| match &item.item.kind {
TraitImplItemKind::Function(func) => func.def.name.0.contents == method_name,
_ => false,
})
}
Expand Down
18 changes: 12 additions & 6 deletions aztec_macros/src/utils/parse_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ use noirc_frontend::{
InfixExpression, LValue, Lambda, LetStatement, Literal, MemberAccessExpression,
MethodCallExpression, ModuleDeclaration, NoirFunction, NoirStruct, NoirTrait,
NoirTraitImpl, NoirTypeAlias, Path, PathSegment, Pattern, PrefixExpression, Statement,
StatementKind, TraitImplItem, TraitItem, TypeImpl, UnresolvedGeneric, UnresolvedGenerics,
UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, UnresolvedTypeExpression,
UseTree, UseTreeKind,
StatementKind, TraitImplItem, TraitImplItemKind, TraitItem, TypeImpl, UnresolvedGeneric,
UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
UnresolvedTypeExpression, UseTree, UseTreeKind,
},
parser::{Item, ItemKind, ParsedSubModule, ParserError},
ParsedModule,
Expand Down Expand Up @@ -137,14 +137,20 @@ fn empty_trait_item(trait_item: &mut TraitItem) {
}

fn empty_trait_impl_item(trait_impl_item: &mut TraitImplItem) {
trait_impl_item.span = Default::default();

empty_trait_impl_item_kind(&mut trait_impl_item.kind);
}

fn empty_trait_impl_item_kind(trait_impl_item: &mut TraitImplItemKind) {
match trait_impl_item {
TraitImplItem::Function(noir_function) => empty_noir_function(noir_function),
TraitImplItem::Constant(name, typ, default_value) => {
TraitImplItemKind::Function(noir_function) => empty_noir_function(noir_function),
TraitImplItemKind::Constant(name, typ, default_value) => {
empty_ident(name);
empty_unresolved_type(typ);
empty_expression(default_value);
}
TraitImplItem::Type { name, alias } => {
TraitImplItemKind::Type { name, alias } => {
empty_ident(name);
empty_unresolved_type(alias);
}
Expand Down
20 changes: 16 additions & 4 deletions compiler/noirc_frontend/src/ast/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,13 @@ pub struct TraitBound {
}

#[derive(Clone, Debug)]
pub enum TraitImplItem {
pub struct TraitImplItem {
pub kind: TraitImplItemKind,
pub span: Span,
}

#[derive(Clone, Debug)]
pub enum TraitImplItemKind {
Function(NoirFunction),
Constant(Ident, UnresolvedType, Expression),
Type { name: Ident, alias: UnresolvedType },
Expand Down Expand Up @@ -202,11 +208,17 @@ impl Display for NoirTraitImpl {
}

impl Display for TraitImplItem {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
self.kind.fmt(f)
}
}

impl Display for TraitImplItemKind {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
TraitImplItem::Function(function) => function.fmt(f),
TraitImplItem::Type { name, alias } => write!(f, "type {name} = {alias};"),
TraitImplItem::Constant(name, typ, value) => {
TraitImplItemKind::Function(function) => function.fmt(f),
TraitImplItemKind::Type { name, alias } => write!(f, "type {name} = {alias};"),
TraitImplItemKind::Constant(name, typ, value) => {
write!(f, "let {name}: {typ} = {value};")
}
}
Expand Down
42 changes: 30 additions & 12 deletions compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ use crate::{

use super::{
FunctionReturnType, GenericTypeArgs, IntegerBitSize, ItemVisibility, Pattern, Signedness,
UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
UnresolvedTypeExpression,
TraitImplItemKind, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType,
UnresolvedTypeData, UnresolvedTypeExpression,
};

#[derive(Debug, Copy, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -69,6 +69,10 @@ pub trait Visitor {
true
}

fn visit_trait_impl_item_kind(&mut self, _: &TraitImplItemKind, _span: Span) -> bool {
true
}

fn visit_trait_impl_item_function(&mut self, _: &NoirFunction, _span: Span) -> bool {
true
}
Expand All @@ -78,11 +82,17 @@ pub trait Visitor {
_name: &Ident,
_typ: &UnresolvedType,
_expression: &Expression,
_span: Span,
) -> bool {
true
}

fn visit_trait_impl_item_type(&mut self, _name: &Ident, _alias: &UnresolvedType) -> bool {
fn visit_trait_impl_item_type(
&mut self,
_name: &Ident,
_alias: &UnresolvedType,
_span: Span,
) -> bool {
true
}

Expand Down Expand Up @@ -569,24 +579,32 @@ impl TraitImplItem {
}

pub fn accept_children(&self, visitor: &mut impl Visitor) {
match self {
TraitImplItem::Function(noir_function) => {
let span = Span::from(
noir_function.name_ident().span().start()..noir_function.span().end(),
);
self.kind.accept(self.span, visitor);
}
}

impl TraitImplItemKind {
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
if visitor.visit_trait_impl_item_kind(self, span) {
self.accept_children(span, visitor);
}
}

pub fn accept_children(&self, span: Span, visitor: &mut impl Visitor) {
match self {
TraitImplItemKind::Function(noir_function) => {
if visitor.visit_trait_impl_item_function(noir_function, span) {
noir_function.accept(span, visitor);
}
}
TraitImplItem::Constant(name, unresolved_type, expression) => {
if visitor.visit_trait_impl_item_constant(name, unresolved_type, expression) {
TraitImplItemKind::Constant(name, unresolved_type, expression) => {
if visitor.visit_trait_impl_item_constant(name, unresolved_type, expression, span) {
unresolved_type.accept(visitor);
expression.accept(visitor);
}
}
TraitImplItem::Type { name, alias } => {
if visitor.visit_trait_impl_item_type(name, alias) {
TraitImplItemKind::Type { name, alias } => {
if visitor.visit_trait_impl_item_type(name, alias, span) {
alias.accept(visitor);
}
}
Expand Down
10 changes: 5 additions & 5 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ use rustc_hash::FxHashMap as HashMap;

use crate::ast::{
Documented, FunctionDefinition, Ident, ItemVisibility, LetStatement, ModuleDeclaration,
NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Pattern, TraitImplItem,
NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Pattern, TraitImplItemKind,
TraitItem, TypeImpl,
};
use crate::hir::resolution::errors::ResolverError;
Expand Down Expand Up @@ -1124,18 +1124,18 @@ pub(crate) fn collect_trait_impl_items(
let module = ModuleId { krate, local_id };

for item in std::mem::take(&mut trait_impl.items) {
match item.item {
TraitImplItem::Function(impl_method) => {
match item.item.kind {
TraitImplItemKind::Function(impl_method) => {
let func_id = interner.push_empty_fn();
let location = Location::new(impl_method.span(), file_id);
interner.push_function(func_id, &impl_method.def, module, location);
interner.set_doc_comments(ReferenceId::Function(func_id), item.doc_comments);
unresolved_functions.push_fn(local_id, func_id, impl_method);
}
TraitImplItem::Constant(name, typ, expr) => {
TraitImplItemKind::Constant(name, typ, expr) => {
associated_constants.push((name, typ, expr));
}
TraitImplItem::Type { name, alias } => {
TraitImplItemKind::Type { name, alias } => {
associated_types.push((name, alias));
}
}
Expand Down
Loading

0 comments on commit da32bd8

Please sign in to comment.