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.

Sign up for GitHub

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

Jump to bottom

feat!: Default integers to u64 #2764

Merged
merged 3 commits into from
Sep 22, 2023
Merged

feat!: Default integers to u64 #2764

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
76db7bb
Default integers to u64
guipublic Sep 20, 2023
94fdcd2
fix unit test
guipublic Sep 20, 2023
28bb461
default to u64 only for loop ranges
guipublic Sep 21, 2023
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
4 changes: 1 addition & 3 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -332,9 +332,7 @@ mod test {

"#;

// expect a deprecation warning since we are changing for-loop default type.
// There is a deprecation warning per for-loop.
let expected_num_errors = 2;
let expected_num_errors = 0;
type_check_src_code_errors_expected(
src,
expected_num_errors,
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/hir_def/types.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -410,6 +410,10 @@
Type::FieldElement
}

pub fn default_range_loop_type() -> Type {
Type::Integer(Signedness::Unsigned, 64)
}

pub fn type_variable(id: TypeVariableId) -> Type {
Type::TypeVariable(Shared::new(TypeBinding::Unbound(id)), TypeVariableKind::Normal)
}
Expand Down Expand Up @@ -563,7 +567,7 @@
Type::Bool => write!(f, "bool"),
Type::String(len) => write!(f, "str<{len}>"),
Type::FmtString(len, elements) => {
write!(f, "fmtstr<{len}, {elements}>")

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

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (fmtstr)
}
Type::Unit => write!(f, "()"),
Type::Error => write!(f, "error"),
Expand Down
85 changes: 48 additions & 37 deletions compiler/noirc_frontend/src/monomorphization/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@

next_local_id: u32,
next_function_id: u32,

is_range_loop: bool,
}

type HirType = crate::Type;
Expand Down Expand Up @@ -112,6 +114,7 @@
next_function_id: 0,
interner,
lambda_envs_stack: Vec::new(),
is_range_loop: false,
}
}

Expand Down Expand Up @@ -200,7 +203,7 @@
let meta = self.interner.function_meta(&f);
let name = self.interner.function_name(&f).to_owned();

let return_type = Self::convert_type(meta.return_type());
let return_type = self.convert_type(meta.return_type());
let parameters = self.parameters(meta.parameters);
let body = self.expr(*self.interner.function(&f).as_expr());
let unconstrained = meta.is_unconstrained
Expand Down Expand Up @@ -236,7 +239,7 @@
let new_id = self.next_local_id();
let definition = self.interner.definition(ident.id);
let name = definition.name.clone();
new_params.push((new_id, definition.mutable, name, Self::convert_type(typ)));
new_params.push((new_id, definition.mutable, name, self.convert_type(typ)));
self.define_local(ident.id, new_id);
}
HirPattern::Mutable(pattern, _) => self.parameter(*pattern, typ, new_params),
Expand Down Expand Up @@ -283,7 +286,7 @@
}
HirExpression::Literal(HirLiteral::Bool(value)) => Literal(Bool(value)),
HirExpression::Literal(HirLiteral::Integer(value)) => {
let typ = Self::convert_type(&self.interner.id_type(expr));
let typ = self.convert_type(&self.interner.id_type(expr));
Literal(Integer(value, typ))
}
HirExpression::Literal(HirLiteral::Array(array)) => match array {
Expand All @@ -300,7 +303,7 @@
ast::Expression::Unary(ast::Unary {
operator: prefix.operator,
rhs: Box::new(self.expr(prefix.rhs)),
result_type: Self::convert_type(&self.interner.id_type(expr)),
result_type: self.convert_type(&self.interner.id_type(expr)),
location,
})
}
Expand All @@ -325,13 +328,15 @@

HirExpression::Cast(cast) => ast::Expression::Cast(ast::Cast {
lhs: Box::new(self.expr(cast.lhs)),
r#type: Self::convert_type(&cast.r#type),
r#type: self.convert_type(&cast.r#type),
location: self.interner.expr_location(&expr),
}),

HirExpression::For(for_expr) => {
self.is_range_loop = true;
let start = self.expr(for_expr.start_range);
let end = self.expr(for_expr.end_range);
self.is_range_loop = false;
kevaundray marked this conversation as resolved.
Show resolved Hide resolved
let index_variable = self.next_local_id();
self.define_local(for_expr.identifier.id, index_variable);

Expand All @@ -340,7 +345,7 @@
ast::Expression::For(ast::For {
index_variable,
index_name: self.interner.definition_name(for_expr.identifier.id).to_owned(),
index_type: Self::convert_type(&self.interner.id_type(for_expr.start_range)),
index_type: self.convert_type(&self.interner.id_type(for_expr.start_range)),
start_range: Box::new(start),
end_range: Box::new(end),
start_range_location: self.interner.expr_location(&for_expr.start_range),
Expand All @@ -357,7 +362,7 @@
condition: Box::new(cond),
consequence: Box::new(then),
alternative: else_,
typ: Self::convert_type(&self.interner.id_type(expr)),
typ: self.convert_type(&self.interner.id_type(expr)),
})
}

Expand All @@ -381,7 +386,7 @@
array: node_interner::ExprId,
array_elements: Vec<node_interner::ExprId>,
) -> ast::Expression {
let typ = Self::convert_type(&self.interner.id_type(array));
let typ = self.convert_type(&self.interner.id_type(array));
let contents = vecmap(array_elements, |id| self.expr(id));
ast::Expression::Literal(ast::Literal::Array(ast::ArrayLiteral { contents, typ }))
}
Expand All @@ -392,7 +397,7 @@
repeated_element: node_interner::ExprId,
length: HirType,
) -> ast::Expression {
let typ = Self::convert_type(&self.interner.id_type(array));
let typ = self.convert_type(&self.interner.id_type(array));

let contents = self.expr(repeated_element);
let length = length
Expand All @@ -404,7 +409,7 @@
}

fn index(&mut self, id: node_interner::ExprId, index: HirIndexExpression) -> ast::Expression {
let element_type = Self::convert_type(&self.interner.id_type(id));
let element_type = self.convert_type(&self.interner.id_type(id));

let collection = Box::new(self.expr(index.collection));
let index = Box::new(self.expr(index.index));
Expand Down Expand Up @@ -451,7 +456,7 @@
for (field_name, expr_id) in constructor.fields {
let new_id = self.next_local_id();
let field_type = field_type_map.get(&field_name.0.contents).unwrap();
let typ = Self::convert_type(field_type);
let typ = self.convert_type(field_type);

field_vars.insert(field_name.0.contents.clone(), (new_id, typ));
let expression = Box::new(self.expr(expr_id));
Expand Down Expand Up @@ -547,7 +552,7 @@
let mutable = false;
let definition = Definition::Local(fresh_id);
let name = i.to_string();
let typ = Self::convert_type(&field_type);
let typ = self.convert_type(&field_type);

let new_rhs =
ast::Expression::Ident(ast::Ident { location, mutable, definition, name, typ });
Expand Down Expand Up @@ -589,7 +594,7 @@
let mutable = definition.mutable;

let definition = self.lookup_local(ident.id)?;
let typ = Self::convert_type(&self.interner.id_type(ident.id));
let typ = self.convert_type(&self.interner.id_type(ident.id));

Some(ast::Ident { location: Some(ident.location), mutable, definition, name, typ })
}
Expand All @@ -604,7 +609,7 @@
let typ = self.interner.id_type(expr_id);

let definition = self.lookup_function(*func_id, expr_id, &typ);
let typ = Self::convert_type(&typ);
let typ = self.convert_type(&typ);
let ident = ast::Ident { location, mutable, definition, name, typ: typ.clone() };
let ident_expression = ast::Expression::Ident(ident);
if self.is_function_closure_type(&typ) {
Expand Down Expand Up @@ -641,21 +646,21 @@
}

/// Convert a non-tuple/struct type to a monomorphized type
fn convert_type(typ: &HirType) -> ast::Type {
fn convert_type(&self, typ: &HirType) -> ast::Type {
match typ {
HirType::FieldElement => ast::Type::Field,
HirType::Integer(sign, bits) => ast::Type::Integer(*sign, *bits),
HirType::Bool => ast::Type::Bool,
HirType::String(size) => ast::Type::String(size.evaluate_to_u64().unwrap_or(0)),
HirType::FmtString(size, fields) => {
let size = size.evaluate_to_u64().unwrap_or(0);
let fields = Box::new(Self::convert_type(fields.as_ref()));
let fields = Box::new(self.convert_type(fields.as_ref()));
ast::Type::FmtString(size, fields)
}
HirType::Unit => ast::Type::Unit,

HirType::Array(length, element) => {
let element = Box::new(Self::convert_type(element.as_ref()));
let element = Box::new(self.convert_type(element.as_ref()));

if let Some(length) = length.evaluate_to_u64() {
ast::Type::Array(length, element)
Expand All @@ -666,7 +671,7 @@

HirType::NamedGeneric(binding, _) => {
if let TypeBinding::Bound(binding) = &*binding.borrow() {
return Self::convert_type(binding);
return self.convert_type(binding);
}

// Default any remaining unbound type variables.
Expand All @@ -682,7 +687,7 @@

HirType::TypeVariable(binding, kind) => {
if let TypeBinding::Bound(binding) = &*binding.borrow() {
return Self::convert_type(binding);
return self.convert_type(binding);
}

// Default any remaining unbound type variables.
Expand All @@ -692,27 +697,33 @@
// like automatic solving of traits. It should be fine since it is strictly
// after type checking, but care should be taken that it doesn't change which
// impls are chosen.
let default = kind.default_type();
let monomorphized_default = Self::convert_type(&default);
let default =
if self.is_range_loop && matches!(kind, TypeVariableKind::IntegerOrField) {
Type::default_range_loop_type()
} else {
kind.default_type()
};

let monomorphized_default = self.convert_type(&default);
*binding.borrow_mut() = TypeBinding::Bound(default);
monomorphized_default
}

HirType::Struct(def, args) => {
let fields = def.borrow().get_fields(args);
let fields = vecmap(fields, |(_, field)| Self::convert_type(&field));
let fields = vecmap(fields, |(_, field)| self.convert_type(&field));
ast::Type::Tuple(fields)
}

HirType::Tuple(fields) => {
let fields = vecmap(fields, Self::convert_type);
let fields = vecmap(fields, |x| self.convert_type(x));
ast::Type::Tuple(fields)
}

HirType::Function(args, ret, env) => {
let args = vecmap(args, Self::convert_type);
let ret = Box::new(Self::convert_type(ret));
let env = Self::convert_type(env);
let args = vecmap(args, |x| self.convert_type(x));
let ret = Box::new(self.convert_type(ret));
let env = self.convert_type(env);
match &env {
ast::Type::Unit => ast::Type::Function(args, ret, Box::new(env)),
ast::Type::Tuple(_elements) => ast::Type::Tuple(vec![
Expand All @@ -728,7 +739,7 @@
}

HirType::MutableReference(element) => {
let element = Self::convert_type(element);
let element = self.convert_type(element);
ast::Type::MutableReference(Box::new(element))
}

Expand All @@ -742,7 +753,7 @@
}

fn is_function_closure(&self, raw_func_id: node_interner::ExprId) -> bool {
let t = Self::convert_type(&self.interner.id_type(raw_func_id));
let t = self.convert_type(&self.interner.id_type(raw_func_id));
if self.is_function_closure_type(&t) {
true
} else if let ast::Type::Tuple(elements) = t {
Expand Down Expand Up @@ -775,7 +786,7 @@
let hir_arguments = vecmap(&call.arguments, |id| self.interner.expression(id));
let func: Box<ast::Expression>;
let return_type = self.interner.id_type(id);
let return_type = Self::convert_type(&return_type);
let return_type = self.convert_type(&return_type);
let location = call.location;

if let ast::Expression::Ident(ident) = original_func.as_ref() {
Expand Down Expand Up @@ -810,7 +821,7 @@
definition: Definition::Local(local_id),
mutable: false,
name: "tmp".to_string(),
typ: Self::convert_type(&self.interner.id_type(call.func)),
typ: self.convert_type(&self.interner.id_type(call.func)),
});

func = Box::new(ast::Expression::ExtractTupleField(
Expand Down Expand Up @@ -1009,12 +1020,12 @@
let location = self.interner.expr_location(&index);
let array = Box::new(self.lvalue(*array));
let index = Box::new(self.expr(index));
let element_type = Self::convert_type(&typ);
let element_type = self.convert_type(&typ);
ast::LValue::Index { array, index, element_type, location }
}
HirLValue::Dereference { lvalue, element_type } => {
let reference = Box::new(self.lvalue(*lvalue));
let element_type = Self::convert_type(&element_type);
let element_type = self.convert_type(&element_type);
ast::LValue::Dereference { reference, element_type }
}
}
Expand All @@ -1030,9 +1041,9 @@
}

fn lambda_no_capture(&mut self, lambda: HirLambda) -> ast::Expression {
let ret_type = Self::convert_type(&lambda.return_type);
let ret_type = self.convert_type(&lambda.return_type);
let lambda_name = "lambda";
let parameter_types = vecmap(&lambda.parameters, |(_, typ)| Self::convert_type(typ));
let parameter_types = vecmap(&lambda.parameters, |(_, typ)| self.convert_type(typ));

// Manually convert to Parameters type so we can reuse the self.parameters method
let parameters =
Expand Down Expand Up @@ -1069,7 +1080,7 @@
) -> (ast::Expression, ast::Expression) {
// returns (<closure setup>, <closure variable>)
// which can be used directly in callsites or transformed
// directly to a single `Expression`

Check warning on line 1083 in compiler/noirc_frontend/src/monomorphization/mod.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (callsites)
// for other cases by `lambda` which is called by `expr`
//
// it solves the problem of detecting special cases where
Expand All @@ -1081,9 +1092,9 @@
// patterns in the resulting tree,
// which seems more fragile, we directly reuse the return parameters
// of this function in those cases
let ret_type = Self::convert_type(&lambda.return_type);
let ret_type = self.convert_type(&lambda.return_type);
let lambda_name = "lambda";
let parameter_types = vecmap(&lambda.parameters, |(_, typ)| Self::convert_type(typ));
let parameter_types = vecmap(&lambda.parameters, |(_, typ)| self.convert_type(typ));

// Manually convert to Parameters type so we can reuse the self.parameters method
let parameters =
Expand Down Expand Up @@ -1116,7 +1127,7 @@
}));
let expr_type = self.interner.id_type(expr);
let env_typ = if let types::Type::Function(_, _, function_env_type) = expr_type {
Self::convert_type(&function_env_type)
self.convert_type(&function_env_type)
} else {
unreachable!("expected a Function type for a Lambda node")
};
Expand Down
7 changes: 0 additions & 7 deletions compiler/noirc_frontend/src/parser/errors.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,6 @@ pub enum ParserErrorReason {
PatternInTraitFunctionParameter,
#[error("comptime keyword is deprecated")]
ComptimeDeprecated,
#[error("the default type of a for-loop range will be changing from a Field to a u64")]
ForLoopDefaultTypeChanging,
#[error("{0} are experimental and aren't fully supported yet")]
ExperimentalFeature(&'static str),
#[error("Where clauses are allowed only on functions with generic parameters")]
Expand Down Expand Up @@ -134,11 +132,6 @@ impl From<ParserError> for Diagnostic {
"The 'comptime' keyword has been deprecated. It can be removed without affecting your program".into(),
error.span,
),
ParserErrorReason::ForLoopDefaultTypeChanging => Diagnostic::simple_warning(
"The default type for the incrementor in a for-loop will be changed.".into(),
"The default type in a for-loop will be changing from Field to u64".into(),
error.span,
),
ParserErrorReason::ExperimentalFeature(_) => Diagnostic::simple_warning(
reason.to_string(),
"".into(),
Expand Down
4 changes: 0 additions & 4 deletions compiler/noirc_frontend/src/parser/parser.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -347,10 +347,10 @@
.or(mut_pattern)
.map_with_span(|token, span| (token, span))
.or_not()
.then(filter_map(move |span, found: Token| match found {

Check warning on line 350 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (refmut)
Token::Ident(ref word) if word == "self" => Ok(span),
_ => Err(ParserError::expected_label(ParsingRuleLabel::Parameter, found, span)),
}))

Check warning on line 353 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (refmut)
.map(|(pattern_keyword, span)| {
let ident = Ident::new("self".to_string(), span);
let path = Path::from_single("Self".to_owned(), span);
Expand Down Expand Up @@ -922,7 +922,7 @@
LValue::MemberAccess { object: Box::new(lvalue), field_name }
}
LValueRhs::Index(index) => LValue::Index { array: Box::new(lvalue), index },
});

Check warning on line 925 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (thans)

dereferences.then(lvalues).foldr(|_, lvalue| LValue::Dereference(Box::new(lvalue)))
}
Expand Down Expand Up @@ -950,7 +950,7 @@
}

fn optional_visibility() -> impl NoirParser<Visibility> {
keyword(Keyword::Pub).or_not().map(|opt| match opt {

Check warning on line 953 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (lvalues)
Some(_) => Visibility::Public,
None => Visibility::Private,
})
Expand All @@ -958,7 +958,7 @@

fn optional_distinctness() -> impl NoirParser<Distinctness> {
keyword(Keyword::Distinct).or_not().map(|opt| match opt {
Some(_) => Distinctness::Distinct,

Check warning on line 961 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (lvalues)

Check warning on line 961 in compiler/noirc_frontend/src/parser/parser.rs

View workflow job for this annotation

GitHub Actions / Spellcheck / Spellcheck 

Unknown word (foldr)
None => Distinctness::DuplicationAllowed,
})
}
Expand Down Expand Up @@ -1384,10 +1384,6 @@
.then(expr_no_constructors.clone())
.map(|(start, end)| ForRange::Range(start, end))
.or(expr_no_constructors.map(ForRange::Array))
.validate(|expr, span, emit| {
emit(ParserError::with_reason(ParserErrorReason::ForLoopDefaultTypeChanging, span));
expr
})
}

fn array_expr<P>(expr_parser: P) -> impl NoirParser<ExpressionKind>
Expand Down