From 8b89075273cdf915b04afb25850c55421488ffb0 Mon Sep 17 00:00:00 2001 From: Alex Vitkov Date: Tue, 15 Aug 2023 11:08:20 +0300 Subject: [PATCH 1/3] fix: warning if last expression of block is unused --- crates/noirc_frontend/src/ast/statement.rs | 8 +------ crates/noirc_frontend/src/graph/mod.rs | 2 +- .../src/hir/type_check/errors.rs | 9 +++++++ .../noirc_frontend/src/hir/type_check/expr.rs | 24 ++++++++++--------- 4 files changed, 24 insertions(+), 19 deletions(-) diff --git a/crates/noirc_frontend/src/ast/statement.rs b/crates/noirc_frontend/src/ast/statement.rs index e35394e072..7d51ed60b4 100644 --- a/crates/noirc_frontend/src/ast/statement.rs +++ b/crates/noirc_frontend/src/ast/statement.rs @@ -78,17 +78,11 @@ impl Statement { Statement::Expression(expr) } } - - // Don't wrap expressions that are not the last expression in - // a block in a Semi so that we can report errors in the type checker - // for unneeded expressions like { 1 + 2; 3 } - (_, Some(_), false) => Statement::Expression(expr), (_, None, false) => { emit_error(missing_semicolon); Statement::Expression(expr) } - - (_, Some(_), true) => Statement::Semi(expr), + (_, Some(_), _) => Statement::Semi(expr), (_, None, true) => Statement::Expression(expr), } } diff --git a/crates/noirc_frontend/src/graph/mod.rs b/crates/noirc_frontend/src/graph/mod.rs index 7a1daef45f..90490c47c3 100644 --- a/crates/noirc_frontend/src/graph/mod.rs +++ b/crates/noirc_frontend/src/graph/mod.rs @@ -30,7 +30,7 @@ impl CrateId { pub struct CrateName(SmolStr); impl CrateName { - fn is_valid_name(name: &str) -> bool { + fn is_valid_name(name: &str) -> bool { !name.is_empty() && name.chars().all(|n| !CHARACTER_BLACK_LIST.contains(&n)) } } diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 84bf511d31..259659764d 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -92,6 +92,8 @@ pub enum TypeCheckError { CallDeprecated { name: String, note: Option, span: Span }, #[error("{0}")] ResolverError(ResolverError), + #[error("Unused expression result (of type {expr_type})")] + UnusedResultError { expr_type: Type, expr_span: Span }, } impl TypeCheckError { @@ -205,6 +207,13 @@ impl From for Diagnostic { Diagnostic::simple_warning(primary_message, secondary_message, span) } + TypeCheckError::UnusedResultError { expr_type, expr_span } => { + Diagnostic::simple_warning( + format!("Unused expression result (of type {expr_type})"), + String::new(), + expr_span, + ) + } } } } diff --git a/crates/noirc_frontend/src/hir/type_check/expr.rs b/crates/noirc_frontend/src/hir/type_check/expr.rs index 99b312adb0..41d9843802 100644 --- a/crates/noirc_frontend/src/hir/type_check/expr.rs +++ b/crates/noirc_frontend/src/hir/type_check/expr.rs @@ -231,19 +231,21 @@ impl<'interner> TypeChecker<'interner> { for (i, stmt) in statements.iter().enumerate() { let expr_type = self.check_statement(stmt); - if i + 1 < statements.len() { - let id = match self.interner.statement(stmt) { - crate::hir_def::stmt::HirStatement::Expression(expr) => expr, - _ => *expr_id, - }; + if let crate::hir_def::stmt::HirStatement::Semi(expr) = + self.interner.statement(stmt) + { + let inner_expr_type = self.interner.id_type(expr); + let span = self.interner.expr_span(&expr); - let span = self.interner.expr_span(&id); - self.unify(&expr_type, &Type::Unit, || TypeCheckError::TypeMismatch { - expected_typ: Type::Unit.to_string(), - expr_typ: expr_type.to_string(), - expr_span: span, + self.unify(&inner_expr_type, &Type::Unit, || { + TypeCheckError::UnusedResultError { + expr_type: inner_expr_type.clone(), + expr_span: span, + } }); - } else { + } + + if i + 1 == statements.len() { block_type = expr_type; } } From 225d7e347f7f185a6a56d1c5a1b8b50d2271d69f Mon Sep 17 00:00:00 2001 From: Alex Vitkov <44268717+alexvitkov@users.noreply.github.com> Date: Wed, 16 Aug 2023 07:47:18 +0300 Subject: [PATCH 2/3] fix: UnusedResultError formatting change Co-authored-by: jfecher --- crates/noirc_frontend/src/hir/type_check/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 259659764d..45f378c1e8 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -209,7 +209,7 @@ impl From for Diagnostic { } TypeCheckError::UnusedResultError { expr_type, expr_span } => { Diagnostic::simple_warning( - format!("Unused expression result (of type {expr_type})"), + format!("Unused expression result of type {expr_type}"), String::new(), expr_span, ) From ea840540b1cbcd333a6094355d47f5bc284e6a50 Mon Sep 17 00:00:00 2001 From: Alex Vitkov <44268717+alexvitkov@users.noreply.github.com> Date: Wed, 16 Aug 2023 07:47:25 +0300 Subject: [PATCH 3/3] fix: UnusedResultError formatting change Co-authored-by: jfecher --- crates/noirc_frontend/src/hir/type_check/errors.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 45f378c1e8..991372c821 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -92,7 +92,7 @@ pub enum TypeCheckError { CallDeprecated { name: String, note: Option, span: Span }, #[error("{0}")] ResolverError(ResolverError), - #[error("Unused expression result (of type {expr_type})")] + #[error("Unused expression result of type {expr_type}")] UnusedResultError { expr_type: Type, expr_span: Span }, }