Skip to content

Commit e903406

Browse files
committed
fix: prevent panic within remove_possibly_mutated_cached_make_arrays (noir-lang/noir#7264)
chore: early check type equality in try_unify (noir-lang/noir#7263) feat(LSP): suggest enum variants without parameters (noir-lang/noir#7261) feat(experimental): Parse match expressions (noir-lang/noir#7243) feat(experimental): Implement zeroed for enums (noir-lang/noir#7252) fix(ssa): Only attempt to inline constant Brillig calls for entry points (noir-lang/noir#7260) fix: Add missing `is_empty` check for enums (noir-lang/noir#7257) feat(experimental): Implement enum tag constants (noir-lang/noir#7183) fix(unrolling): Fetch original bytecode size from the original function (noir-lang/noir#7253) fix(ssa): Use number of SSA instructions for the Brillig unrolling bytecode size limit (noir-lang/noir#7242) feat: Sync from aztec-packages (noir-lang/noir#7241) chore: bump gates diff (noir-lang/noir#7245) feat: simplify subtraction from self to return zero (noir-lang/noir#7189) feat: allow specifying multiple patterns in nargo test (noir-lang/noir#7186) fix: Avoid type error when calling something with a type alias of a function (noir-lang/noir#7239) feat: Allow resolved types in constructors (noir-lang/noir#7223) chore: clarify to_radix docs examples (noir-lang/noir#7230) chore: fix struct example (noir-lang/noir#7198) feat(optimization): Add purity analysis to SSA (noir-lang/noir#7197) chore: start tracking time to run critical library tests (noir-lang/noir#7221) chore: Rework defunctionalize pass to not rely on DFG bugs (noir-lang/noir#7222) fix(brillig): Globals entry point reachability analysis (noir-lang/noir#7188) chore: update docs to use devcontainer feature (noir-lang/noir#7206) chore(ci): Add test for global vars entry points regression (noir-lang/noir#7209) chore(ssa): Flip the SSA Brillig constraint check to off by default (noir-lang/noir#7211) chore(docs): moving references to noir-starter to awesome-noir (noir-lang/noir#7203) chore: build docs in the merge queue (noir-lang/noir#7218) fix: correct reversed callstacks (noir-lang/noir#7212) chore: exclude dependency fetching time from benchmarks (noir-lang/noir#7210) feat(experimental): Support enums in comptime code (noir-lang/noir#7194)
2 parents 6a98665 + 4d5f9e3 commit e903406

File tree

21 files changed

+583
-141
lines changed

21 files changed

+583
-141
lines changed

.noir-sync-commit

+1-1
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
a9e985064303b0843cbf68fb5a9d41f9ade1e30d
1+
130d99125a09110a3ee3e877d88d83b5aa37f369

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen.rs

+5-1
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,11 @@ pub(crate) fn gen_brillig_for(
5858
brillig: &Brillig,
5959
) -> Result<GeneratedBrillig<FieldElement>, InternalError> {
6060
// Create the entry point artifact
61-
let globals_memory_size = brillig.globals_memory_size.get(&func.id()).copied().unwrap_or(0);
61+
let globals_memory_size = brillig
62+
.globals_memory_size
63+
.get(&func.id())
64+
.copied()
65+
.expect("Should have the globals memory size specified for an entry point");
6266
let mut entry_point = BrilligContext::new_entry_point_artifact(
6367
arguments,
6468
FunctionContext::return_values(func),

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_globals.rs

+8-12
Original file line numberDiff line numberDiff line change
@@ -182,20 +182,11 @@ impl BrilligGlobals {
182182
&self,
183183
brillig_function_id: FunctionId,
184184
) -> SsaToBrilligGlobals {
185-
let mut globals_allocations = HashMap::default();
186-
187-
// First check whether the `brillig_function_id` is itself an entry point,
188-
// if so we can fetch the global allocations directly from `self.entry_point_globals_map`.
189-
if let Some(globals) = self.entry_point_globals_map.get(&brillig_function_id) {
190-
globals_allocations.extend(globals);
191-
return globals_allocations;
192-
}
193-
194-
// If the Brillig function we are compiling is not an entry point, we should search
195-
// for the entry point which triggers the given function.
196185
let entry_points = self.inner_call_to_entry_point.get(&brillig_function_id);
186+
187+
let mut globals_allocations = HashMap::default();
197188
if let Some(entry_points) = entry_points {
198-
// A Brillig function can be used by multiple entry points. Fetch both globals allocations
189+
// A Brillig function is used by multiple entry points. Fetch both globals allocations
199190
// in case one is used by the internal call.
200191
let entry_point_allocations = entry_points
201192
.iter()
@@ -204,6 +195,11 @@ impl BrilligGlobals {
204195
for map in entry_point_allocations {
205196
globals_allocations.extend(map);
206197
}
198+
} else if let Some(globals) = self.entry_point_globals_map.get(&brillig_function_id) {
199+
// If there is no mapping from an inner call to an entry point, that means `brillig_function_id`
200+
// is itself an entry point and we can fetch the global allocations directly from `self.entry_point_globals_map`.
201+
// vec![globals]
202+
globals_allocations.extend(globals);
207203
} else {
208204
unreachable!(
209205
"ICE: Expected global allocation to be set for function {brillig_function_id}"

noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs

+124
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,49 @@ impl Ssa {
9898
function.constant_fold(false, brillig_info);
9999
}
100100

101+
// It could happen that we inlined all calls to a given brillig function.
102+
// In that case it's unused so we can remove it. This is what we check next.
103+
self.remove_unused_brillig_functions(brillig_functions)
104+
}
105+
106+
fn remove_unused_brillig_functions(
107+
mut self,
108+
mut brillig_functions: BTreeMap<FunctionId, Function>,
109+
) -> Ssa {
110+
// Remove from the above map functions that are called
111+
for function in self.functions.values() {
112+
for block_id in function.reachable_blocks() {
113+
for instruction_id in function.dfg[block_id].instructions() {
114+
let instruction = &function.dfg[*instruction_id];
115+
let Instruction::Call { func: func_id, arguments: _ } = instruction else {
116+
continue;
117+
};
118+
119+
let func_value = &function.dfg[*func_id];
120+
let Value::Function(func_id) = func_value else { continue };
121+
122+
if function.runtime().is_acir() {
123+
brillig_functions.remove(func_id);
124+
}
125+
}
126+
}
127+
}
128+
129+
// The ones that remain are never called: let's remove them.
130+
for (func_id, func) in &brillig_functions {
131+
// We never want to remove the main function (it could be `unconstrained` or it
132+
// could have been turned into brillig if `--force-brillig` was given).
133+
// We also don't want to remove entry points.
134+
let runtime = func.runtime();
135+
if self.main_id == *func_id
136+
|| (runtime.is_entry_point() && matches!(runtime, RuntimeType::Acir(_)))
137+
{
138+
continue;
139+
}
140+
141+
self.functions.remove(func_id);
142+
}
143+
101144
self
102145
}
103146
}
@@ -682,6 +725,11 @@ impl<'brillig> Context<'brillig> {
682725

683726
// Should we consider calls to slice_push_back and similar to be mutating operations as well?
684727
if let Store { value: array, .. } | ArraySet { array, .. } = instruction {
728+
if function.dfg.is_global(*array) {
729+
// Early return as we expect globals to be immutable.
730+
return;
731+
};
732+
685733
let instruction = match &function.dfg[*array] {
686734
Value::Instruction { instruction, .. } => &function.dfg[*instruction],
687735
_ => return,
@@ -1533,6 +1581,82 @@ mod test {
15331581
assert_normalized_ssa_equals(ssa, expected);
15341582
}
15351583

1584+
#[test]
1585+
fn inlines_brillig_call_with_entry_point_globals() {
1586+
let src = "
1587+
g0 = Field 2
1588+
1589+
acir(inline) fn main f0 {
1590+
b0():
1591+
v1 = call f1() -> Field
1592+
return v1
1593+
}
1594+
1595+
brillig(inline) fn one f1 {
1596+
b0():
1597+
v1 = add g0, Field 3
1598+
return v1
1599+
}
1600+
";
1601+
let ssa = Ssa::from_str(src).unwrap();
1602+
let mut ssa = ssa.dead_instruction_elimination();
1603+
let used_globals_map = std::mem::take(&mut ssa.used_globals);
1604+
let brillig = ssa.to_brillig_with_globals(false, used_globals_map);
1605+
1606+
let expected = "
1607+
g0 = Field 2
1608+
1609+
acir(inline) fn main f0 {
1610+
b0():
1611+
return Field 5
1612+
}
1613+
";
1614+
1615+
let ssa = ssa.fold_constants_with_brillig(&brillig);
1616+
assert_normalized_ssa_equals(ssa, expected);
1617+
}
1618+
1619+
#[test]
1620+
fn inlines_brillig_call_with_non_entry_point_globals() {
1621+
let src = "
1622+
g0 = Field 2
1623+
1624+
acir(inline) fn main f0 {
1625+
b0():
1626+
v1 = call f1() -> Field
1627+
return v1
1628+
}
1629+
1630+
brillig(inline) fn entry_point f1 {
1631+
b0():
1632+
v1 = call f2() -> Field
1633+
return v1
1634+
}
1635+
1636+
brillig(inline) fn one f2 {
1637+
b0():
1638+
v1 = add g0, Field 3
1639+
return v1
1640+
}
1641+
";
1642+
let ssa = Ssa::from_str(src).unwrap();
1643+
let mut ssa = ssa.dead_instruction_elimination();
1644+
let used_globals_map = std::mem::take(&mut ssa.used_globals);
1645+
let brillig = ssa.to_brillig_with_globals(false, used_globals_map);
1646+
1647+
let expected = "
1648+
g0 = Field 2
1649+
1650+
acir(inline) fn main f0 {
1651+
b0():
1652+
return Field 5
1653+
}
1654+
";
1655+
1656+
let ssa = ssa.fold_constants_with_brillig(&brillig);
1657+
assert_normalized_ssa_equals(ssa, expected);
1658+
}
1659+
15361660
#[test]
15371661
fn does_not_use_cached_constrain_in_block_that_is_not_dominated() {
15381662
let src = "

noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs

+18
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ pub enum ExpressionKind {
3131
Cast(Box<CastExpression>),
3232
Infix(Box<InfixExpression>),
3333
If(Box<IfExpression>),
34+
Match(Box<MatchExpression>),
3435
Variable(Path),
3536
Tuple(Vec<Expression>),
3637
Lambda(Box<Lambda>),
@@ -465,6 +466,12 @@ pub struct IfExpression {
465466
pub alternative: Option<Expression>,
466467
}
467468

469+
#[derive(Debug, PartialEq, Eq, Clone)]
470+
pub struct MatchExpression {
471+
pub expression: Expression,
472+
pub rules: Vec<(/*pattern*/ Expression, /*branch*/ Expression)>,
473+
}
474+
468475
#[derive(Debug, PartialEq, Eq, Clone)]
469476
pub struct Lambda {
470477
pub parameters: Vec<(Pattern, UnresolvedType)>,
@@ -612,6 +619,7 @@ impl Display for ExpressionKind {
612619
Cast(cast) => cast.fmt(f),
613620
Infix(infix) => infix.fmt(f),
614621
If(if_expr) => if_expr.fmt(f),
622+
Match(match_expr) => match_expr.fmt(f),
615623
Variable(path) => path.fmt(f),
616624
Constructor(constructor) => constructor.fmt(f),
617625
MemberAccess(access) => access.fmt(f),
@@ -790,6 +798,16 @@ impl Display for IfExpression {
790798
}
791799
}
792800

801+
impl Display for MatchExpression {
802+
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
803+
writeln!(f, "match {} {{", self.expression)?;
804+
for (pattern, branch) in &self.rules {
805+
writeln!(f, " {pattern} -> {branch},")?;
806+
}
807+
write!(f, "}}")
808+
}
809+
}
810+
793811
impl Display for Lambda {
794812
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
795813
let parameters = vecmap(&self.parameters, |(name, r#type)| format!("{name}: {type}"));

noir/noir-repo/compiler/noirc_frontend/src/ast/visitor.rs

+24-1
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use crate::{
2222

2323
use super::{
2424
ForBounds, FunctionReturnType, GenericTypeArgs, IntegerBitSize, ItemVisibility,
25-
NoirEnumeration, Pattern, Signedness, TraitBound, TraitImplItemKind, TypePath,
25+
MatchExpression, NoirEnumeration, Pattern, Signedness, TraitBound, TraitImplItemKind, TypePath,
2626
UnresolvedGenerics, UnresolvedTraitConstraint, UnresolvedType, UnresolvedTypeData,
2727
UnresolvedTypeExpression,
2828
};
@@ -222,6 +222,10 @@ pub trait Visitor {
222222
true
223223
}
224224

225+
fn visit_match_expression(&mut self, _: &MatchExpression, _: Span) -> bool {
226+
true
227+
}
228+
225229
fn visit_tuple(&mut self, _: &[Expression], _: Span) -> bool {
226230
true
227231
}
@@ -866,6 +870,9 @@ impl Expression {
866870
ExpressionKind::If(if_expression) => {
867871
if_expression.accept(self.span, visitor);
868872
}
873+
ExpressionKind::Match(match_expression) => {
874+
match_expression.accept(self.span, visitor);
875+
}
869876
ExpressionKind::Tuple(expressions) => {
870877
if visitor.visit_tuple(expressions, self.span) {
871878
visit_expressions(expressions, visitor);
@@ -1073,6 +1080,22 @@ impl IfExpression {
10731080
}
10741081
}
10751082

1083+
impl MatchExpression {
1084+
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
1085+
if visitor.visit_match_expression(self, span) {
1086+
self.accept_children(visitor);
1087+
}
1088+
}
1089+
1090+
pub fn accept_children(&self, visitor: &mut impl Visitor) {
1091+
self.expression.accept(visitor);
1092+
for (pattern, branch) in &self.rules {
1093+
pattern.accept(visitor);
1094+
branch.accept(visitor);
1095+
}
1096+
}
1097+
}
1098+
10761099
impl Lambda {
10771100
pub fn accept(&self, span: Span, visitor: &mut impl Visitor) {
10781101
if visitor.visit_lambda(self, span) {

noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs

+8-3
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,9 @@ use crate::{
77
ast::{
88
ArrayLiteral, BlockExpression, CallExpression, CastExpression, ConstructorExpression,
99
Expression, ExpressionKind, Ident, IfExpression, IndexExpression, InfixExpression,
10-
ItemVisibility, Lambda, Literal, MemberAccessExpression, MethodCallExpression, Path,
11-
PathSegment, PrefixExpression, StatementKind, UnaryOp, UnresolvedTypeData,
12-
UnresolvedTypeExpression,
10+
ItemVisibility, Lambda, Literal, MatchExpression, MemberAccessExpression,
11+
MethodCallExpression, Path, PathSegment, PrefixExpression, StatementKind, UnaryOp,
12+
UnresolvedTypeData, UnresolvedTypeExpression,
1313
},
1414
hir::{
1515
comptime::{self, InterpreterError},
@@ -51,6 +51,7 @@ impl<'context> Elaborator<'context> {
5151
ExpressionKind::Cast(cast) => self.elaborate_cast(*cast, expr.span),
5252
ExpressionKind::Infix(infix) => return self.elaborate_infix(*infix, expr.span),
5353
ExpressionKind::If(if_) => self.elaborate_if(*if_),
54+
ExpressionKind::Match(match_) => self.elaborate_match(*match_),
5455
ExpressionKind::Variable(variable) => return self.elaborate_variable(variable),
5556
ExpressionKind::Tuple(tuple) => self.elaborate_tuple(tuple),
5657
ExpressionKind::Lambda(lambda) => self.elaborate_lambda(*lambda, None),
@@ -926,6 +927,10 @@ impl<'context> Elaborator<'context> {
926927
(HirExpression::If(if_expr), ret_type)
927928
}
928929

930+
fn elaborate_match(&mut self, _match_expr: MatchExpression) -> (HirExpression, Type) {
931+
(HirExpression::Error, Type::Error)
932+
}
933+
929934
fn elaborate_tuple(&mut self, tuple: Vec<Expression>) -> (HirExpression, Type) {
930935
let mut element_ids = Vec::with_capacity(tuple.len());
931936
let mut element_types = Vec::with_capacity(tuple.len());

noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/display.rs

+12-3
Original file line numberDiff line numberDiff line change
@@ -8,9 +8,9 @@ use crate::{
88
ArrayLiteral, AsTraitPath, AssignStatement, BlockExpression, CallExpression,
99
CastExpression, ConstrainStatement, ConstructorExpression, Expression, ExpressionKind,
1010
ForBounds, ForLoopStatement, ForRange, GenericTypeArgs, IfExpression, IndexExpression,
11-
InfixExpression, LValue, Lambda, LetStatement, Literal, MemberAccessExpression,
12-
MethodCallExpression, Pattern, PrefixExpression, Statement, StatementKind, UnresolvedType,
13-
UnresolvedTypeData,
11+
InfixExpression, LValue, Lambda, LetStatement, Literal, MatchExpression,
12+
MemberAccessExpression, MethodCallExpression, Pattern, PrefixExpression, Statement,
13+
StatementKind, UnresolvedType, UnresolvedTypeData,
1414
},
1515
hir_def::traits::TraitConstraint,
1616
node_interner::{InternedStatementKind, NodeInterner},
@@ -241,6 +241,7 @@ impl<'interner> TokenPrettyPrinter<'interner> {
241241
| Token::GreaterEqual
242242
| Token::Equal
243243
| Token::NotEqual
244+
| Token::FatArrow
244245
| Token::Arrow => write!(f, " {token} "),
245246
Token::Assign => {
246247
if last_was_op {
@@ -602,6 +603,14 @@ fn remove_interned_in_expression_kind(
602603
.alternative
603604
.map(|alternative| remove_interned_in_expression(interner, alternative)),
604605
})),
606+
ExpressionKind::Match(match_expr) => ExpressionKind::Match(Box::new(MatchExpression {
607+
expression: remove_interned_in_expression(interner, match_expr.expression),
608+
rules: vecmap(match_expr.rules, |(pattern, branch)| {
609+
let pattern = remove_interned_in_expression(interner, pattern);
610+
let branch = remove_interned_in_expression(interner, branch);
611+
(pattern, branch)
612+
}),
613+
})),
605614
ExpressionKind::Variable(_) => expr,
606615
ExpressionKind::Tuple(expressions) => ExpressionKind::Tuple(vecmap(expressions, |expr| {
607616
remove_interned_in_expression(interner, expr)

0 commit comments

Comments
 (0)