diff --git a/crates/noirc_frontend/src/ast/expression.rs b/crates/noirc_frontend/src/ast/expression.rs index b1170ff0ed0..c7ed4d0c766 100644 --- a/crates/noirc_frontend/src/ast/expression.rs +++ b/crates/noirc_frontend/src/ast/expression.rs @@ -364,11 +364,19 @@ pub struct FunctionDefinition { pub body: BlockExpression, pub span: Span, pub where_clause: Vec, - pub return_type: UnresolvedType, + pub return_type: FunctionReturnType, pub return_visibility: noirc_abi::AbiVisibility, pub return_distinctness: noirc_abi::AbiDistinctness, } +#[derive(Debug, PartialEq, Eq, Clone)] +pub enum FunctionReturnType { + /// Returns type is not specified. + Default(Span), + /// Everything else. + Ty(UnresolvedType, Span), +} + /// Describes the types of smart contract functions that are allowed. /// - All Noir programs in the non-contract context can be seen as `Secret`. #[derive(serde::Serialize, serde::Deserialize, Debug, Copy, Clone, PartialEq, Eq)] @@ -636,3 +644,12 @@ impl Display for FunctionDefinition { ) } } + +impl Display for FunctionReturnType { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + FunctionReturnType::Default(_) => f.write_str(""), + FunctionReturnType::Ty(ty, _) => write!(f, "{ty}"), + } + } +} diff --git a/crates/noirc_frontend/src/ast/function.rs b/crates/noirc_frontend/src/ast/function.rs index 02af960f7a8..43db3294dd9 100644 --- a/crates/noirc_frontend/src/ast/function.rs +++ b/crates/noirc_frontend/src/ast/function.rs @@ -1,6 +1,6 @@ use std::fmt::Display; -use crate::{token::Attribute, Ident, Pattern}; +use crate::{token::Attribute, FunctionReturnType, Ident, Pattern}; use super::{FunctionDefinition, UnresolvedType}; @@ -41,7 +41,10 @@ impl NoirFunction { } pub fn return_type(&self) -> UnresolvedType { - self.def.return_type.clone() + match &self.def.return_type { + FunctionReturnType::Default(_) => UnresolvedType::Unit, + FunctionReturnType::Ty(ty, _) => ty.clone(), + } } pub fn name(&self) -> &str { &self.name_ident().0.contents diff --git a/crates/noirc_frontend/src/ast/traits.rs b/crates/noirc_frontend/src/ast/traits.rs index bb5d2117037..4a649a64cc6 100644 --- a/crates/noirc_frontend/src/ast/traits.rs +++ b/crates/noirc_frontend/src/ast/traits.rs @@ -3,7 +3,10 @@ use std::fmt::Display; use iter_extended::vecmap; use noirc_errors::Span; -use crate::{BlockExpression, Expression, Ident, NoirFunction, UnresolvedGenerics, UnresolvedType}; +use crate::{ + BlockExpression, Expression, FunctionReturnType, Ident, NoirFunction, UnresolvedGenerics, + UnresolvedType, +}; /// AST node for trait definitions: /// `trait name { ... items ... }` @@ -24,7 +27,7 @@ pub enum TraitItem { name: Ident, generics: Vec, parameters: Vec<(Ident, UnresolvedType)>, - return_type: UnresolvedType, + return_type: FunctionReturnType, where_clause: Vec, body: Option, }, diff --git a/crates/noirc_frontend/src/hir/resolution/resolver.rs b/crates/noirc_frontend/src/hir/resolution/resolver.rs index 4bfb5428ed7..6f30c9ded50 100644 --- a/crates/noirc_frontend/src/hir/resolution/resolver.rs +++ b/crates/noirc_frontend/src/hir/resolution/resolver.rs @@ -726,6 +726,7 @@ impl<'a> Resolver<'a> { location, typ, parameters: parameters.into(), + return_type: func.def.return_type.clone(), return_visibility: func.def.return_visibility, return_distinctness: func.def.return_distinctness, has_body: !func.def.body.is_empty(), diff --git a/crates/noirc_frontend/src/hir/type_check/errors.rs b/crates/noirc_frontend/src/hir/type_check/errors.rs index 84bf511d314..686c7c807b7 100644 --- a/crates/noirc_frontend/src/hir/type_check/errors.rs +++ b/crates/noirc_frontend/src/hir/type_check/errors.rs @@ -5,6 +5,7 @@ use thiserror::Error; use crate::hir::resolution::errors::ResolverError; use crate::hir_def::expr::HirBinaryOp; use crate::hir_def::types::Type; +use crate::FunctionReturnType; use crate::Signedness; #[derive(Error, Debug, Clone, PartialEq, Eq)] @@ -23,6 +24,8 @@ pub enum Source { Comparison, #[error("BinOp")] BinOp, + #[error("Return")] + Return(FunctionReturnType, Span), } #[derive(Error, Debug, Clone, PartialEq, Eq)] @@ -195,6 +198,21 @@ impl From for Diagnostic { Source::StringLen => format!("Can only compare strings of the same length. Here LHS is of length {lhs}, and RHS is {rhs}"), Source::Comparison => format!("Unsupported types for comparison: {lhs} and {rhs}"), Source::BinOp => format!("Unsupported types for binary operation: {lhs} and {rhs}"), + Source::Return(ret_ty, expr_span) => { + let ret_ty_span = match ret_ty { + FunctionReturnType::Default(span) | FunctionReturnType::Ty(_, span) => span + }; + + let mut diagnostic = Diagnostic::simple_error(format!("expected type {lhs}, found type {rhs}"), format!("expected {lhs} because of return type"), ret_ty_span); + + if let FunctionReturnType::Default(_) = ret_ty { + diagnostic.add_note(format!("help: try adding a return type: `-> {rhs}`")); + } + + diagnostic.add_secondary(format!("{rhs} returned here"), expr_span); + + return diagnostic + }, }; Diagnostic::simple_error(message, String::new(), span) diff --git a/crates/noirc_frontend/src/hir/type_check/mod.rs b/crates/noirc_frontend/src/hir/type_check/mod.rs index 8768120af2f..67107206683 100644 --- a/crates/noirc_frontend/src/hir/type_check/mod.rs +++ b/crates/noirc_frontend/src/hir/type_check/mod.rs @@ -14,10 +14,13 @@ mod stmt; pub use errors::TypeCheckError; use crate::{ + hir_def::{expr::HirExpression, stmt::HirStatement}, node_interner::{ExprId, FuncId, NodeInterner, StmtId}, Type, }; +use self::errors::Source; + type TypeCheckFn = Box Result<(), TypeCheckError>>; pub struct TypeChecker<'interner> { @@ -57,16 +60,29 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec Vec (noirc_errors::Span, bool) { + let (expr_span, empty_function) = + if let HirExpression::Block(block) = interner.expression(function_body_id) { + let last_stmt = block.statements().last(); + let mut span = interner.expr_span(function_body_id); + + if let Some(last_stmt) = last_stmt { + if let HirStatement::Expression(expr) = interner.statement(last_stmt) { + span = interner.expr_span(&expr); + } + } + + (span, last_stmt.is_none()) + } else { + (interner.expr_span(function_body_id), false) + }; + (expr_span, empty_function) +} + impl<'interner> TypeChecker<'interner> { fn new(interner: &'interner mut NodeInterner) -> Self { Self { delayed_type_checks: Vec::new(), interner, errors: vec![] } @@ -149,7 +187,6 @@ mod test { stmt::HirStatement, }; use crate::node_interner::{DefinitionKind, FuncId, NodeInterner}; - use crate::BinaryOpKind; use crate::{ hir::{ def_map::{CrateDefMap, LocalModuleId, ModuleDefId}, @@ -157,6 +194,7 @@ mod test { }, parse_program, FunctionKind, Path, }; + use crate::{BinaryOpKind, FunctionReturnType}; #[test] fn basic_let() { @@ -237,6 +275,7 @@ mod test { return_visibility: noirc_abi::AbiVisibility::Private, return_distinctness: noirc_abi::AbiDistinctness::DuplicationAllowed, has_body: true, + return_type: FunctionReturnType::Default(Span::default()), }; interner.push_fn_meta(func_meta, func_id); diff --git a/crates/noirc_frontend/src/hir_def/function.rs b/crates/noirc_frontend/src/hir_def/function.rs index 225731626f0..3a4cb2a1036 100644 --- a/crates/noirc_frontend/src/hir_def/function.rs +++ b/crates/noirc_frontend/src/hir_def/function.rs @@ -7,7 +7,7 @@ use super::stmt::HirPattern; use crate::hir::def_map::ModuleId; use crate::node_interner::{ExprId, NodeInterner}; use crate::{token::Attribute, FunctionKind}; -use crate::{ContractFunctionType, Type}; +use crate::{ContractFunctionType, FunctionReturnType, Type}; /// A Hir function is a block expression /// with a list of statements @@ -137,6 +137,8 @@ pub struct FuncMeta { pub parameters: Parameters, + pub return_type: FunctionReturnType, + pub return_visibility: AbiVisibility, pub return_distinctness: AbiDistinctness, diff --git a/crates/noirc_frontend/src/parser/parser.rs b/crates/noirc_frontend/src/parser/parser.rs index efa4e694cdc..c70a2e1dae8 100644 --- a/crates/noirc_frontend/src/parser/parser.rs +++ b/crates/noirc_frontend/src/parser/parser.rs @@ -23,6 +23,7 @@ //! prevent other parsers from being tried afterward since there is no longer an error. Thus, they should //! be limited to cases like the above `fn` example where it is clear we shouldn't back out of the //! current parser to try alternative parsers in a `choice` expression. +use super::spanned; use super::{ foldl_with_span, labels::ParsingRuleLabel, parameter_name_recovery, parameter_recovery, parenthesized, then_commit, then_commit_ignore, top_level_statement_recovery, ExprParser, @@ -34,10 +35,11 @@ use crate::lexer::Lexer; use crate::parser::{force, ignore_then_commit, statement_recovery}; use crate::token::{Attribute, Keyword, Token, TokenKind}; use crate::{ - BinaryOp, BinaryOpKind, BlockExpression, ConstrainStatement, FunctionDefinition, Ident, - IfExpression, InfixExpression, LValue, Lambda, Literal, NoirFunction, NoirStruct, NoirTrait, - NoirTypeAlias, Path, PathKind, Pattern, Recoverable, TraitConstraint, TraitImpl, TraitImplItem, - TraitItem, TypeImpl, UnaryOp, UnresolvedTypeExpression, UseTree, UseTreeKind, + BinaryOp, BinaryOpKind, BlockExpression, ConstrainStatement, FunctionDefinition, + FunctionReturnType, Ident, IfExpression, InfixExpression, LValue, Lambda, Literal, + NoirFunction, NoirStruct, NoirTrait, NoirTypeAlias, Path, PathKind, Pattern, Recoverable, + TraitConstraint, TraitImpl, TraitImplItem, TraitItem, TypeImpl, UnaryOp, + UnresolvedTypeExpression, UseTree, UseTreeKind, }; use chumsky::prelude::*; @@ -258,17 +260,19 @@ fn lambda_return_type() -> impl NoirParser { .map(|ret| ret.unwrap_or(UnresolvedType::Unspecified)) } -fn function_return_type() -> impl NoirParser<((AbiDistinctness, AbiVisibility), UnresolvedType)> { +fn function_return_type() -> impl NoirParser<((AbiDistinctness, AbiVisibility), FunctionReturnType)> +{ just(Token::Arrow) .ignore_then(optional_distinctness()) .then(optional_visibility()) - .then(parse_type()) + .then(spanned(parse_type())) .or_not() - .map(|ret| { - ret.unwrap_or(( + .map_with_span(|ret, span| match ret { + Some((head, (ty, span))) => (head, FunctionReturnType::Ty(ty, span)), + None => ( (AbiDistinctness::DuplicationAllowed, AbiVisibility::Private), - UnresolvedType::Unit, - )) + FunctionReturnType::Default(span), + ), }) }