Skip to content

Commit

Permalink
chore: Add deprecation message for default type in for loop (#2689)
Browse files Browse the repository at this point in the history
  • Loading branch information
kevaundray authored Sep 15, 2023
1 parent 31e489e commit 9b77c1a
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 5 deletions.
29 changes: 24 additions & 5 deletions compiler/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,14 @@ mod test {
"#;

type_check_src_code(src, vec![String::from("main"), String::from("foo")]);
// 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;
type_check_src_code_errors_expected(
src,
expected_num_errors,
vec![String::from("main"), String::from("foo")],
);
}
#[test]
fn basic_closure() {
Expand Down Expand Up @@ -389,15 +396,27 @@ mod test {
}
}

fn type_check_src_code(src: &str, func_namespace: Vec<String>) {
type_check_src_code_errors_expected(src, 0, func_namespace)
}
// This function assumes that there is only one function and this is the
// func id that is returned
fn type_check_src_code(src: &str, func_namespace: Vec<String>) {
fn type_check_src_code_errors_expected(
src: &str,
expected_number_errors: usize,
func_namespace: Vec<String>,
) {
let (program, errors) = parse_program(src);
let mut interner = NodeInterner::default();

// Using assert_eq here instead of assert(errors.is_empty()) displays
// the whole vec if the assert fails rather than just two booleans
assert_eq!(errors, vec![]);
assert_eq!(
errors.len(),
expected_number_errors,
"expected {} errors, but got {}, errors: {:?}",
expected_number_errors,
errors.len(),
errors
);

let main_id = interner.push_fn(HirFunction::empty());
interner.push_function_definition("main".into(), main_id);
Expand Down
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,8 @@ 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 @@ -132,6 +134,11 @@ 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: 4 additions & 0 deletions compiler/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1384,6 +1384,10 @@ where
.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

0 comments on commit 9b77c1a

Please sign in to comment.