Skip to content

Commit

Permalink
feat(frontend): Explicit numeric generics and type kinds (#5155)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves #4633, but only in the elaborator as we will be moving to this
new infrastructure for the frontend soon and resolving edge cases across
both the old resolver and the elaborator was becoming a time sink.

## Summary\*

We now have an `UnresolvedGeneric` type rather than simply representing
generics using idents.
```rust
pub enum UnresolvedGeneric {
    Variable(Ident),
    Numeric { ident: Ident, typ: UnresolvedType },
}
```
We also have a new `ResolvedGeneric` struct when storing generics during
resolution and elaboration. We were previously storing a tuple of three
elements, but now that I need to distinguish when we have a numeric
generic it was simpler to make this a struct.

Some example usage:
```rust
// Used as a field of a struct
struct Foo<let N: u64> {
    inner: [u64; N],
}
fn baz<let N: u64>() -> Foo<N> {
    Foo { inner: [1; N] }
}
// Used in an impl
impl<let N: u64> Foo<N> {
    fn bar(self) -> u64 {
        N * self.inner[0]
    }
}
```
More examples can be found in `numeric_generics` and documentation will
come in a follow-up.

We make sure to error out when an explicit numeric generic is used as a
type.
For example, this code:
```rust
struct Trash<let N: u64> {
    a: Field,
    b: N,
}
fn id_two<let I: Field>(x: I) -> I {
    x
}
``` 
<img width="749" alt="Screenshot 2024-06-04 at 5 42 53 PM"
src="https://github.com/noir-lang/noir/assets/43554004/a515a508-7ea4-4a23-bfd3-6f7bbae28e55">


## Additional Context

We do not ban the old strategy for implicit numeric generics just yet as
a lot of code uses it. Currently this warning is issued:
<img width="965" alt="Screenshot 2024-06-04 at 11 34 19 AM"
src="https://github.com/noir-lang/noir/assets/43554004/60c79337-98b2-4e70-a11d-947f6611fc41">


## Documentation\*

Check one:
- [ ] No documentation needed.
- [ ] Documentation included in this PR.
- [X] **[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.

---------

Co-authored-by: jfecher <jake@aztecprotocol.com>
  • Loading branch information
vezenovm and jfecher authored Jun 24, 2024
1 parent cf938bc commit d4e03d0
Show file tree
Hide file tree
Showing 43 changed files with 1,501 additions and 264 deletions.
4 changes: 2 additions & 2 deletions aztec_macros/src/transforms/storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ pub fn inject_context_in_storage(module: &mut SortedModule) -> Result<(), AztecM
r#struct.attributes.iter().any(|attr| is_custom_attribute(attr, "aztec(storage)"))
})
.unwrap();
storage_struct.generics.push(ident("Context"));
storage_struct.generics.push(ident("Context").into());
storage_struct
.fields
.iter_mut()
Expand Down Expand Up @@ -243,7 +243,7 @@ pub fn generate_storage_implementation(
span: Some(Span::default()),
},
type_span: Span::default(),
generics: vec![generic_context_ident],
generics: vec![generic_context_ident.into()],

methods: vec![(init, Span::default())],

Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_driver/tests/stdlib_warnings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use std::path::Path;
use noirc_driver::{file_manager_with_stdlib, prepare_crate, ErrorsAndWarnings};
use noirc_frontend::hir::{def_map::parse_file, Context};

#[ignore = "Temporarily ignoring the test until the stdlib is updated to use explicit numeric generics"]
#[test]
fn stdlib_does_not_produce_constant_warnings() -> Result<(), ErrorsAndWarnings> {
// We use a minimal source file so that if stdlib produces warnings then we can expect these warnings to _always_
Expand Down
69 changes: 68 additions & 1 deletion compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,11 @@ use crate::ast::{
Ident, ItemVisibility, Path, Pattern, Recoverable, Statement, StatementKind,
UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
};
use crate::hir::def_collector::errors::DefCollectorErrorKind;
use crate::macros_api::StructId;
use crate::node_interner::ExprId;
use crate::token::{Attributes, Token, Tokens};
use crate::{Kind, Type};
use acvm::{acir::AcirField, FieldElement};
use iter_extended::vecmap;
use noirc_errors::{Span, Spanned};
Expand Down Expand Up @@ -46,7 +48,72 @@ pub enum ExpressionKind {

/// A Vec of unresolved names for type variables.
/// For `fn foo<A, B>(...)` this corresponds to vec!["A", "B"].
pub type UnresolvedGenerics = Vec<Ident>;
pub type UnresolvedGenerics = Vec<UnresolvedGeneric>;

#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum UnresolvedGeneric {
Variable(Ident),
Numeric { ident: Ident, typ: UnresolvedType },
}

impl UnresolvedGeneric {
pub fn span(&self) -> Span {
match self {
UnresolvedGeneric::Variable(ident) => ident.0.span(),
UnresolvedGeneric::Numeric { ident, typ } => {
ident.0.span().merge(typ.span.unwrap_or_default())
}
}
}

pub fn kind(&self) -> Result<Kind, DefCollectorErrorKind> {
match self {
UnresolvedGeneric::Variable(_) => Ok(Kind::Normal),
UnresolvedGeneric::Numeric { typ, .. } => {
let typ = self.resolve_numeric_kind_type(typ)?;
Ok(Kind::Numeric(Box::new(typ)))
}
}
}

fn resolve_numeric_kind_type(
&self,
typ: &UnresolvedType,
) -> Result<Type, DefCollectorErrorKind> {
use crate::ast::UnresolvedTypeData::{FieldElement, Integer};

match typ.typ {
FieldElement => Ok(Type::FieldElement),
Integer(sign, bits) => Ok(Type::Integer(sign, bits)),
// Only fields and integers are supported for numeric kinds
_ => Err(DefCollectorErrorKind::UnsupportedNumericGenericType {
ident: self.ident().clone(),
typ: typ.typ.clone(),
}),
}
}

pub(crate) fn ident(&self) -> &Ident {
match self {
UnresolvedGeneric::Variable(ident) | UnresolvedGeneric::Numeric { ident, .. } => ident,
}
}
}

impl Display for UnresolvedGeneric {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {
UnresolvedGeneric::Variable(ident) => write!(f, "{ident}"),
UnresolvedGeneric::Numeric { ident, typ } => write!(f, "let {ident}: {typ}"),
}
}
}

impl From<Ident> for UnresolvedGeneric {
fn from(value: Ident) -> Self {
UnresolvedGeneric::Variable(value)
}
}

impl ExpressionKind {
pub fn into_path(self) -> Option<Path> {
Expand Down
6 changes: 5 additions & 1 deletion compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -146,7 +146,7 @@ pub struct UnaryRhsMethodCall {
}

/// The precursor to TypeExpression, this is the type that the parser allows
/// to be used in the length position of an array type. Only constants, variables,
/// to be used in the length position of an array type. Only constant integers, variables,
/// and numeric binary operators are allowed here.
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum UnresolvedTypeExpression {
Expand Down Expand Up @@ -259,6 +259,10 @@ impl UnresolvedType {
pub fn unspecified() -> UnresolvedType {
UnresolvedType { typ: UnresolvedTypeData::Unspecified, span: None }
}

pub(crate) fn is_type_expression(&self) -> bool {
matches!(&self.typ, UnresolvedTypeData::Expression(_))
}
}

impl UnresolvedTypeData {
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/ast/structure.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ impl NoirStruct {
pub fn new(
name: Ident,
attributes: Vec<SecondaryAttribute>,
generics: Vec<Ident>,
generics: UnresolvedGenerics,
fields: Vec<(Ident, UnresolvedType)>,
span: Span,
) -> NoirStruct {
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/ast/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::node_interner::TraitId;
#[derive(Clone, Debug)]
pub struct NoirTrait {
pub name: Ident,
pub generics: Vec<Ident>,
pub generics: UnresolvedGenerics,
pub where_clause: Vec<UnresolvedTraitConstraint>,
pub span: Span,
pub items: Vec<TraitItem>,
Expand All @@ -26,7 +26,7 @@ pub struct NoirTrait {
pub enum TraitItem {
Function {
name: Ident,
generics: Vec<Ident>,
generics: UnresolvedGenerics,
parameters: Vec<(Ident, UnresolvedType)>,
return_type: FunctionReturnType,
where_clause: Vec<UnresolvedTraitConstraint>,
Expand Down
17 changes: 15 additions & 2 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use crate::{
},
node_interner::{DefinitionKind, ExprId, FuncId},
token::Tokens,
QuotedType, Shared, StructType, Type,
Kind, QuotedType, Shared, StructType, Type,
};

use super::Elaborator;
Expand All @@ -52,7 +52,20 @@ impl<'context> Elaborator<'context> {
ExpressionKind::If(if_) => self.elaborate_if(*if_),
ExpressionKind::Variable(variable, generics) => {
let generics = generics.map(|option_inner| {
option_inner.into_iter().map(|generic| self.resolve_type(generic)).collect()
option_inner
.into_iter()
.map(|generic| {
// All type expressions should resolve to a `Type::Constant`
if generic.is_type_expression() {
self.resolve_type_inner(
generic,
&Kind::Numeric(Box::new(Type::default_int_type())),
)
} else {
self.resolve_type(generic)
}
})
.collect()
});
return self.elaborate_variable(variable, generics);
}
Expand Down
Loading

0 comments on commit d4e03d0

Please sign in to comment.