Skip to content

Commit

Permalink
Merge branch 'master' into intrinsic-die
Browse files Browse the repository at this point in the history
* master:
  feat: Improved error message for unexpected return type (#2302)
  feat(stdlib): Implement `str` `as_bytes` and `into_bytes` function (#2298)
  chore(ci): automatically convert changelog entries to sentence case (#2325)
  • Loading branch information
TomAFrench committed Aug 15, 2023
2 parents 6f24de7 + d7e1e65 commit 9db7912
Show file tree
Hide file tree
Showing 16 changed files with 157 additions and 22 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "str_as_bytes"
type = "bin"
authors = [""]
compiler_version = "0.9.0"

[dependencies]
18 changes: 18 additions & 0 deletions crates/nargo_cli/tests/execution_success/str_as_bytes/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
use dep::std;
fn main() {
let a = "hello";
let b = a.as_bytes();
assert(b[0]==104);
assert(b[1]==101);
assert(b[2]==108);
assert(b[3]==108);
assert(b[4]==111);
assert(b.len()==5);
let mut c = a.as_bytes_vec();
assert(c.get(0)==104);
assert(c.get(1)==101);
assert(c.get(2)==108);
assert(c.get(3)==108);
assert(c.get(4)==111);
assert(c.len()==5);
}
3 changes: 3 additions & 0 deletions crates/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ pub(crate) enum Intrinsic {
SlicePopFront,
SliceInsert,
SliceRemove,
StrAsBytes,
Println,
ToBits(Endian),
ToRadix(Endian),
Expand All @@ -60,6 +61,7 @@ impl std::fmt::Display for Intrinsic {
Intrinsic::SlicePopFront => write!(f, "slice_pop_front"),
Intrinsic::SliceInsert => write!(f, "slice_insert"),
Intrinsic::SliceRemove => write!(f, "slice_remove"),
Intrinsic::StrAsBytes => write!(f, "str_as_bytes"),
Intrinsic::ToBits(Endian::Big) => write!(f, "to_be_bits"),
Intrinsic::ToBits(Endian::Little) => write!(f, "to_le_bits"),
Intrinsic::ToRadix(Endian::Big) => write!(f, "to_be_radix"),
Expand Down Expand Up @@ -107,6 +109,7 @@ impl Intrinsic {
"slice_pop_front" => Some(Intrinsic::SlicePopFront),
"slice_insert" => Some(Intrinsic::SliceInsert),
"slice_remove" => Some(Intrinsic::SliceRemove),
"str_as_bytes" => Some(Intrinsic::StrAsBytes),
"to_le_radix" => Some(Intrinsic::ToRadix(Endian::Little)),
"to_be_radix" => Some(Intrinsic::ToRadix(Endian::Big)),
"to_le_bits" => Some(Intrinsic::ToBits(Endian::Little)),
Expand Down
4 changes: 4 additions & 0 deletions crates/noirc_evaluator/src/ssa/ir/instruction/call.rs
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,10 @@ pub(super) fn simplify_call(
SimplifyResult::None
}
}
Intrinsic::StrAsBytes => {
// Strings are already represented as bytes internally
SimplifyResult::SimplifiedTo(arguments[0])
}
Intrinsic::AssertConstant => {
if arguments.iter().all(|argument| dfg.is_constant(*argument)) {
SimplifyResult::Remove
Expand Down
19 changes: 18 additions & 1 deletion crates/noirc_frontend/src/ast/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,11 +364,19 @@ pub struct FunctionDefinition {
pub body: BlockExpression,
pub span: Span,
pub where_clause: Vec<TraitConstraint>,
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)]
Expand Down Expand Up @@ -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}"),
}
}
}
7 changes: 5 additions & 2 deletions crates/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
@@ -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};

Expand Down Expand Up @@ -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
Expand Down
7 changes: 5 additions & 2 deletions crates/noirc_frontend/src/ast/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<generics> { ... items ... }`
Expand All @@ -24,7 +27,7 @@ pub enum TraitItem {
name: Ident,
generics: Vec<Ident>,
parameters: Vec<(Ident, UnresolvedType)>,
return_type: UnresolvedType,
return_type: FunctionReturnType,
where_clause: Vec<TraitConstraint>,
body: Option<BlockExpression>,
},
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down
18 changes: 18 additions & 0 deletions crates/noirc_frontend/src/hir/type_check/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,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)]
Expand All @@ -24,6 +25,8 @@ pub enum Source {
Comparison,
#[error("BinOp")]
BinOp,
#[error("Return")]
Return(FunctionReturnType, Span),
}

#[derive(Error, Debug, Clone, PartialEq, Eq)]
Expand Down Expand Up @@ -199,6 +202,21 @@ impl From<TypeCheckError> 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)
Expand Down
49 changes: 44 additions & 5 deletions crates/noirc_frontend/src/hir/type_check/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<dyn FnOnce() -> Result<(), TypeCheckError>>;

pub struct TypeChecker<'interner> {
Expand Down Expand Up @@ -57,23 +60,58 @@ pub fn type_check_func(interner: &mut NodeInterner, func_id: FuncId) -> Vec<Type

// Check declared return type and actual return type
if !can_ignore_ret {
let (expr_span, empty_function) = function_info(interner, function_body_id);

let func_span = interner.expr_span(function_body_id); // XXX: We could be more specific and return the span of the last stmt, however stmts do not have spans yet
function_last_type.unify_with_coercions(
&declared_return_type,
*function_body_id,
interner,
&mut errors,
|| TypeCheckError::TypeMismatch {
expected_typ: declared_return_type.to_string(),
expr_typ: function_last_type.to_string(),
expr_span: func_span,
|| {
let mut error = TypeCheckError::TypeMismatchWithSource {
lhs: declared_return_type.clone(),
rhs: function_last_type.clone(),
span: func_span,
source: Source::Return(meta.return_type, expr_span),
};

if empty_function {
error = error.add_context(
"implicitly returns `()` as its body has no tail or `return` expression",
);
}

error
},
);
}

errors
}

fn function_info(
interner: &mut NodeInterner,
function_body_id: &ExprId,
) -> (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![] }
Expand Down Expand Up @@ -149,14 +187,14 @@ mod test {
stmt::HirStatement,
};
use crate::node_interner::{DefinitionKind, FuncId, NodeInterner};
use crate::BinaryOpKind;
use crate::{
hir::{
def_map::{CrateDefMap, LocalModuleId, ModuleDefId},
resolution::{path_resolver::PathResolver, resolver::Resolver},
},
parse_program, FunctionKind, Path,
};
use crate::{BinaryOpKind, FunctionReturnType};

#[test]
fn basic_let() {
Expand Down Expand Up @@ -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);

Expand Down
4 changes: 3 additions & 1 deletion crates/noirc_frontend/src/hir_def/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -137,6 +137,8 @@ pub struct FuncMeta {

pub parameters: Parameters,

pub return_type: FunctionReturnType,

pub return_visibility: AbiVisibility,

pub return_distinctness: AbiDistinctness,
Expand Down
24 changes: 14 additions & 10 deletions crates/noirc_frontend/src/parser/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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::*;
Expand Down Expand Up @@ -258,17 +260,19 @@ fn lambda_return_type() -> impl NoirParser<UnresolvedType> {
.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),
),
})
}

Expand Down
1 change: 1 addition & 0 deletions noir_stdlib/Nargo.toml
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
[package]
name = "std"
type = "lib"
authors = [""]
compiler_version = "0.1"

Expand Down
1 change: 1 addition & 0 deletions noir_stdlib/src/lib.nr
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ mod unsafe;
mod collections;
mod compat;
mod option;
mod string;

// Oracle calls are required to be wrapped in an unconstrained function
// Thus, the only argument to the `println` oracle is expected to always be an ident
Expand Down
11 changes: 11 additions & 0 deletions noir_stdlib/src/string.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
use crate::collections::vec::Vec;
impl<N> str<N> {
/// Converts the given string into a byte array
#[builtin(str_as_bytes)]
fn as_bytes(_self: Self) -> [u8; N] { }

/// return a byte vector of the str content
fn as_bytes_vec(self: Self) -> Vec<u8> {
Vec::from_slice(self.as_bytes().as_slice())
}
}
5 changes: 4 additions & 1 deletion release-please-config.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,8 @@
}
]
}
}
},
"plugins": [
"sentence-case"
]
}

0 comments on commit 9db7912

Please sign in to comment.