Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: Derive generic types #5674

Merged
merged 8 commits into from
Aug 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@
empty_ident(ident);
empty_unresolved_type(typ);
}
UnresolvedGeneric::Resolved(..) => (),
}
}

Expand All @@ -374,9 +375,9 @@
}

fn empty_unresolved_trait_constraints(
unresolved_trait_constriants: &mut [UnresolvedTraitConstraint],

Check warning on line 378 in aztec_macros/src/utils/parse_utils.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (constriants)
) {
for trait_constraint in unresolved_trait_constriants.iter_mut() {

Check warning on line 380 in aztec_macros/src/utils/parse_utils.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (constriants)
empty_unresolved_trait_constraint(trait_constraint);
}
}
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
82 changes: 62 additions & 20 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,35 +502,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 Expand Up @@ -1300,6 +1337,11 @@ impl<'context> Elaborator<'context> {
self.add_generics(&trait_impl.generics);
trait_impl.resolved_generics = self.generics.clone();

for (_, _, method) in trait_impl.methods.functions.iter_mut() {
// Attach any trait constraints on the impl to the function
method.def.where_clause.append(&mut trait_impl.where_clause.clone());
}

// Fetch trait constraints here
let trait_generics = trait_impl
.trait_id
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 @@ -249,7 +249,7 @@
Ok(Value::Type(Type::Struct(struct_def_rc, generics)))
}

/// fn generics(self) -> [Quoted]
/// fn generics(self) -> [Type]
fn struct_def_generics(
interner: &NodeInterner,
mut arguments: Vec<(Value, Location)>,
Expand All @@ -269,12 +269,10 @@
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 Expand Up @@ -439,7 +437,7 @@
})?;

let typ =
interpreter.elaborate_item(interpreter.current_function, |elab| elab.resolve_type(typ));

Check warning on line 440 in compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elab)

Check warning on line 440 in compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (elab)

Ok(Value::Type(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
2 changes: 0 additions & 2 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,8 +173,6 @@
let module = ModuleId { krate, local_id: self.module_id };

for (_, func_id, noir_function) in &mut unresolved_functions.functions {
// Attach any trait constraints on the impl to the function
noir_function.def.where_clause.append(&mut trait_impl.where_clause.clone());
let location = Location::new(noir_function.def.span, self.file_id);
context.def_interner.push_function(*func_id, &noir_function.def, module, location);
}
Expand Down Expand Up @@ -539,7 +537,7 @@
}
}
TraitItem::Type { name } => {
// TODO(nickysn or alexvitkov): implement context.def_interner.push_empty_type_alias and get an id, instead of using TypeAliasId::dummy_id()

Check warning on line 540 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (nickysn)

Check warning on line 540 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (alexvitkov)
if let Err((first_def, second_def)) = self.def_collector.def_map.modules
[trait_id.0.local_id.0]
.declare_type_alias(name.clone(), TypeAliasId::dummy_id())
Expand Down Expand Up @@ -725,7 +723,7 @@
// if it's an inline module, or the first char of a the file if it's an external module.
// - `location` will always point to the token "foo" in `mod foo` regardless of whether
// it's inline or external.
// Eventually the location put in `ModuleData` is used for codelenses about `contract`s,

Check warning on line 726 in compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (codelenses)
// so we keep using `location` so that it continues to work as usual.
let location = Location::new(mod_name.span(), mod_location.file);
let new_module = ModuleData::new(parent, location, is_contract);
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 @@ -191,6 +191,12 @@
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 Expand Up @@ -1801,7 +1807,7 @@
}

let recur_on_binding = |id, replacement: &Type| {
// Prevent recuring forever if there's a `T := T` binding

Check warning on line 1810 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (recuring)
if replacement.type_variable_id() == Some(id) {
replacement.clone()
} else {
Expand Down Expand Up @@ -1872,7 +1878,7 @@
Type::Tuple(fields)
}
Type::Forall(typevars, typ) => {
// Trying to substitute_helper a variable de, substitute_bound_typevarsfined within a nested Forall

Check warning on line 1881 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (typevarsfined)
// is usually impossible and indicative of an error in the type checker somewhere.
for var in typevars {
assert!(!type_bindings.contains_key(&var.id()));
Expand Down Expand Up @@ -2018,7 +2024,7 @@

/// Replace any `Type::NamedGeneric` in this type with a `Type::TypeVariable`
/// using to the same inner `TypeVariable`. This is used during monomorphization
/// to bind to named generics since they are unbindable during type checking.

Check warning on line 2027 in compiler/noirc_frontend/src/hir_def/types.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (unbindable)
pub fn replace_named_generics_with_type_variables(&mut self) {
match self {
Type::FieldElement
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
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "regression_5671"
type = "bin"
authors = [""]
compiler_version = ">=0.32.0"

[dependencies]
20 changes: 20 additions & 0 deletions test_programs/compile_success_empty/regression_5671/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#[foo]
struct MyOtherStruct<A, B> {
field1: A,
field2: B,
}

comptime fn foo(_s: StructDefinition) -> Quoted {
quote {
impl<A, B> Eq for MyOtherStruct<A, B> where A: Eq, B: Eq {
fn eq(self, other: Self) -> bool {
(self.field1 == other.field1) & (self.field2 == other.field2)
}
}
}
}

fn main() {
let x = MyOtherStruct { field1: 1, field2: 2 };
assert_eq(x, x);
}
Loading
Loading