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

feat: type alias for numeric generics #7583

Open
wants to merge 37 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
37 commits
Select commit Hold shift + click to select a range
a1b9b69
type alias for numeric generics
guipublic Mar 5, 2025
0c2f4ff
Merge branch 'master' into gd/issue_7272
guipublic Mar 5, 2025
82d639e
fix unit tests
guipublic Mar 5, 2025
ff24516
Merge branch 'master' into gd/issue_7272
guipublic Mar 5, 2025
4d4dabe
use the aliased expression in loop bounds
guipublic Mar 11, 2025
531467a
Merge branch 'master' into gd/issue_7272
guipublic Mar 11, 2025
8f50d5e
code review: get location before parsing semi-column
guipublic Mar 11, 2025
249f71b
code review
guipublic Mar 11, 2025
ca3298f
Code Review: use option instead of an enum
guipublic Mar 11, 2025
7884d9f
Code review
guipublic Mar 11, 2025
e19f7e3
Merge branch 'master' into gd/issue_7272
guipublic Mar 11, 2025
6739cc9
rename typealias
guipublic Mar 12, 2025
7b79b8a
Merge branch 'master' into gd/issue_7272
guipublic Mar 12, 2025
c5e1dcc
Disable numeric type alias composition
guipublic Mar 14, 2025
0074169
Merge branch 'master' into gd/issue_7272
guipublic Mar 14, 2025
44abde3
remove 'custom errors span' in test as it conflict with negative lite…
guipublic Mar 14, 2025
e04ad34
Merge branch 'master' into gd/issue_7272
guipublic Mar 14, 2025
97b7063
format
guipublic Mar 14, 2025
faf9f0c
remove custom error spans from test, completely
guipublic Mar 14, 2025
efcecff
add integration test
guipublic Mar 17, 2025
e1552b4
Merge branch 'master' into gd/issue_7272
guipublic Mar 17, 2025
a4ae121
format numeric type alias
guipublic Mar 18, 2025
e5de933
Merge branch 'master' into gd/issue_7272
guipublic Mar 18, 2025
7522b3a
Merge branch 'master' into gd/issue_7272
guipublic Mar 18, 2025
fd35270
chore: avoid unnecessary mutable sets of `num_expr`
TomAFrench Mar 18, 2025
fc7b048
chore: fix me breaking things
TomAFrench Mar 18, 2025
58b39a9
chore: move tests to `aliases` module and add identity test
TomAFrench Mar 18, 2025
1744fea
code review
guipublic Mar 18, 2025
3eb7c2f
Merge branch 'master' into gd/issue_7272
guipublic Mar 19, 2025
cb8d263
Code review, allow variable as numeric expression
guipublic Mar 19, 2025
44cd174
Merge branch 'master' into gd/issue_7272
TomAFrench Mar 19, 2025
d67a9cb
Strengthen error on type alias referencing other type alias
guipublic Mar 19, 2025
fcc9c04
Merge branch 'master' into gd/issue_7272
guipublic Mar 19, 2025
49d865f
fix composing numeric type aliases
guipublic Mar 20, 2025
fb837af
Code review: more specific errors and more unit tests
guipublic Mar 20, 2025
afa5cc9
Merge branch 'master' into gd/issue_7272
guipublic Mar 20, 2025
02fe712
Merge branch 'master' into gd/issue_7272
guipublic Mar 21, 2025
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
60 changes: 60 additions & 0 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ mod traits;
mod type_alias;
mod visitor;

use noirc_errors::Located;
use noirc_errors::Location;
pub use visitor::AttributeTarget;
pub use visitor::Visitor;
Expand All @@ -33,6 +34,7 @@ pub use structure::*;
pub use traits::*;
pub use type_alias::*;

use crate::signed_field::SignedField;
use crate::{
BinaryTypeOperator,
node_interner::{InternedUnresolvedTypeData, QuotedTypeId},
Expand Down Expand Up @@ -450,6 +452,21 @@ impl UnresolvedTypeData {
| UnresolvedTypeData::Error => false,
}
}

pub(crate) fn try_into_expression(&self) -> Option<UnresolvedTypeExpression> {
match self {
UnresolvedTypeData::Expression(expr) => Some(expr.clone()),
UnresolvedTypeData::Parenthesized(unresolved_type) => {
unresolved_type.typ.try_into_expression()
}
UnresolvedTypeData::Named(path, generics, _)
if path.is_ident() && generics.is_empty() =>
{
Some(UnresolvedTypeExpression::Variable(path.clone()))
}
_ => None,
}
}
}

impl UnresolvedTypeExpression {
Expand Down Expand Up @@ -530,6 +547,38 @@ impl UnresolvedTypeExpression {
}
}

pub fn to_expression_kind(&self) -> ExpressionKind {
match self {
UnresolvedTypeExpression::Variable(path) => ExpressionKind::Variable(path.clone()),
UnresolvedTypeExpression::Constant(int, _) => {
ExpressionKind::Literal(Literal::Integer(SignedField {
field: *int,
is_negative: false,
}))
}
UnresolvedTypeExpression::BinaryOperation(lhs, op, rhs, location) => {
ExpressionKind::Infix(Box::new(InfixExpression {
lhs: Expression { kind: lhs.to_expression_kind(), location: *location },
operator: Located::from(*location, Self::operator_to_binary_op_kind_helper(op)),
rhs: Expression { kind: rhs.to_expression_kind(), location: *location },
}))
}
UnresolvedTypeExpression::AsTraitPath(path) => {
ExpressionKind::AsTraitPath(*path.clone())
}
}
}

fn operator_to_binary_op_kind_helper(op: &BinaryTypeOperator) -> BinaryOpKind {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a method on BinaryTypeOperator

match op {
BinaryTypeOperator::Addition => BinaryOpKind::Add,
BinaryTypeOperator::Subtraction => BinaryOpKind::Subtract,
BinaryTypeOperator::Multiplication => BinaryOpKind::Multiply,
BinaryTypeOperator::Division => BinaryOpKind::Divide,
BinaryTypeOperator::Modulo => BinaryOpKind::Modulo,
}
}

fn operator_allowed(op: BinaryOpKind) -> bool {
matches!(
op,
Expand All @@ -553,6 +602,17 @@ impl UnresolvedTypeExpression {
}
}
}

pub(crate) fn is_valid_expression(&self) -> bool {
match self {
UnresolvedTypeExpression::Variable(path) => path.is_ident(),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment in elaborator/mod.rs mentions this function should allow globals & constants. We can have multiple segment paths that still resolve to constants or globals. E.g. crate::CONSTANT or foo::bar::my_global, but this method disallows these.

UnresolvedTypeExpression::Constant(_, _) => true,
UnresolvedTypeExpression::BinaryOperation(lhs, _, rhs, _) => {
lhs.is_valid_expression() && rhs.is_valid_expression()
}
UnresolvedTypeExpression::AsTraitPath(_) => true,
}
}
}

#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
Expand Down
6 changes: 4 additions & 2 deletions compiler/noirc_frontend/src/ast/type_alias.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,18 @@ use noirc_errors::Location;
use std::fmt::Display;

/// Ast node for type aliases
/// Depending on 'numeric_type', a Type Alias can be an alias to a normal type, or to a numeric generic type
#[derive(Clone, Debug)]
pub struct NoirTypeAlias {
pub struct TypeAlias {
pub name: Ident,
pub generics: UnresolvedGenerics,
pub typ: UnresolvedType,
pub visibility: ItemVisibility,
pub location: Location,
pub numeric_type: Option<UnresolvedType>,
}

impl Display for NoirTypeAlias {
impl Display for TypeAlias {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let generics = vecmap(&self.generics, |generic| generic.to_string());
write!(f, "type {}<{}> = {}", self.name, generics.join(", "), self.typ)
Expand Down
12 changes: 6 additions & 6 deletions compiler/noirc_frontend/src/ast/visitor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use crate::{
CastExpression, ConstrainExpression, ConstructorExpression, Expression, ExpressionKind,
ForLoopStatement, ForRange, Ident, IfExpression, IndexExpression, InfixExpression, LValue,
Lambda, LetStatement, Literal, MemberAccessExpression, MethodCallExpression,
ModuleDeclaration, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, NoirTypeAlias, Path,
ModuleDeclaration, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl, Path,
PrefixExpression, Statement, StatementKind, TraitImplItem, TraitItem, TypeImpl, UseTree,
UseTreeKind,
},
Expand All @@ -22,9 +22,9 @@ use crate::{

use super::{
ForBounds, FunctionReturnType, GenericTypeArgs, IntegerBitSize, ItemVisibility,
MatchExpression, NoirEnumeration, Pattern, Signedness, TraitBound, TraitImplItemKind, TypePath,
UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
UnresolvedTypeExpression, UnsafeExpression,
MatchExpression, NoirEnumeration, Pattern, Signedness, TraitBound, TraitImplItemKind,
TypeAlias, TypePath, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType,
UnresolvedTypeData, UnresolvedTypeExpression, UnsafeExpression,
};

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

fn visit_noir_type_alias(&mut self, _: &NoirTypeAlias, _: Span) -> bool {
fn visit_noir_type_alias(&mut self, _: &TypeAlias, _: Span) -> bool {
true
}

Expand Down Expand Up @@ -815,7 +815,7 @@ impl NoirEnumeration {
}
}

impl NoirTypeAlias {
impl TypeAlias {
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
if visitor.visit_noir_type_alias(self, span) {
self.accept_children(visitor);
Expand Down
34 changes: 30 additions & 4 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1645,17 +1645,43 @@ impl<'context> Elaborator<'context> {

let name = &alias.type_alias_def.name;
let visibility = alias.type_alias_def.visibility;
let location = alias.type_alias_def.typ.location;
let location = alias.type_alias_def.location;

let generics = self.add_generics(&alias.type_alias_def.generics);
self.current_item = Some(DependencyId::Alias(alias_id));
let typ = self.resolve_type(alias.type_alias_def.typ);

let (typ, num_expr) = if let Some(num_type) = alias.type_alias_def.numeric_type {
let num_type = self.resolve_type(num_type);
let kind = Kind::numeric(num_type);
let num_expr = alias.type_alias_def.typ.typ.try_into_expression();

if let Some(num_expr) = num_expr {
// Checks that the expression only references generics and constants
if !num_expr.is_valid_expression() {
self.errors.push(CompilationError::ResolverError(
ResolverError::RecursiveTypeAlias { location },
));
(Type::Error, None)
} else {
(self.resolve_type_with_kind(alias.type_alias_def.typ, &kind), Some(num_expr))
}
} else {
self.errors.push(CompilationError::ResolverError(
ResolverError::ExpectedNumericExpression {
typ: alias.type_alias_def.typ.typ.to_string(),
location,
},
));
(Type::Error, None)
}
} else {
(self.resolve_type(alias.type_alias_def.typ), None)
};

if visibility != ItemVisibility::Private {
self.check_type_is_not_more_private_then_item(name, visibility, &typ, location);
}

self.interner.set_type_alias(alias_id, typ, generics);
self.interner.set_type_alias(alias_id, typ, generics, num_expr);
self.generics.clear();
}

Expand Down
26 changes: 24 additions & 2 deletions compiler/noirc_frontend/src/elaborator/patterns.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use crate::{
DataType, Kind, Shared, Type, TypeAlias, TypeBindings,
ast::{
ERROR_IDENT, Expression, ExpressionKind, Ident, ItemVisibility, Path, Pattern, TypePath,
UnresolvedType,
UnresolvedType, UnresolvedTypeExpression,
},
hir::{
def_collector::dc_crate::CompilationError,
Expand Down Expand Up @@ -537,11 +537,24 @@ impl Elaborator<'_> {

pub(super) fn elaborate_variable(&mut self, variable: Path) -> (ExprId, Type) {
let unresolved_turbofish = variable.segments.last().unwrap().generics.clone();

let location = variable.location;
let (expr, item) = self.resolve_variable(variable);
let definition_id = expr.id;

if expr.id == DefinitionId::dummy_id() {
if let Some(PathResolutionItem::TypeAlias(alias)) = item {
Comment on lines +544 to +545
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is the outer == dummy_id check necessary here? Seems like all numeric type aliases are expected to have that ID anyway and only checking for PathResolutionItem::TypeAlias would be more clear I think.

// A type alias to a numeric generics is considered like a variable
// but it is not a real variable so it does not resolve to a valid Identifier
// In order to handle this, we retrieve the numeric generics expression that the type aliases to
let type_alias = self.interner.get_type_alias(alias);
if let Some(expr) = type_alias.borrow().numeric_expr.clone() {
let expr = UnresolvedTypeExpression::to_expression_kind(&expr);
let expr = Expression::new(expr, type_alias.borrow().location);
return self.elaborate_expression(expr);
}
}
}

let type_generics = item.map(|item| self.resolve_item_turbofish(item)).unwrap_or_default();

let definition = self.interner.try_definition(definition_id);
Expand Down Expand Up @@ -860,6 +873,15 @@ impl Elaborator<'_> {
Err(_) => error,
},
None => match self.lookup_global(path) {
Ok((dummy_id, PathResolutionItem::TypeAlias(type_alias_id)))
if dummy_id == DefinitionId::dummy_id() =>
{
// Allow path which resolves to a type alias
return (
(HirIdent::non_trait_method(dummy_id, location), 4),
Some(PathResolutionItem::TypeAlias(type_alias_id)),
);
}
Ok((id, item)) => {
return ((HirIdent::non_trait_method(id, location), 0), Some(item));
}
Expand Down
29 changes: 23 additions & 6 deletions compiler/noirc_frontend/src/elaborator/scope.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,14 +88,31 @@ impl Elaborator<'_> {
return Ok((self.interner.function_definition_id(function), item));
}

if let PathResolutionItem::Global(global) = item {
let global = self.interner.get_global(global);
return Ok((global.definition_id, item));
}

let expected = "global variable";
let got = "local variable";
Err(ResolverError::Expected { location, expected, got })
match item {
PathResolutionItem::Global(global) => {
let global = self.interner.get_global(global);
Ok((global.definition_id, item))
}
PathResolutionItem::TypeAlias(type_alias_id) => {
let type_alias = self.interner.get_type_alias(type_alias_id);

if type_alias.borrow().numeric_expr.is_some() {
// Type alias to numeric generics are aliases to some global value
// Therefore we allow this case although we cannot provide the value yet
return Ok((DefinitionId::dummy_id(), item));
Comment on lines +102 to +104
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks potentially suspicious - when are these dummy ids filled in? If we assign each numeric type alias a DefinitionId we can probably do away with the dummy ids here.

}
if matches!(type_alias.borrow().typ, Type::Alias(_, _))
|| matches!(type_alias.borrow().typ, Type::Error)
{
// Type alias to a type alias is not supported, but the error is handled in define_type_alias()
return Ok((DefinitionId::dummy_id(), item));
}
Err(ResolverError::Expected { location, expected, got })
}
_ => Err(ResolverError::Expected { location, expected, got }),
}
}

pub fn push_scope(&mut self) {
Expand Down
21 changes: 20 additions & 1 deletion compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ impl Elaborator<'_> {
resolved_type
}

pub(crate) fn resolve_type_with_kind(&mut self, typ: UnresolvedType, kind: &Kind) -> Type {
let location = typ.location;
let resolved_type = self.resolve_type_inner(typ, kind);
if resolved_type.is_nested_slice() {
self.push_err(ResolverError::NestedSlices { location });
}
resolved_type
}

Comment on lines +64 to +72
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this added from this PR? We used to have this method although it looks like it was removed a while back. Just flagging this in case it was the result of a merge and we don't need it any more.

/// Translates an UnresolvedType into a Type and appends any
/// freshly created TypeVariables created to new_variables.
pub fn resolve_type_inner(&mut self, typ: UnresolvedType, kind: &Kind) -> Type {
Expand Down Expand Up @@ -512,7 +521,17 @@ impl Elaborator<'_> {
) -> Type {
match length {
UnresolvedTypeExpression::Variable(path) => {
let typ = self.resolve_named_type(path, GenericTypeArgs::default());
let mut ab = GenericTypeArgs::default();
// Use generics from path, if they exist
if let Some(last_segment) = path.segments.last() {
if let Some(generics) = &last_segment.generics {
ab.ordered_args = generics.clone();
}
}
let mut typ = self.resolve_named_type(path, ab);
if let Type::Alias(alias, vec) = typ {
typ = alias.borrow().get_type(&vec);
}
self.check_kind(typ, expected_kind, location)
}
UnresolvedTypeExpression::Constant(int, _span) => {
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,16 +14,16 @@ use crate::{Generics, Type};
use crate::hir::Context;
use crate::hir::resolution::import::{ImportDirective, resolve_import};

use crate::ast::{Expression, NoirEnumeration};
use crate::ast::{Expression, NoirEnumeration, TypeAlias};
use crate::node_interner::{
FuncId, GlobalId, ModuleAttributes, NodeInterner, ReferenceId, TraitId, TraitImplId,
TypeAliasId, TypeId,
};

use crate::ast::{
ExpressionKind, Ident, ItemVisibility, LetStatement, Literal, NoirFunction, NoirStruct,
NoirTrait, NoirTypeAlias, Path, PathSegment, UnresolvedGenerics, UnresolvedTraitConstraint,
UnresolvedType, UnsupportedNumericGenericType,
NoirTrait, Path, PathSegment, UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType,
UnsupportedNumericGenericType,
};

use crate::elaborator::FrontendOptions;
Expand Down Expand Up @@ -109,7 +109,7 @@ pub struct UnresolvedTraitImpl {
pub struct UnresolvedTypeAlias {
pub file_id: FileId,
pub module_id: LocalModuleId,
pub type_alias_def: NoirTypeAlias,
pub type_alias_def: TypeAlias,
}

#[derive(Debug, Clone)]
Expand Down
5 changes: 2 additions & 3 deletions compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,7 @@ use rustc_hash::FxHashMap as HashMap;
use crate::ast::{
Documented, Expression, FunctionDefinition, Ident, ItemVisibility, LetStatement,
ModuleDeclaration, NoirEnumeration, NoirFunction, NoirStruct, NoirTrait, NoirTraitImpl,
NoirTypeAlias, Pattern, TraitImplItemKind, TraitItem, TypeImpl, UnresolvedType,
UnresolvedTypeData,
Pattern, TraitImplItemKind, TraitItem, TypeAlias, TypeImpl, UnresolvedType, UnresolvedTypeData,
};
use crate::hir::resolution::errors::ResolverError;
use crate::node_interner::{ModuleAttributes, NodeInterner, ReferenceId, TypeId};
Expand Down Expand Up @@ -353,7 +352,7 @@ impl ModCollector<'_> {
fn collect_type_aliases(
&mut self,
context: &mut Context,
type_aliases: Vec<Documented<NoirTypeAlias>>,
type_aliases: Vec<Documented<TypeAlias>>,
krate: CrateId,
) -> Vec<CompilationError> {
let mut errors: Vec<CompilationError> = vec![];
Expand Down
Loading
Loading