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: resolve generic length of slice inconsistency #4232

Closed
wants to merge 19 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4cc3e78
add test for mapping slice
michaeljklein Jan 31, 2024
e750ae6
Merge branch 'master' into michaeljklein/map-slice
michaeljklein Feb 1, 2024
ac2af8b
wip: generic ident for N?-style generics, add lexer case, update pars…
michaeljklein Feb 5, 2024
28122b9
Merge branch 'master' into michaeljklein/map-slice
michaeljklein Feb 6, 2024
3593100
wip propagating prevent_numeric field
michaeljklein Feb 7, 2024
a988432
Merge branch 'master' into michaeljklein/map-slice
michaeljklein Feb 7, 2024
a1204ab
wip 'prevent_numeric', updating tuples, etc
michaeljklein Feb 12, 2024
2936374
Merge branch 'master' into michaeljklein/map-slice
michaeljklein Feb 13, 2024
1f04df2
Merge branch 'master' into michaeljklein/map-slice
michaeljklein Feb 14, 2024
010719a
wip: propagate named_generic, add formatting, add formatting and test…
michaeljklein Feb 14, 2024
7f3ee9c
Merge branch 'master' into michaeljklein/map-slice
michaeljklein Feb 19, 2024
c3b67aa
wip debugging propagation of N?
michaeljklein Feb 19, 2024
71b2ce1
Revert "wip debugging propagation of N?": need to propagate from some…
michaeljklein Feb 19, 2024
f4933b4
wip: add bool:prevent_numeric to Generics type, debugging failing tes…
michaeljklein Feb 19, 2024
ce415a2
wip debugging: seem to have found unrelated issue
michaeljklein Feb 20, 2024
793a52d
wip debugging, brought back annotation for array in node interner, fi…
michaeljklein Feb 21, 2024
034ed52
Merge branch 'master' into michaeljklein/map-slice
michaeljklein Feb 22, 2024
0f53ad3
clippy / fmt
michaeljklein Feb 22, 2024
ceebf8b
Merge branch 'master' into michaeljklein/map-slice
TomAFrench Feb 27, 2024
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
6 changes: 3 additions & 3 deletions compiler/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use std::fmt::Display;

use crate::token::{Attributes, Token};
use crate::{
Distinctness, FunctionVisibility, Ident, Path, Pattern, Recoverable, Statement, StatementKind,
UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
Distinctness, FunctionVisibility, GenericIdent, Ident, Path, Pattern, Recoverable, Statement,
StatementKind, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData, Visibility,
};
use acvm::FieldElement;
use iter_extended::vecmap;
Expand Down Expand Up @@ -32,7 +32,7 @@ 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<GenericIdent>;

impl ExpressionKind {
pub fn into_path(self) -> Option<Path> {
Expand Down
15 changes: 15 additions & 0 deletions compiler/noirc_frontend/src/ast/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -233,6 +233,21 @@ impl<T> Recoverable for Vec<T> {
}
}

#[derive(Eq, PartialEq, Debug, Clone)]
pub struct GenericIdent {
pub ident: Ident,
/// "N?" prevents "N" from being used as a numeric.
/// This allows "N" to be used for both array and slice lengths.
pub prevent_numeric: bool,
}

impl Display for GenericIdent {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let question_mark = if self.prevent_numeric { "?" } else { "" };
write!(f, "{}{}", self.ident, question_mark)
}
}

/// Trait for recoverable nodes during parsing.
/// This is similar to Default but is expected
/// to return an Error node of the appropriate type.
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/ast/structure.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::fmt::Display;

use crate::{token::SecondaryAttribute, Ident, UnresolvedGenerics, UnresolvedType};
use crate::{token::SecondaryAttribute, GenericIdent, Ident, UnresolvedGenerics, UnresolvedType};
use iter_extended::vecmap;
use noirc_errors::Span;

Expand All @@ -18,7 +18,7 @@ impl NoirStruct {
pub fn new(
name: Ident,
attributes: Vec<SecondaryAttribute>,
generics: Vec<Ident>,
generics: Vec<GenericIdent>,
fields: Vec<(Ident, UnresolvedType)>,
span: Span,
) -> NoirStruct {
Expand Down
8 changes: 4 additions & 4 deletions compiler/noirc_frontend/src/ast/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ use iter_extended::vecmap;
use noirc_errors::Span;

use crate::{
node_interner::TraitId, BlockExpression, Expression, FunctionReturnType, Ident, NoirFunction,
Path, UnresolvedGenerics, UnresolvedType,
node_interner::TraitId, BlockExpression, Expression, FunctionReturnType, GenericIdent, Ident,
NoirFunction, Path, UnresolvedGenerics, UnresolvedType,
};

/// AST node for trait definitions:
/// `trait name<generics> { ... items ... }`
#[derive(Clone, Debug)]
pub struct NoirTrait {
pub name: Ident,
pub generics: Vec<Ident>,
pub generics: Vec<GenericIdent>,
pub where_clause: Vec<UnresolvedTraitConstraint>,
pub span: Span,
pub items: Vec<TraitItem>,
Expand All @@ -25,7 +25,7 @@ pub struct NoirTrait {
pub enum TraitItem {
Function {
name: Ident,
generics: Vec<Ident>,
generics: Vec<GenericIdent>,
parameters: Vec<(Ident, UnresolvedType)>,
return_type: FunctionReturnType,
where_clause: Vec<UnresolvedTraitConstraint>,
Expand Down
4 changes: 2 additions & 2 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,7 +515,7 @@ pub(crate) fn check_methods_signatures(
}

// We also need to bind the traits generics to the trait's generics on the impl
for (generic, binding) in the_trait.generics.iter().zip(trait_generics) {
for ((generic, _prevent_numeric), binding) in the_trait.generics.iter().zip(trait_generics) {
generic.bind(binding);
}

Expand Down Expand Up @@ -623,7 +623,7 @@ pub(crate) fn check_methods_signatures(
the_trait.set_methods(trait_methods);
the_trait.self_type_typevar.unbind(the_trait.self_type_typevar_id);

for generic in &the_trait.generics {
for (generic, _prevent_numeric) in &the_trait.generics {
generic.unbind(generic.id());
}
}
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/hir/resolution/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub(crate) fn resolve_function_set(
mut unresolved_functions: UnresolvedFunctions,
self_type: Option<Type>,
trait_impl_id: Option<TraitImplId>,
impl_generics: Vec<(Rc<String>, TypeVariable, Span)>,
impl_generics: Vec<(Rc<String>, TypeVariable, Span, bool)>,
errors: &mut Vec<(CompilationError, FileId)>,
) -> Vec<(FileId, FuncId)> {
let file_id = unresolved_functions.file_id;
Expand Down
101 changes: 63 additions & 38 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ pub struct Resolver<'a> {
/// unique type variables if we're resolving a struct. Empty otherwise.
/// This is a Vec rather than a map to preserve the order a functions generics
/// were declared in.
generics: Vec<(Rc<String>, TypeVariable, Span)>,
/// The bool represents 'prevent_numeric', i.e. "N?"
generics: Vec<(Rc<String>, TypeVariable, Span, bool)>,

/// When resolving lambda expressions, we need to keep track of the variables
/// that are captured. We do this in order to create the hidden environment
Expand Down Expand Up @@ -509,7 +510,7 @@ impl<'a> Resolver<'a> {
let env = Box::new(self.resolve_type_inner(*env, new_variables));

match *env {
Type::Unit | Type::Tuple(_) | Type::NamedGeneric(_, _) => {
Type::Unit | Type::Tuple(_) | Type::NamedGeneric(_, _, _) => {
Type::Function(args, ret, env)
}
_ => {
Expand Down Expand Up @@ -539,8 +540,8 @@ impl<'a> Resolver<'a> {
resolved_type
}

fn find_generic(&self, target_name: &str) -> Option<&(Rc<String>, TypeVariable, Span)> {
self.generics.iter().find(|(name, _, _)| name.as_ref() == target_name)
fn find_generic(&self, target_name: &str) -> Option<&(Rc<String>, TypeVariable, Span, bool)> {
self.generics.iter().find(|(name, _, _, _)| name.as_ref() == target_name)
}

fn resolve_named_type(
Expand Down Expand Up @@ -657,8 +658,8 @@ impl<'a> Resolver<'a> {
fn lookup_generic_or_global_type(&mut self, path: &Path) -> Option<Type> {
if path.segments.len() == 1 {
let name = &path.last_segment().0.contents;
if let Some((name, var, _)) = self.find_generic(name) {
return Some(Type::NamedGeneric(var.clone(), name.clone()));
if let Some((name, var, _, prevent_numeric)) = self.find_generic(name) {
return Some(Type::NamedGeneric(var.clone(), name.clone(), *prevent_numeric));
}
}

Expand All @@ -683,12 +684,13 @@ impl<'a> Resolver<'a> {
None => {
let id = self.interner.next_type_variable_id();
let typevar = TypeVariable::unbound(id);
new_variables.push(typevar.clone());
let prevent_numeric = true;
new_variables.push((typevar.clone(), prevent_numeric));

// 'Named'Generic is a bit of a misnomer here, we want a type variable that
// wont be bound over but this one has no name since we do not currently
// require users to explicitly be generic over array lengths.
Type::NamedGeneric(typevar, Rc::new("".into()))
Type::NamedGeneric(typevar, Rc::new("".into()), prevent_numeric)
}
Some(length) => self.convert_expression_type(length),
}
Expand Down Expand Up @@ -775,14 +777,14 @@ impl<'a> Resolver<'a> {
/// Return the current generics.
/// Needed to keep referring to the same type variables across many
/// methods in a single impl.
pub fn get_generics(&self) -> &[(Rc<String>, TypeVariable, Span)] {
pub fn get_generics(&self) -> &[(Rc<String>, TypeVariable, Span, bool)] {
&self.generics
}

/// Set the current generics that are in scope.
/// Unlike add_generics, this function will not create any new type variables,
/// opting to reuse the existing ones it is directly given.
pub fn set_generics(&mut self, generics: Vec<(Rc<String>, TypeVariable, Span)>) {
pub fn set_generics(&mut self, generics: Vec<(Rc<String>, TypeVariable, Span, bool)>) {
self.generics = generics;
}

Expand All @@ -802,22 +804,23 @@ impl<'a> Resolver<'a> {
// Map the generic to a fresh type variable
let id = self.interner.next_type_variable_id();
let typevar = TypeVariable::unbound(id);
let span = generic.0.span();
let span = generic.ident.0.span();
let prevent_numeric = generic.prevent_numeric;

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

if let Some((_, _, first_span)) = self.find_generic(&name) {
if let Some((_, _, first_span, _)) = self.find_generic(&name) {
self.errors.push(ResolverError::DuplicateDefinition {
name: generic.0.contents.clone(),
name: generic.ident.0.contents.clone(),
first_span: *first_span,
second_span: span,
});
} else {
self.generics.push((name, typevar.clone(), span));
self.generics.push((name, typevar.clone(), span, prevent_numeric));
}

typevar
(typevar, prevent_numeric)
})
}

Expand All @@ -827,23 +830,35 @@ impl<'a> Resolver<'a> {
pub fn add_existing_generics(&mut self, names: &UnresolvedGenerics, generics: &Generics) {
assert_eq!(names.len(), generics.len());

for (name, typevar) in names.iter().zip(generics) {
self.add_existing_generic(&name.0.contents, name.0.span(), typevar.clone());
for (name, (typevar, prevent_numeric)) in names.iter().zip(generics) {
assert!(*prevent_numeric == name.prevent_numeric);
self.add_existing_generic(
&name.ident.0.contents,
name.ident.0.span(),
typevar.clone(),
name.prevent_numeric,
);
}
}

pub fn add_existing_generic(&mut self, name: &str, span: Span, typevar: TypeVariable) {
pub fn add_existing_generic(
&mut self,
name: &str,
span: Span,
typevar: TypeVariable,
prevent_numeric: bool,
) {
// Check for name collisions of this generic
let rc_name = Rc::new(name.to_owned());

if let Some((_, _, first_span)) = self.find_generic(&rc_name) {
if let Some((_, _, first_span, _)) = self.find_generic(&rc_name) {
self.errors.push(ResolverError::DuplicateDefinition {
name: name.to_owned(),
first_span: *first_span,
second_span: span,
});
} else {
self.generics.push((rc_name, typevar, span));
self.generics.push((rc_name, typevar, span, prevent_numeric));
}
}

Expand Down Expand Up @@ -899,7 +914,9 @@ impl<'a> Resolver<'a> {

let attributes = func.attributes().clone();

let mut generics = vecmap(&self.generics, |(_, typevar, _)| typevar.clone());
let mut generics = vecmap(&self.generics, |(_, typevar, _, prevent_numeric)| {
(typevar.clone(), *prevent_numeric)
});
let mut parameters = vec![];
let mut parameter_types = vec![];

Expand Down Expand Up @@ -1058,12 +1075,14 @@ impl<'a> Resolver<'a> {
// We can fail to find the generic in self.generics if it is an implicit one created
// by the compiler. This can happen when, e.g. eliding array lengths using the slice
// syntax [T].
if let Some((name, _, span)) =
self.generics.iter().find(|(name, _, _)| name.as_ref() == &name_to_find)
if let Some((name, _, span, prevent_numeric)) =
self.generics.iter().find(|(name, _, _, _)| name.as_ref() == &name_to_find)
{
let ident = Ident::new(name.to_string(), *span);
let definition = DefinitionKind::GenericType(type_variable);
self.add_variable_decl_inner(ident, false, false, false, definition);
if !*prevent_numeric {
let ident = Ident::new(name.to_string(), *span);
let definition = DefinitionKind::GenericType(type_variable);
self.add_variable_decl_inner(ident, false, false, false, definition);
}
}
}
}
Expand All @@ -1089,14 +1108,16 @@ impl<'a> Resolver<'a> {
| Type::Error
| Type::TypeVariable(_, _)
| Type::Constant(_)
| Type::NamedGeneric(_, _)
| Type::NamedGeneric(_, _, _)
| Type::NotConstant
| Type::TraitAsType(..)
| Type::Forall(_, _) => (),

Type::Array(length, element_type) => {
if let Type::NamedGeneric(type_variable, name) = length.as_ref() {
found.insert(name.to_string(), type_variable.clone());
if let Type::NamedGeneric(type_variable, name, prevent_numeric) = length.as_ref() {
if !prevent_numeric {
found.insert(name.to_string(), type_variable.clone());
}
}
Self::find_numeric_generics_in_type(element_type, found);
}
Expand All @@ -1116,8 +1137,8 @@ impl<'a> Resolver<'a> {

Type::Struct(struct_type, generics) => {
for (i, generic) in generics.iter().enumerate() {
if let Type::NamedGeneric(type_variable, name) = generic {
if struct_type.borrow().generic_is_numeric(i) {
if let Type::NamedGeneric(type_variable, name, prevent_numeric) = generic {
if struct_type.borrow().generic_is_numeric(i) && !prevent_numeric {
found.insert(name.to_string(), type_variable.clone());
}
} else {
Expand All @@ -1127,8 +1148,8 @@ impl<'a> Resolver<'a> {
}
Type::Alias(alias, generics) => {
for (i, generic) in generics.iter().enumerate() {
if let Type::NamedGeneric(type_variable, name) = generic {
if alias.borrow().generic_is_numeric(i) {
if let Type::NamedGeneric(type_variable, name, prevent_numeric) = generic {
if !prevent_numeric && alias.borrow().generic_is_numeric(i) {
found.insert(name.to_string(), type_variable.clone());
}
} else {
Expand All @@ -1138,13 +1159,17 @@ impl<'a> Resolver<'a> {
}
Type::MutableReference(element) => Self::find_numeric_generics_in_type(element, found),
Type::String(length) => {
if let Type::NamedGeneric(type_variable, name) = length.as_ref() {
found.insert(name.to_string(), type_variable.clone());
if let Type::NamedGeneric(type_variable, name, prevent_numeric) = length.as_ref() {
if !prevent_numeric {
found.insert(name.to_string(), type_variable.clone());
}
}
}
Type::FmtString(length, fields) => {
if let Type::NamedGeneric(type_variable, name) = length.as_ref() {
found.insert(name.to_string(), type_variable.clone());
if let Type::NamedGeneric(type_variable, name, prevent_numeric) = length.as_ref() {
if !prevent_numeric {
found.insert(name.to_string(), type_variable.clone());
}
}
Self::find_numeric_generics_in_type(fields, found);
}
Expand Down
Loading
Loading