Skip to content

Commit

Permalink
chore: Remove unused new_variables argument from `resolve_type_inne…
Browse files Browse the repository at this point in the history
…r` (noir-lang#5148)

# Description

## Problem\*

## Summary\*

This argument was previously used to add a fresh generic to a function's
signature when we found a slice type back when those used a generic for
the length. Since Slice types are now their own type this code path
isn't accessible anymore (*).

(\*) The codepath is technically accessible by trying to use a string
slice type but we don't support these yet anyway so I've removed them.

## 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.
  • Loading branch information
jfecher authored May 30, 2024
1 parent 80fd901 commit 05df0cc
Show file tree
Hide file tree
Showing 5 changed files with 46 additions and 121 deletions.
7 changes: 2 additions & 5 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub enum UnresolvedTypeData {
Integer(Signedness, IntegerBitSize), // u32 = Integer(unsigned, ThirtyTwo)
Bool,
Expression(UnresolvedTypeExpression),
String(Option<UnresolvedTypeExpression>),
String(UnresolvedTypeExpression),
FormatString(UnresolvedTypeExpression, Box<UnresolvedType>),
Unit,

Expand Down Expand Up @@ -191,10 +191,7 @@ impl std::fmt::Display for UnresolvedTypeData {
}
Expression(expression) => expression.fmt(f),
Bool => write!(f, "bool"),
String(len) => match len {
None => write!(f, "str<_>"),
Some(len) => write!(f, "str<{len}>"),
},
String(len) => write!(f, "str<{len}>"),
FormatString(len, elements) => write!(f, "fmt<{len}, {elements}"),
Function(args, ret, env) => {
let args = vecmap(args, ToString::to_string).join(", ");
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -566,7 +566,7 @@ impl<'context> Elaborator<'context> {
UnresolvedTypeData::TraitAsType(path, args) => {
self.desugar_impl_trait_arg(path, args, &mut generics, &mut trait_constraints)
}
_ => self.resolve_type_inner(typ, &mut generics),
_ => self.resolve_type_inner(typ),
};

self.check_if_type_is_valid_for_program_input(
Expand Down
77 changes: 20 additions & 57 deletions compiler/noirc_frontend/src/elaborator/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,67 +39,60 @@ impl<'context> Elaborator<'context> {
/// Translates an UnresolvedType to a Type
pub(super) fn resolve_type(&mut self, typ: UnresolvedType) -> Type {
let span = typ.span;
let resolved_type = self.resolve_type_inner(typ, &mut vec![]);
let resolved_type = self.resolve_type_inner(typ);
if resolved_type.is_nested_slice() {
self.push_err(ResolverError::NestedSlices { span: span.unwrap() });
}

resolved_type
}

/// 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,
new_variables: &mut Generics,
) -> Type {
pub fn resolve_type_inner(&mut self, typ: UnresolvedType) -> Type {
use crate::ast::UnresolvedTypeData::*;

let resolved_type = match typ.typ {
FieldElement => Type::FieldElement,
Array(size, elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, new_variables));
let size = self.resolve_array_size(Some(size), new_variables);
let elem = Box::new(self.resolve_type_inner(*elem));
let size = self.convert_expression_type(size);
Type::Array(Box::new(size), elem)
}
Slice(elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, new_variables));
let elem = Box::new(self.resolve_type_inner(*elem));
Type::Slice(elem)
}
Expression(expr) => self.convert_expression_type(expr),
Integer(sign, bits) => Type::Integer(sign, bits),
Bool => Type::Bool,
String(size) => {
let resolved_size = self.resolve_array_size(size, new_variables);
let resolved_size = self.convert_expression_type(size);
Type::String(Box::new(resolved_size))
}
FormatString(size, fields) => {
let resolved_size = self.convert_expression_type(size);
let fields = self.resolve_type_inner(*fields, new_variables);
let fields = self.resolve_type_inner(*fields);
Type::FmtString(Box::new(resolved_size), Box::new(fields))
}
Code => Type::Code,
Unit => Type::Unit,
Unspecified => Type::Error,
Error => Type::Error,
Named(path, args, _) => self.resolve_named_type(path, args, new_variables),
TraitAsType(path, args) => self.resolve_trait_as_type(path, args, new_variables),
Named(path, args, _) => self.resolve_named_type(path, args),
TraitAsType(path, args) => self.resolve_trait_as_type(path, args),

Tuple(fields) => {
Type::Tuple(vecmap(fields, |field| self.resolve_type_inner(field, new_variables)))
}
Tuple(fields) => Type::Tuple(vecmap(fields, |field| self.resolve_type_inner(field))),
Function(args, ret, env) => {
let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables));
let ret = Box::new(self.resolve_type_inner(*ret, new_variables));
let args = vecmap(args, |arg| self.resolve_type_inner(arg));
let ret = Box::new(self.resolve_type_inner(*ret));

// expect() here is valid, because the only places we don't have a span are omitted types
// e.g. a function without return type implicitly has a spanless UnresolvedType::Unit return type
// To get an invalid env type, the user must explicitly specify the type, which will have a span
let env_span =
env.span.expect("Unexpected missing span for closure environment type");

let env = Box::new(self.resolve_type_inner(*env, new_variables));
let env = Box::new(self.resolve_type_inner(*env));

match *env {
Type::Unit | Type::Tuple(_) | Type::NamedGeneric(_, _) => {
Expand All @@ -115,9 +108,9 @@ impl<'context> Elaborator<'context> {
}
}
MutableReference(element) => {
Type::MutableReference(Box::new(self.resolve_type_inner(*element, new_variables)))
Type::MutableReference(Box::new(self.resolve_type_inner(*element)))
}
Parenthesized(typ) => self.resolve_type_inner(*typ, new_variables),
Parenthesized(typ) => self.resolve_type_inner(*typ),
};

if let Type::Struct(_, _) = resolved_type {
Expand All @@ -136,12 +129,7 @@ impl<'context> Elaborator<'context> {
self.generics.iter().find(|(name, _, _)| name.as_ref() == target_name)
}

fn resolve_named_type(
&mut self,
path: Path,
args: Vec<UnresolvedType>,
new_variables: &mut Generics,
) -> Type {
fn resolve_named_type(&mut self, path: Path, args: Vec<UnresolvedType>) -> Type {
if args.is_empty() {
if let Some(typ) = self.lookup_generic_or_global_type(&path) {
return typ;
Expand All @@ -164,7 +152,7 @@ impl<'context> Elaborator<'context> {
}

let span = path.span();
let mut args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables));
let mut args = vecmap(args, |arg| self.resolve_type_inner(arg));

if let Some(type_alias) = self.lookup_type_alias(path.clone()) {
let type_alias = type_alias.borrow();
Expand Down Expand Up @@ -230,13 +218,8 @@ impl<'context> Elaborator<'context> {
}
}

fn resolve_trait_as_type(
&mut self,
path: Path,
args: Vec<UnresolvedType>,
new_variables: &mut Generics,
) -> Type {
let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables));
fn resolve_trait_as_type(&mut self, path: Path, args: Vec<UnresolvedType>) -> Type {
let args = vecmap(args, |arg| self.resolve_type_inner(arg));

if let Some(t) = self.lookup_trait_or_error(path) {
Type::TraitAsType(t.id, Rc::new(t.name.to_string()), args)
Expand Down Expand Up @@ -286,26 +269,6 @@ impl<'context> Elaborator<'context> {
}
}

fn resolve_array_size(
&mut self,
length: Option<UnresolvedTypeExpression>,
new_variables: &mut Generics,
) -> Type {
match length {
None => {
let id = self.interner.next_type_variable_id();
let typevar = TypeVariable::unbound(id);
new_variables.push(typevar.clone());

// '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()))
}
Some(length) => self.convert_expression_type(length),
}
}

pub(super) fn convert_expression_type(&mut self, length: UnresolvedTypeExpression) -> Type {
match length {
UnresolvedTypeExpression::Variable(path) => {
Expand Down Expand Up @@ -622,7 +585,7 @@ impl<'context> Elaborator<'context> {
pub(super) fn resolve_inferred_type(&mut self, typ: UnresolvedType) -> Type {
match &typ.typ {
UnresolvedTypeData::Unspecified => self.interner.next_type_variable(),
_ => self.resolve_type_inner(typ, &mut vec![]),
_ => self.resolve_type(typ),
}
}

Expand Down
77 changes: 22 additions & 55 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,53 +538,51 @@ impl<'a> Resolver<'a> {

/// Translates an UnresolvedType into a Type and appends any
/// freshly created TypeVariables created to new_variables.
fn resolve_type_inner(&mut self, typ: UnresolvedType, new_variables: &mut Generics) -> Type {
fn resolve_type_inner(&mut self, typ: UnresolvedType) -> Type {
use crate::ast::UnresolvedTypeData::*;

let resolved_type = match typ.typ {
FieldElement => Type::FieldElement,
Array(size, elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, new_variables));
let size = self.resolve_array_size(Some(size), new_variables);
let elem = Box::new(self.resolve_type_inner(*elem));
let size = self.convert_expression_type(size);
Type::Array(Box::new(size), elem)
}
Slice(elem) => {
let elem = Box::new(self.resolve_type_inner(*elem, new_variables));
let elem = Box::new(self.resolve_type_inner(*elem));
Type::Slice(elem)
}
Expression(expr) => self.convert_expression_type(expr),
Integer(sign, bits) => Type::Integer(sign, bits),
Bool => Type::Bool,
String(size) => {
let resolved_size = self.resolve_array_size(size, new_variables);
let resolved_size = self.convert_expression_type(size);
Type::String(Box::new(resolved_size))
}
FormatString(size, fields) => {
let resolved_size = self.convert_expression_type(size);
let fields = self.resolve_type_inner(*fields, new_variables);
let fields = self.resolve_type_inner(*fields);
Type::FmtString(Box::new(resolved_size), Box::new(fields))
}
Code => Type::Code,
Unit => Type::Unit,
Unspecified => Type::Error,
Error => Type::Error,
Named(path, args, _) => self.resolve_named_type(path, args, new_variables),
TraitAsType(path, args) => self.resolve_trait_as_type(path, args, new_variables),
Named(path, args, _) => self.resolve_named_type(path, args),
TraitAsType(path, args) => self.resolve_trait_as_type(path, args),

Tuple(fields) => {
Type::Tuple(vecmap(fields, |field| self.resolve_type_inner(field, new_variables)))
}
Tuple(fields) => Type::Tuple(vecmap(fields, |field| self.resolve_type_inner(field))),
Function(args, ret, env) => {
let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables));
let ret = Box::new(self.resolve_type_inner(*ret, new_variables));
let args = vecmap(args, |arg| self.resolve_type_inner(arg));
let ret = Box::new(self.resolve_type_inner(*ret));

// expect() here is valid, because the only places we don't have a span are omitted types
// e.g. a function without return type implicitly has a spanless UnresolvedType::Unit return type
// To get an invalid env type, the user must explicitly specify the type, which will have a span
let env_span =
env.span.expect("Unexpected missing span for closure environment type");

let env = Box::new(self.resolve_type_inner(*env, new_variables));
let env = Box::new(self.resolve_type_inner(*env));

match *env {
Type::Unit | Type::Tuple(_) | Type::NamedGeneric(_, _) => {
Expand All @@ -600,9 +598,9 @@ impl<'a> Resolver<'a> {
}
}
MutableReference(element) => {
Type::MutableReference(Box::new(self.resolve_type_inner(*element, new_variables)))
Type::MutableReference(Box::new(self.resolve_type_inner(*element)))
}
Parenthesized(typ) => self.resolve_type_inner(*typ, new_variables),
Parenthesized(typ) => self.resolve_type_inner(*typ),
};

if let Type::Struct(_, _) = resolved_type {
Expand All @@ -621,12 +619,7 @@ impl<'a> Resolver<'a> {
self.generics.iter().find(|(name, _, _)| name.as_ref() == target_name)
}

fn resolve_named_type(
&mut self,
path: Path,
args: Vec<UnresolvedType>,
new_variables: &mut Generics,
) -> Type {
fn resolve_named_type(&mut self, path: Path, args: Vec<UnresolvedType>) -> Type {
if args.is_empty() {
if let Some(typ) = self.lookup_generic_or_global_type(&path) {
return typ;
Expand All @@ -649,7 +642,7 @@ impl<'a> Resolver<'a> {
}

let span = path.span();
let mut args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables));
let mut args = vecmap(args, |arg| self.resolve_type_inner(arg));

if let Some(type_alias) = self.lookup_type_alias(path.clone()) {
let type_alias = type_alias.borrow();
Expand Down Expand Up @@ -715,13 +708,8 @@ impl<'a> Resolver<'a> {
}
}

fn resolve_trait_as_type(
&mut self,
path: Path,
args: Vec<UnresolvedType>,
new_variables: &mut Generics,
) -> Type {
let args = vecmap(args, |arg| self.resolve_type_inner(arg, new_variables));
fn resolve_trait_as_type(&mut self, path: Path, args: Vec<UnresolvedType>) -> Type {
let args = vecmap(args, |arg| self.resolve_type_inner(arg));

if let Some(t) = self.lookup_trait_or_error(path) {
Type::TraitAsType(t.id, Rc::new(t.name.to_string()), args)
Expand Down Expand Up @@ -774,26 +762,6 @@ impl<'a> Resolver<'a> {
}
}

fn resolve_array_size(
&mut self,
length: Option<UnresolvedTypeExpression>,
new_variables: &mut Generics,
) -> Type {
match length {
None => {
let id = self.interner.next_type_variable_id();
let typevar = TypeVariable::unbound(id);
new_variables.push(typevar.clone());

// '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()))
}
Some(length) => self.convert_expression_type(length),
}
}

fn convert_expression_type(&mut self, length: UnresolvedTypeExpression) -> Type {
match length {
UnresolvedTypeExpression::Variable(path) => {
Expand Down Expand Up @@ -846,11 +814,10 @@ impl<'a> Resolver<'a> {
/// Translates an UnresolvedType to a Type
pub fn resolve_type(&mut self, typ: UnresolvedType) -> Type {
let span = typ.span;
let resolved_type = self.resolve_type_inner(typ, &mut vec![]);
let resolved_type = self.resolve_type_inner(typ);
if resolved_type.is_nested_slice() {
self.errors.push(ResolverError::NestedSlices { span: span.unwrap() });
}

resolved_type
}

Expand Down Expand Up @@ -891,7 +858,7 @@ impl<'a> Resolver<'a> {
fn resolve_inferred_type(&mut self, typ: UnresolvedType) -> Type {
match &typ.typ {
UnresolvedTypeData::Unspecified => self.interner.next_type_variable(),
_ => self.resolve_type_inner(typ, &mut vec![]),
_ => self.resolve_type(typ),
}
}

Expand Down Expand Up @@ -1019,7 +986,7 @@ impl<'a> Resolver<'a> {
// indicate we should code generate in the same way. Thus, we unify the attributes into one flag here.
let has_inline_attribute = has_no_predicates_attribute || should_fold;

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

Expand All @@ -1032,7 +999,7 @@ impl<'a> Resolver<'a> {
}

let pattern = self.resolve_pattern(pattern, DefinitionKind::Local(None));
let typ = self.resolve_type_inner(typ, &mut generics);
let typ = self.resolve_type_inner(typ);

parameters.push((pattern, typ.clone(), visibility));
parameter_types.push(typ);
Expand Down
4 changes: 1 addition & 3 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -745,9 +745,7 @@ fn bool_type() -> impl NoirParser<UnresolvedType> {

fn string_type() -> impl NoirParser<UnresolvedType> {
keyword(Keyword::String)
.ignore_then(
type_expression().delimited_by(just(Token::Less), just(Token::Greater)).or_not(),
)
.ignore_then(type_expression().delimited_by(just(Token::Less), just(Token::Greater)))
.map_with_span(|expr, span| UnresolvedTypeData::String(expr).with_span(span))
}

Expand Down

0 comments on commit 05df0cc

Please sign in to comment.