Skip to content

Commit

Permalink
fix: Derive generic types (#5674)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Resolves an issue alluded to in
#5667 where we still could not
derive generic types

## Summary\*

This PR allows us to derive generic types by allowing a new "resolved
generic" type in the UnresolvedGeneric enum.

The issue before was that when we splice `Type`s into quoted snippets we
lower them as a special token kind which preserves the original `Type`
so that we don't have to worry about re-resolving it if it is a struct
type that is not imported into the caller's scope, etc. This also means
though that any type variables in the original type are preserved. In
the following snippet:

```rs
// Lets say this is `MyType<T>`
let my_type: Type = ...;
quote {
    impl<T> Foo for $my_type { ... }
}
```

The impl will introduce a fresh generic `T` into scope, but the `T` in
`MyType<T>` will still be the old `T` with a different TypeVariableId.
To fix this there were two options:
- Add another function to lower types in a way they'd need to be
reparsed, and would thus pick up new type variables but be susceptible
to "name not in scope" errors mentioned earlier.
- Somehow get the `T` in `impl<T>` to refer to the same `T` in
`MyType<T>`.

I chose the later approach since it is much simpler from a user's
perspective and leads to fewer footguns over forgetting to convert the
type into a `Quoted` before splicing it into other code. This way, users
just need to ensure the generics inserted as part of the impl generics
come from the type which is what is typically done anyway. This means
that the original snippet above will actually still fail since it still
introduces a new `T`. To fix it, we have to get the T from the type's
generics:

```rs
let generics = my_struct.generics().map(|g| quote { $g }).join(quote {,});
let my_type = my_struct.as_type();
quote {
    impl<$generics> Foo for $my_type { ... }
}
```

This is what `derive` does already since you don't know the names of the
generics ahead of time in practice.

## Additional Context



## 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.

---------

Co-authored-by: Michael J Klein <[email protected]>
  • Loading branch information
jfecher and michaeljklein authored Aug 2, 2024
1 parent 0afb680 commit 19e58a9
Show file tree
Hide file tree
Showing 15 changed files with 134 additions and 44 deletions.
1 change: 1 addition & 0 deletions aztec_macros/src/utils/parse_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,7 @@ fn empty_unresolved_generic(unresolved_generic: &mut UnresolvedGeneric) {
empty_ident(ident);
empty_unresolved_type(typ);
}
UnresolvedGeneric::Resolved(..) => (),
}
}

Expand Down
19 changes: 17 additions & 2 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::ast::{
};
use crate::hir::def_collector::errors::DefCollectorErrorKind;
use crate::macros_api::StructId;
use crate::node_interner::ExprId;
use crate::node_interner::{ExprId, QuotedTypeId};
use crate::token::{Attributes, Token, Tokens};
use crate::{Kind, Type};
use acvm::{acir::AcirField, FieldElement};
Expand Down Expand Up @@ -51,7 +51,16 @@ pub type UnresolvedGenerics = Vec<UnresolvedGeneric>;
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum UnresolvedGeneric {
Variable(Ident),
Numeric { ident: Ident, typ: UnresolvedType },
Numeric {
ident: Ident,
typ: UnresolvedType,
},

/// Already-resolved generics can be parsed as generics when a macro
/// splices existing types into a generic list. In this case we have
/// to validate the type refers to a named generic and treat that
/// as a ResolvedGeneric when this is resolved.
Resolved(QuotedTypeId, Span),
}

impl UnresolvedGeneric {
Expand All @@ -61,6 +70,7 @@ impl UnresolvedGeneric {
UnresolvedGeneric::Numeric { ident, typ } => {
ident.0.span().merge(typ.span.unwrap_or_default())
}
UnresolvedGeneric::Resolved(_, span) => *span,
}
}

Expand All @@ -71,6 +81,9 @@ impl UnresolvedGeneric {
let typ = self.resolve_numeric_kind_type(typ)?;
Ok(Kind::Numeric(Box::new(typ)))
}
UnresolvedGeneric::Resolved(..) => {
panic!("Don't know the kind of a resolved generic here")
}
}
}

Expand All @@ -94,6 +107,7 @@ impl UnresolvedGeneric {
pub(crate) fn ident(&self) -> &Ident {
match self {
UnresolvedGeneric::Variable(ident) | UnresolvedGeneric::Numeric { ident, .. } => ident,
UnresolvedGeneric::Resolved(..) => panic!("UnresolvedGeneric::Resolved no ident"),
}
}
}
Expand All @@ -103,6 +117,7 @@ impl Display for UnresolvedGeneric {
match self {
UnresolvedGeneric::Variable(ident) => write!(f, "{ident}"),
UnresolvedGeneric::Numeric { ident, typ } => write!(f, "let {ident}: {typ}"),
UnresolvedGeneric::Resolved(..) => write!(f, "(resolved)"),
}
}
}
Expand Down
77 changes: 57 additions & 20 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -523,35 +523,72 @@ impl<'context> Elaborator<'context> {
/// Each generic will have a fresh Shared<TypeBinding> associated with it.
pub fn add_generics(&mut self, generics: &UnresolvedGenerics) -> Generics {
vecmap(generics, |generic| {
// Map the generic to a fresh type variable
let id = self.interner.next_type_variable_id();
let typevar = TypeVariable::unbound(id);
let ident = generic.ident();
let span = ident.0.span();
let mut is_error = false;
let (type_var, name, kind) = match self.resolve_generic(generic) {
Ok(values) => values,
Err(error) => {
self.push_err(error);
is_error = true;
let id = self.interner.next_type_variable_id();
(TypeVariable::unbound(id), Rc::new("(error)".into()), Kind::Normal)
}
};

// Resolve the generic's kind
let kind = self.resolve_generic_kind(generic);
let span = generic.span();
let name_owned = name.as_ref().clone();
let resolved_generic = ResolvedGeneric { name, type_var, kind, span };

// Check for name collisions of this generic
let name = Rc::new(ident.0.contents.clone());

let resolved_generic =
ResolvedGeneric { name: name.clone(), type_var: typevar.clone(), kind, span };

if let Some(generic) = self.find_generic(&name) {
self.push_err(ResolverError::DuplicateDefinition {
name: ident.0.contents.clone(),
first_span: generic.span,
second_span: span,
});
} else {
self.generics.push(resolved_generic.clone());
// Checking `is_error` here prevents DuplicateDefinition errors when
// we have multiple generics from macros which fail to resolve and
// are all given the same default name "(error)".
if !is_error {
if let Some(generic) = self.find_generic(&name_owned) {
self.push_err(ResolverError::DuplicateDefinition {
name: name_owned,
first_span: generic.span,
second_span: span,
});
} else {
self.generics.push(resolved_generic.clone());
}
}

resolved_generic
})
}

fn resolve_generic(
&mut self,
generic: &UnresolvedGeneric,
) -> Result<(TypeVariable, Rc<String>, Kind), ResolverError> {
// Map the generic to a fresh type variable
match generic {
UnresolvedGeneric::Variable(_) | UnresolvedGeneric::Numeric { .. } => {
let id = self.interner.next_type_variable_id();
let typevar = TypeVariable::unbound(id);
let ident = generic.ident();

let kind = self.resolve_generic_kind(generic);
let name = Rc::new(ident.0.contents.clone());
Ok((typevar, name, kind))
}
// An already-resolved generic is only possible if it is the result of a
// previous macro call being inserted into a generics list.
UnresolvedGeneric::Resolved(id, span) => {
match self.interner.get_quoted_type(*id).follow_bindings() {
Type::NamedGeneric(type_variable, name, kind) => {
Ok((type_variable, name, kind))
}
other => Err(ResolverError::MacroResultInGenericsListNotAGeneric {
span: *span,
typ: other.clone(),
}),
}
}
}
}

/// Return the kind of an unresolved generic.
/// If a numeric generic has been specified, resolve the annotated type to make
/// sure only primitive numeric types are being used.
Expand Down
10 changes: 4 additions & 6 deletions compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@ fn struct_def_as_type(
Ok(Value::Type(Type::Struct(struct_def_rc, generics)))
}

/// fn generics(self) -> [Quoted]
/// fn generics(self) -> [Type]
fn struct_def_generics(
interner: &NodeInterner,
arguments: Vec<(Value, Location)>,
Expand All @@ -314,12 +314,10 @@ fn struct_def_generics(
let struct_def = interner.get_struct(struct_def);
let struct_def = struct_def.borrow();

let generics = struct_def.generics.iter().map(|generic| {
let name = Token::Ident(generic.type_var.borrow().to_string());
Value::Quoted(Rc::new(vec![name]))
});
let generics =
struct_def.generics.iter().map(|generic| Value::Type(generic.clone().as_named_generic()));

let typ = Type::Slice(Box::new(Type::Quoted(QuotedType::Quoted)));
let typ = Type::Slice(Box::new(Type::Quoted(QuotedType::Type)));
Ok(Value::Slice(generics.collect(), typ))
}

Expand Down
7 changes: 6 additions & 1 deletion compiler/noirc_frontend/src/hir/comptime/value.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,12 @@ impl<'value, 'interner> Display for ValuePrinter<'value, 'interner> {
Value::Quoted(tokens) => {
write!(f, "quote {{")?;
for token in tokens.iter() {
write!(f, " {token}")?;
match token {
Token::QuotedType(id) => {
write!(f, " {}", self.interner.get_quoted_type(*id))?;
}
other => write!(f, " {other}")?,
}
}
write!(f, " }}")
}
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,8 @@ pub enum ResolverError {
MacroIsNotComptime { span: Span },
#[error("Annotation name must refer to a comptime function")]
NonFunctionInAnnotation { span: Span },
#[error("Type `{typ}` was inserted into the generics list from a macro, but is not a generic")]
MacroResultInGenericsListNotAGeneric { span: Span, typ: Type },
}

impl ResolverError {
Expand Down Expand Up @@ -458,6 +460,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
*span,
)
},
ResolverError::MacroResultInGenericsListNotAGeneric { span, typ } => {
Diagnostic::simple_error(
format!("Type `{typ}` was inserted into a generics list from a macro, but it is not a generic"),
format!("Type `{typ}` is not a generic"),
*span,
)
}
}
}
}
6 changes: 6 additions & 0 deletions compiler/noirc_frontend/src/hir_def/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -193,6 +193,12 @@ pub struct ResolvedGeneric {
pub span: Span,
}

impl ResolvedGeneric {
pub fn as_named_generic(self) -> Type {
Type::NamedGeneric(self.type_var, self.name, self.kind)
}
}

impl std::hash::Hash for StructType {
fn hash<H: std::hash::Hasher>(&self, state: &mut H) {
self.id.hash(state);
Expand Down
12 changes: 10 additions & 2 deletions compiler/noirc_frontend/src/parser/parser/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,10 @@ use super::{
attributes::{attributes, validate_attributes},
block, fresh_statement, ident, keyword, maybe_comp_time, nothing, optional_visibility,
parameter_name_recovery, parameter_recovery, parenthesized, parse_type, pattern,
primitives::token_kind,
self_parameter, where_clause, NoirParser,
};
use crate::token::{Keyword, Token};
use crate::token::{Keyword, Token, TokenKind};
use crate::{ast::IntegerBitSize, parser::spanned};
use crate::{
ast::{
Expand Down Expand Up @@ -110,8 +111,15 @@ pub(super) fn generic_type() -> impl NoirParser<UnresolvedGeneric> {
ident().map(UnresolvedGeneric::Variable)
}

pub(super) fn resolved_generic() -> impl NoirParser<UnresolvedGeneric> {
token_kind(TokenKind::QuotedType).map_with_span(|token, span| match token {
Token::QuotedType(id) => UnresolvedGeneric::Resolved(id, span),
_ => unreachable!("token_kind(QuotedType) guarantees we parse a quoted type"),
})
}

pub(super) fn generic() -> impl NoirParser<UnresolvedGeneric> {
generic_type().or(numeric_generic())
generic_type().or(numeric_generic()).or(resolved_generic())
}

/// non_empty_ident_list: ident ',' non_empty_ident_list
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/parser/parser/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ fn quoted_type() -> impl NoirParser<UnresolvedType> {
/// This is the type of an already resolved type.
/// The only way this can appear in the token input is if an already resolved `Type` object
/// was spliced into a macro's token stream via the `$` operator.
fn resolved_type() -> impl NoirParser<UnresolvedType> {
pub(super) fn resolved_type() -> impl NoirParser<UnresolvedType> {
token_kind(TokenKind::QuotedType).map_with_span(|token, span| match token {
Token::QuotedType(id) => UnresolvedTypeData::Resolved(id).with_span(span),
_ => unreachable!("token_kind(QuotedType) guarantees we parse a quoted type"),
Expand Down
4 changes: 2 additions & 2 deletions noir_stdlib/src/cmp.nr
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,9 @@ trait Eq {
comptime fn derive_eq(s: StructDefinition) -> Quoted {
let typ = s.as_type();

let impl_generics = s.generics().join(quote {,});
let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});

let where_clause = s.generics().map(|name| quote { $name: Default }).join(quote {,});
let where_clause = s.generics().map(|name| quote { $name: Eq }).join(quote {,});

// `(self.a == other.a) & (self.b == other.b) & ...`
let equalities = s.fields().map(
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/default.nr
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ trait Default {
comptime fn derive_default(s: StructDefinition) -> Quoted {
let typ = s.as_type();

let impl_generics = s.generics().join(quote {,});
let impl_generics = s.generics().map(|g| quote { $g }).join(quote {,});

let where_clause = s.generics().map(|name| quote { $name: Default }).join(quote {,});

Expand Down
5 changes: 2 additions & 3 deletions noir_stdlib/src/meta/struct_def.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,9 @@ impl StructDefinition {
#[builtin(struct_def_as_type)]
fn as_type(self) -> Type {}

/// Return each generic on this struct. The names of these generics are unchanged
/// so users may need to keep name collisions in mind if this is used directly in a macro.
/// Return each generic on this struct.
#[builtin(struct_def_generics)]
fn generics(self) -> [Quoted] {}
fn generics(self) -> [Type] {}

/// Returns (name, type) pairs of each field in this struct. Each type is as-is
/// with any generic arguments unchanged.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
comptime fn derive_default(typ: StructDefinition) -> Quoted {
let generics: [Quoted] = typ.generics();
let generics = typ.generics();
assert_eq(
generics.len(), 0, "derive_default: Deriving Default on generic types is currently unimplemented"
);
Expand Down
19 changes: 14 additions & 5 deletions test_programs/execution_success/derive/src/main.nr
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ struct MyStruct { my_field: u32 }

comptime fn derive_do_nothing(s: StructDefinition) -> Quoted {
let typ = s.as_type();
let generics = s.generics().join(quote {,});
let generics = s.generics().map(|g| quote { $g }).join(quote {,});
quote {
impl<$generics> DoNothing for $typ {
fn do_nothing(_self: Self) {
Expand All @@ -21,15 +21,24 @@ comptime fn derive_do_nothing(s: StructDefinition) -> Quoted {

// Test stdlib derive fns & multiple traits
#[derive(Eq, Default)]
struct MyOtherStruct {
field1: u32,
field2: u64,
struct MyOtherStruct<A, B> {
field1: A,
field2: B,
field3: MyOtherOtherStruct<B>,
}

#[derive(Eq, Default)]
struct MyOtherOtherStruct<T> {
x: T,
}

fn main() {
let s = MyStruct { my_field: 1 };
s.do_nothing();

let o = MyOtherStruct::default();
let o: MyOtherStruct<Field, u32> = MyOtherStruct::default();
assert_eq(o, o);

let o: MyOtherStruct<u8, [str<2>]> = MyOtherStruct::default();
assert_eq(o, o);
}
3 changes: 3 additions & 0 deletions tooling/nargo_fmt/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -188,6 +188,9 @@ impl HasItem for UnresolvedGeneric {
result.push_str(&typ);
result
}
UnresolvedGeneric::Resolved(..) => {
unreachable!("Found macro result UnresolvedGeneric::Resolved in formatter")
}
}
}
}
Expand Down

0 comments on commit 19e58a9

Please sign in to comment.