Skip to content

Commit

Permalink
fix(frontend): Disallow signed numeric generics (noir-lang/noir#5572)
Browse files Browse the repository at this point in the history
chore: disable aztec-packages CI checks (noir-lang/noir#5566)
feat: LSP inlay parameter hints (noir-lang/noir#5553)
feat: Add `TraitDefinition::as_trait_constraint()` (noir-lang/noir#5541)
fix: Fix `uhashmap` test name (noir-lang/noir#5563)
fix: let unary traits work at comptime (noir-lang/noir#5507)
feat: Add a compile-time hash map type (noir-lang/noir#5543)
chore(docs): Docs for turbofish operator (noir-lang/noir#5555)
fix(frontend): Error for when impl is stricter than trait (noir-lang/noir#5343)
chore: filter warnings from elaborator in Aztec Macros (noir-lang/noir#5556)
fix: Don't panic when a macro fails to resolve (noir-lang/noir#5537)
fix(ssa): More robust array deduplication check (noir-lang/noir#5547)
fix: Fix occurs check (noir-lang/noir#5535)
fix: type_of for pointer types (noir-lang/noir#5536)
chore: Release Noir(0.32.0) (noir-lang/noir#5268)
fix: revert PR #5449 (noir-lang/noir#5548)
fix: never panic in LSP inlay hints (noir-lang/noir#5534)
feat: LSP document symbol (noir-lang/noir#5532)
feat: add comptime support for `modulus_*` compiler builtins (noir-lang/noir#5530)
chore: Remove unknown annotation warning (noir-lang/noir#5531)
feat: Add TraitConstraint type (noir-lang/noir#5499)
fix: Error on empty function bodies (noir-lang/noir#5519)
feat: LSP inlay hints for let and global (noir-lang/noir#5510)
chore: Remove dbg on find_func_with_name (noir-lang/noir#5526)
chore: Remove the remainder of legacy code (noir-lang/noir#5525)
chore: Remove `--use-legacy` and resolution code (noir-lang/noir#5248)
chore: switch to Noir Keccak implementation with variable size support (noir-lang/noir#5508)
chore: standardize experimental feature disclaimer across documentation (noir-lang/noir#5367)
  • Loading branch information
AztecBot committed Jul 20, 2024
2 parents 7a76d30 + 697795e commit f9c28e5
Show file tree
Hide file tree
Showing 28 changed files with 640 additions and 113 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
c6e5c4b304eb4e87ca519151f610e7e03b1a4fba
2b4853e71859f225acc123160e87c522212b16b5
4 changes: 2 additions & 2 deletions noir/noir-repo/.github/workflows/test-js-packages.yml
Original file line number Diff line number Diff line change
Expand Up @@ -517,8 +517,8 @@ jobs:
fail-fast: false
matrix:
project:
- { repo: AztecProtocol/aztec-nr, path: ./ }
- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts }
# - { repo: AztecProtocol/aztec-nr, path: ./ }
# - { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-contracts }
# Disabled as aztec-packages requires a setup-step in order to generate a `Nargo.toml`
#- { repo: AztecProtocol/aztec-packages, path: ./noir-projects/noir-protocol-circuits }
- { repo: zac-williamson/noir-edwards, path: ./, ref: 0016ce82cd58b6ebb0c43c271725590bcff4e755 }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -294,24 +294,7 @@ impl<F: AcirField> GeneratedAcir<F> {
outputs: (outputs[0], outputs[1], outputs[2]),
},
BlackBoxFunc::Keccak256 => {
let var_message_size = match inputs.to_vec().pop() {
Some(var_message_size) => var_message_size[0],
None => {
return Err(InternalError::MissingArg {
name: "".to_string(),
arg: "message_size".to_string(),
call_stack: self.call_stack.clone(),
});
}
};

BlackBoxFuncCall::Keccak256 {
inputs: inputs[0].clone(),
var_message_size,
outputs: outputs
.try_into()
.expect("Compiler should generate correct size outputs"),
}
unreachable!("unexpected BlackBox {}", func_name.to_string())
}
BlackBoxFunc::Keccakf1600 => BlackBoxFuncCall::Keccakf1600 {
inputs: inputs[0]
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
use iter_extended::vecmap;

use crate::ssa::ir::types::Type;

use super::{
basic_block::BasicBlockId,
dfg::{CallStack, InsertInstructionResult},
function::Function,
function::{Function, RuntimeType},
instruction::{Instruction, InstructionId},
value::ValueId,
};
Expand All @@ -20,7 +22,7 @@ pub(crate) struct FunctionInserter<'f> {
/// array unnecessarily. An extra bool is included as part of the key to
/// distinguish between Type::Array and Type::Slice, as both are valid
/// types for a Value::Array
const_arrays: HashMap<im::Vector<ValueId>, ValueId>,
const_arrays: HashMap<(im::Vector<ValueId>, bool), ValueId>,
}

impl<'f> FunctionInserter<'f> {
Expand All @@ -42,15 +44,26 @@ impl<'f> FunctionInserter<'f> {
let new_array: im::Vector<ValueId> =
array.iter().map(|id| self.resolve(*id)).collect();

if self.const_arrays.get(&new_array) == Some(&value) {
value
} else {
let new_array_clone = new_array.clone();
let new_id = self.function.dfg.make_array(new_array, typ);
self.values.insert(value, new_id);
self.const_arrays.insert(new_array_clone, new_id);
new_id
}
// Flag to determine the type of the value's array list
let is_array = matches!(typ, Type::Array { .. });
if let Some(fetched_value) =
self.const_arrays.get(&(new_array.clone(), is_array))
{
// Arrays in ACIR are immutable, but in Brillig arrays are copy-on-write
// so for function's with a Brillig runtime we make sure to check that value
// in our constants array map matches the resolved array value id.
if matches!(self.function.runtime(), RuntimeType::Acir(_)) {
return *fetched_value;
} else if *fetched_value == value {
return value;
}
};

let new_array_clone = new_array.clone();
let new_id = self.function.dfg.make_array(new_array, typ);
self.values.insert(value, new_id);
self.const_arrays.insert((new_array_clone, is_array), new_id);
new_id
}
_ => value,
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -489,7 +489,9 @@ fn simplify_black_box_func(
SimplifyResult::None
}
}
BlackBoxFunc::Keccak256 => SimplifyResult::None,
BlackBoxFunc::Keccak256 => {
unreachable!("Keccak256 should have been replaced by calls to Keccakf1600")
}
BlackBoxFunc::Poseidon2Permutation => SimplifyResult::None, //TODO(Guillaume)
BlackBoxFunc::EcdsaSecp256k1 => {
simplify_signature(dfg, arguments, acvm::blackbox_solver::ecdsa_secp256k1_verify)
Expand Down
12 changes: 5 additions & 7 deletions noir/noir-repo/compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,13 +323,11 @@ impl UnresolvedTypeExpression {

fn from_expr_helper(expr: Expression) -> Result<UnresolvedTypeExpression, Expression> {
match expr.kind {
ExpressionKind::Literal(Literal::Integer(int, sign)) => {
assert!(!sign, "Negative literal is not allowed here");
match int.try_to_u32() {
Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)),
None => Err(expr),
}
}
// TODO(https://github.com/noir-lang/noir/issues/5571): The `sign` bool from `Literal::Integer` should be used to construct the constant type expression.
ExpressionKind::Literal(Literal::Integer(int, _)) => match int.try_to_u32() {
Some(int) => Ok(UnresolvedTypeExpression::Constant(int, expr.span)),
None => Err(expr),
},
ExpressionKind::Variable(path, _) => Ok(UnresolvedTypeExpression::Variable(path)),
ExpressionKind::Prefix(prefix) if prefix.operator == UnaryOp::Minus => {
let lhs = Box::new(UnresolvedTypeExpression::Constant(0, expr.span));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -613,7 +613,12 @@ impl<'a> Interpreter<'a> {
UnaryOp::MutableReference => self.evaluate_no_dereference(prefix.rhs)?,
_ => self.evaluate(prefix.rhs)?,
};
self.evaluate_prefix_with_value(rhs, prefix.operator, id)

if self.interner.get_selected_impl_for_expression(id).is_some() {
self.evaluate_overloaded_prefix(prefix, rhs, id)
} else {
self.evaluate_prefix_with_value(rhs, prefix.operator, id)
}
}

fn evaluate_prefix_with_value(
Expand Down Expand Up @@ -953,6 +958,25 @@ impl<'a> Interpreter<'a> {
}
}

fn evaluate_overloaded_prefix(
&mut self,
prefix: HirPrefixExpression,
rhs: Value,
id: ExprId,
) -> IResult<Value> {
let method =
prefix.trait_method_id.expect("ice: expected prefix operator trait at this point");
let operator = prefix.operator;

let method_id = resolve_trait_method(self.interner, method, id)?;
let type_bindings = self.interner.get_instantiation_bindings(id).clone();

let rhs = (rhs, self.interner.expr_location(&prefix.rhs));

let location = self.interner.expr_location(&id);
self.call_function(method_id, vec![rhs], type_bindings, location)
}

/// Given the result of a `cmp` operation, convert it into the boolean result of the given operator.
/// - `<`: `ordering == Ordering::Less`
/// - `<=`: `ordering != Ordering::Greater`
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,14 @@ use std::{

use acvm::{AcirField, FieldElement};
use chumsky::Parser;
use iter_extended::vecmap;
use noirc_errors::Location;

use crate::{
ast::{IntegerBitSize, TraitBound},
hir::comptime::{errors::IResult, InterpreterError, Value},
macros_api::{NodeInterner, Signedness},
macros_api::{NodeInterner, Path, Signedness, UnresolvedTypeData},
node_interner::TraitId,
parser,
token::{SpannedToken, Token, Tokens},
QuotedType, Type,
Expand Down Expand Up @@ -42,6 +44,9 @@ pub(super) fn call_builtin(
"struct_def_generics" => struct_def_generics(interner, arguments, location),
"trait_constraint_eq" => trait_constraint_eq(interner, arguments, location),
"trait_constraint_hash" => trait_constraint_hash(interner, arguments, location),
"trait_def_as_trait_constraint" => {
trait_def_as_trait_constraint(interner, arguments, location)
}
"quoted_as_trait_constraint" => quoted_as_trait_constraint(interner, arguments, location),
_ => {
let item = format!("Comptime evaluation for builtin function {name}");
Expand Down Expand Up @@ -103,6 +108,16 @@ fn get_trait_constraint(value: Value, location: Location) -> IResult<TraitBound>
}
}

fn get_trait_def(value: Value, location: Location) -> IResult<TraitId> {
match value {
Value::TraitDefinition(id) => Ok(id),
value => {
let expected = Type::Quoted(QuotedType::TraitDefinition);
Err(InterpreterError::TypeMismatch { expected, value, location })
}
}
}

fn get_quoted(value: Value, location: Location) -> IResult<Rc<Tokens>> {
match value {
Value::Code(tokens) => Ok(tokens),
Expand Down Expand Up @@ -456,3 +471,24 @@ fn modulus_num_bits(
let bits = FieldElement::max_num_bits().into();
Ok(Value::U64(bits))
}

fn trait_def_as_trait_constraint(
interner: &mut NodeInterner,
mut arguments: Vec<(Value, Location)>,
location: Location,
) -> Result<Value, InterpreterError> {
check_argument_count(1, &arguments, location)?;

let trait_id = get_trait_def(arguments.pop().unwrap().0, location)?;
let the_trait = interner.get_trait(trait_id);

let trait_path = Path::from_ident(the_trait.name.clone());

let trait_generics = vecmap(&the_trait.generics, |generic| {
let name = Path::from_single(generic.name.as_ref().clone(), generic.span);
UnresolvedTypeData::Named(name, Vec::new(), false).with_span(generic.span)
});

let trait_id = Some(trait_id);
Ok(Value::TraitConstraint(TraitBound { trait_path, trait_id, trait_generics }))
}
4 changes: 4 additions & 0 deletions noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ pub enum ParserErrorReason {
InvalidBitSize(u32),
#[error("{0}")]
Lexer(LexerErrorKind),
// TODO(https://github.com/noir-lang/noir/issues/5571): This error can be removed once support is expanded for type-level integers.
// This error reason was added to prevent the panic outline in this issue: https://github.com/noir-lang/noir/issues/5552.
#[error("Only unsigned integers allowed for numeric generics")]
SignedNumericGeneric,
}

/// Represents a parsing error, or a parsing error in the making.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,17 @@ use super::{
parameter_name_recovery, parameter_recovery, parenthesized, parse_type, pattern,
self_parameter, where_clause, NoirParser,
};
use crate::ast::{
FunctionDefinition, FunctionReturnType, ItemVisibility, NoirFunction, Param, Visibility,
};
use crate::parser::spanned;
use crate::token::{Keyword, Token};
use crate::{
ast::{UnresolvedGeneric, UnresolvedGenerics},
ast::{
FunctionDefinition, FunctionReturnType, ItemVisibility, NoirFunction, Param, Visibility,
},
macros_api::UnresolvedTypeData,
parser::{ParserError, ParserErrorReason},
};
use crate::{
ast::{Signedness, UnresolvedGeneric, UnresolvedGenerics},
parser::labels::ParsingRuleLabel,
};

Expand Down Expand Up @@ -85,6 +89,19 @@ pub(super) fn numeric_generic() -> impl NoirParser<UnresolvedGeneric> {
.then_ignore(just(Token::Colon))
.then(parse_type())
.map(|(ident, typ)| UnresolvedGeneric::Numeric { ident, typ })
.validate(|generic, span, emit| {
if let UnresolvedGeneric::Numeric { typ, .. } = &generic {
if let UnresolvedTypeData::Integer(signedness, _) = typ.typ {
if matches!(signedness, Signedness::Signed) {
emit(ParserError::with_reason(
ParserErrorReason::SignedNumericGeneric,
span,
));
}
}
}
generic
})
}

pub(super) fn generic_type() -> impl NoirParser<UnresolvedGeneric> {
Expand Down Expand Up @@ -233,6 +250,7 @@ mod test {
"fn func_name<let T:>(y: T) {}",
"fn func_name<T:>(y: T) {}",
"fn func_name<T: u64>(y: T) {}",
"fn func_name<let N: i8>() {}",
],
);
}
Expand Down
3 changes: 3 additions & 0 deletions noir/noir-repo/noir_stdlib/src/embedded_curve_ops.nr
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,9 @@ fn multi_scalar_mul_array_return<let N: u32>(points: [EmbeddedCurvePoint; N], sc
#[foreign(multi_scalar_mul)]
pub(crate) fn multi_scalar_mul_slice(points: [EmbeddedCurvePoint], scalars: [EmbeddedCurveScalar]) -> [Field; 3] {}

#[foreign(multi_scalar_mul)]
pub(crate) fn multi_scalar_mul_slice(points: [EmbeddedCurvePoint], scalars: [EmbeddedCurveScalar]) -> [Field; 3] {}

// docs:start:fixed_base_scalar_mul
pub fn fixed_base_scalar_mul(scalar: EmbeddedCurveScalar) -> EmbeddedCurvePoint
// docs:end:fixed_base_scalar_mul
Expand Down
5 changes: 3 additions & 2 deletions noir/noir-repo/noir_stdlib/src/hash/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -106,11 +106,12 @@ pub fn hash_to_field(inputs: [Field]) -> Field {
sum
}

#[foreign(keccak256)]
// docs:start:keccak256
pub fn keccak256<let N: u32>(input: [u8; N], message_size: u32) -> [u8; 32]
// docs:end:keccak256
{}
{
crate::hash::keccak::keccak256(input, message_size)
}

#[foreign(poseidon2_permutation)]
pub fn poseidon2_permutation<let N: u32>(_input: [Field; N], _state_length: u32) -> [Field; N] {}
Expand Down
3 changes: 2 additions & 1 deletion noir/noir-repo/noir_stdlib/src/meta/mod.nr
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
mod type_def;
mod trait_constraint;
mod trait_def;
mod type_def;
mod quoted;

/// Calling unquote as a macro (via `unquote!(arg)`) will unquote
Expand Down
4 changes: 4 additions & 0 deletions noir/noir-repo/noir_stdlib/src/meta/trait_def.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
impl TraitDefinition {
#[builtin(trait_def_as_trait_constraint)]
fn as_trait_constraint(_self: Self) -> TraitConstraint {}
}
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use std::ops::Neg;

fn main() {
comptime
{
Expand All @@ -13,3 +15,22 @@ fn main() {
assert([1, 2] != array);
}
}

struct MyType {
value: i32,
}

comptime impl Neg for MyType {
comptime fn neg(self) -> Self {
self
}
}

fn neg_at_comptime() {
comptime
{
let value = MyType { value: 1 };
let _result = -value;
}
}

Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
[package]
name = "trait_as_constraint"
type = "bin"
authors = [""]
compiler_version = ">=0.31.0"

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
#[test_as_constraint]
trait Foo<T> {}

comptime fn test_as_constraint(t: TraitDefinition) {
let constraint = t.as_trait_constraint();
assert(constraint == constraint);
}

fn main() {}
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
[package]
name = "hashmap"
name = "uhashmap"
type = "bin"
authors = [""]

[dependencies]
[dependencies]
Loading

0 comments on commit f9c28e5

Please sign in to comment.