Skip to content

Commit

Permalink
Merge branch 'master' into tf/restructure-integration-tests
Browse files Browse the repository at this point in the history
  • Loading branch information
TomAFrench committed Nov 16, 2023
2 parents 394da91 + a22898d commit 543e3cc
Show file tree
Hide file tree
Showing 21 changed files with 280 additions and 92 deletions.
2 changes: 1 addition & 1 deletion compiler/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ pub(crate) fn display_instruction(
)
}
Instruction::RangeCheck { value, max_bit_size, .. } => {
write!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
writeln!(f, "range_check {} to {} bits", show(*value), *max_bit_size,)
}
}
}
16 changes: 16 additions & 0 deletions compiler/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -662,6 +662,22 @@ impl<'f> Context<'f> {
self.remember_store(address, value);
Instruction::Store { address, value }
}
Instruction::RangeCheck { value, max_bit_size, assert_message } => {
// Replace value with `value * predicate` to zero out value when predicate is inactive.

// Condition needs to be cast to argument type in order to multiply them together.
let argument_type = self.inserter.function.dfg.type_of_value(value);
let casted_condition = self.insert_instruction(
Instruction::Cast(condition, argument_type),
call_stack.clone(),
);

let value = self.insert_instruction(
Instruction::binary(BinaryOp::Mul, value, casted_condition),
call_stack.clone(),
);
Instruction::RangeCheck { value, max_bit_size, assert_message }
}
other => other,
}
} else {
Expand Down
27 changes: 1 addition & 26 deletions compiler/readme.md
Original file line number Diff line number Diff line change
@@ -1,32 +1,11 @@
# Structure

Below we briefly describe the purpose of each crate in this repository.

## acir - Abstract Circuit Intermediate Representation

This is the intermediate representation that Noir compiles down to. It is agnostic to any particular NP-Complete language.

## acvm - Abstract Circuit Virtual Machine

This is the virtual machine that runs ACIR. Given a proving system to power it, one can create and verify proofs, create smart contracts that verify proofs.
Below we briefly describe the purpose of each crate related to the compiler in this repository.

## fm - File Manager

This is the abstraction that the compiler uses to manage source files.

## lsp

This is the platform agnostic implementation of Noir's Language Server. It implements the various features supported, but doesn't bind to any particular transport. Binding to a transport must be done when consuming the crate.

## nargo

This is the default package manager used by Noir. One may draw similarities to Rusts' Cargo.

## noir_field

Since the DSL allows a user to create constants which can be as large as the field size, we must have a datatype that is able to store them. This is the purpose of the field crate.
One could alternatively use a BigInt library and store the modulus in a Context struct that gets passed to every compiler pass.

## noirc_abi

When consuming input from the user, a common ABI must be provided such that input provided in JSON/TOML can be converted to noir data types. This crate defines such an ABI.
Expand All @@ -42,7 +21,3 @@ This crate can be seen as the middle end. It is in charge of generating the ACIR
## noirc_frontend

This crate comprises of the first few compiler passes that together we denote as the compiler frontend (in order): lexing, parsing, name resolution, type checking, and monomorphization. If any of these passes error, the resulting monomorphized AST will not be passed to the middle-end (noirc_evaluator)

## wasm

This crate is used to compile the Noir compiler into wasm. This is useful in the context where one wants to compile noir programs in the web browser.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "conditional_underflow"
type = "bin"
authors = [""]

[dependencies]
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
x = "4"
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
// Regression test for https://github.com/noir-lang/noir/issues/3493
fn main(x: u4) {
if x == 10 {
x + 15;
}
if x == 9 {
x << 3;
}
if x == 8 {
x * 3;
}
if x == 7 {
x - 8;
}
}
1 change: 1 addition & 0 deletions tooling/nargo_fmt/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ config! {
error_on_lost_comment: bool, true, "Error if unable to get comments";
short_array_element_width_threshold: usize, 10, "Width threshold for an array element to be considered short";
array_width: usize, 100, "Maximum width of an array literal before falling back to vertical formatting";
fn_call_width: usize, 60, "Maximum width of the args of a function call before falling back to vertical formatting";
single_line_if_else_max_width: usize, 50, "Maximum line length for single line if-else expressions";
}

Expand Down
2 changes: 2 additions & 0 deletions tooling/nargo_fmt/src/rewrite.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
mod array;
mod infix;

pub(crate) use array::rewrite as array;
pub(crate) use infix::rewrite as infix;
85 changes: 85 additions & 0 deletions tooling/nargo_fmt/src/rewrite/array.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
use noirc_frontend::{hir::resolution::errors::Span, token::Token, Expression};

use crate::{
utils::{Expr, FindToken},
visitor::FmtVisitor,
};

pub(crate) fn rewrite(mut visitor: FmtVisitor, array: Vec<Expression>, array_span: Span) -> String {
let pattern: &[_] = &[' ', '\t'];

visitor.indent.block_indent(visitor.config);
let nested_indent = visitor.shape();

let indent_str = nested_indent.indent.to_string();

let mut last_position = array_span.start() + 1;
let end_position = array_span.end() - 1;

let mut items = array.into_iter().peekable();

let mut result = Vec::new();
while let Some(item) = items.next() {
let item_span = item.span;

let start: u32 = last_position;
let end = item_span.start();

let leading = visitor.slice(start..end).trim_matches(pattern);
let item = visitor.format_sub_expr(item);
let next_start = items.peek().map_or(end_position, |expr| expr.span.start());
let trailing = visitor.slice(item_span.end()..next_start);
let offset = trailing
.find_token(Token::Comma)
.map(|span| span.end() as usize)
.unwrap_or(trailing.len());
let trailing = trailing[..offset].trim_end_matches(',').trim_matches(pattern);
last_position = item_span.end() + offset as u32;

let (leading, _) = visitor.format_comment_in_block(leading, 0, false);
let (trailing, _) = visitor.format_comment_in_block(trailing, 0, false);

result.push(Expr { leading, value: item, trailing, different_line: false });
}

let slice = visitor.slice(last_position..end_position);
let (comment, _) = visitor.format_comment_in_block(slice, 0, false);
result.push(Expr {
leading: "".into(),
value: "".into(),
trailing: comment,
different_line: false,
});

visitor.indent.block_unindent(visitor.config);

let mut items_str = String::new();
let mut items = result.into_iter().peekable();
while let Some(next) = items.next() {
items_str.push_str(&next.leading);
if next.leading.contains('\n') && !next.value.is_empty() {
items_str.push_str(&indent_str);
}
items_str.push_str(&next.value);
items_str.push_str(&next.trailing);

if let Some(item) = items.peek() {
if !item.value.is_empty() {
items_str.push(',');
}

if !item.leading.contains('\n') && !next.value.is_empty() {
items_str.push(' ');
}
}
}

crate::visitor::expr::wrap_exprs(
"[",
"]",
items_str.trim().into(),
nested_indent,
visitor.shape(),
true,
)
}
4 changes: 4 additions & 0 deletions tooling/nargo_fmt/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,3 +244,7 @@ pub(crate) fn first_line_width(exprs: &str) -> usize {
pub(crate) fn is_single_line(s: &str) -> bool {
!s.chars().any(|c| c == '\n')
}

pub(crate) fn last_line_contains_single_line_comment(s: &str) -> bool {
s.lines().last().map_or(false, |line| line.contains("//"))
}
20 changes: 14 additions & 6 deletions tooling/nargo_fmt/src/visitor.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
mod expr;
pub(crate) mod expr;
mod item;
mod stmt;

Expand Down Expand Up @@ -53,7 +53,7 @@ impl<'me> FmtVisitor<'me> {
(span.start() + offset..span.end()).into()
}

fn shape(&self) -> Shape {
pub(crate) fn shape(&self) -> Shape {
Shape {
width: self.config.max_width.saturating_sub(self.indent.width()),
indent: self.indent,
Expand Down Expand Up @@ -163,7 +163,7 @@ impl<'me> FmtVisitor<'me> {
self.push_vertical_spaces(slice);
process_last_slice(self, "", slice);
} else {
let (result, last_end) = self.format_comment_in_block(slice, start);
let (result, last_end) = self.format_comment_in_block(slice, start, true);
if result.trim().is_empty() {
process_last_slice(self, slice, slice);
} else {
Expand All @@ -174,7 +174,12 @@ impl<'me> FmtVisitor<'me> {
}
}

fn format_comment_in_block(&mut self, slice: &str, start: u32) -> (String, u32) {
pub(crate) fn format_comment_in_block(
&mut self,
slice: &str,
start: u32,
fix_indent: bool,
) -> (String, u32) {
let mut result = String::new();
let mut last_end = 0;

Expand All @@ -185,7 +190,8 @@ impl<'me> FmtVisitor<'me> {
let diff = start;
let big_snippet = &self.source[..(span.start() + diff) as usize];
let last_char = big_snippet.chars().rev().find(|rev_c| ![' ', '\t'].contains(rev_c));
let fix_indent = last_char.map_or(true, |rev_c| ['{', '\n'].contains(&rev_c));
let fix_indent =
fix_indent && last_char.map_or(true, |rev_c| ['{', '\n'].contains(&rev_c));

if let Token::LineComment(_, _) | Token::BlockComment(_, _) = comment.into_token() {
let starts_with_newline = slice.starts_with('\n');
Expand All @@ -210,7 +216,9 @@ impl<'me> FmtVisitor<'me> {
(true, _) => {
result.push_str(&self.indent.to_string_with_newline());
}
_ => {}
(false, _) => {
result.push(' ');
}
};
}

Expand Down
34 changes: 26 additions & 8 deletions tooling/nargo_fmt/src/visitor/expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,13 @@ impl FmtVisitor<'_> {
self.span_before(call_expr.func.span.end()..span.end(), Token::LeftParen);

let callee = self.format_sub_expr(*call_expr.func);
let args = format_parens(self.fork(), false, call_expr.arguments, args_span);
let args = format_parens(
self.config.fn_call_width.into(),
self.fork(),
false,
call_expr.arguments,
args_span,
);

format!("{callee}{args}")
}
Expand All @@ -74,7 +80,13 @@ impl FmtVisitor<'_> {

let object = self.format_sub_expr(method_call_expr.object);
let method = method_call_expr.method_name.to_string();
let args = format_parens(self.fork(), false, method_call_expr.arguments, args_span);
let args = format_parens(
self.config.fn_call_width.into(),
self.fork(),
false,
method_call_expr.arguments,
args_span,
);

format!("{object}.{method}{args}")
}
Expand All @@ -92,7 +104,7 @@ impl FmtVisitor<'_> {
format!("{collection}{index}")
}
ExpressionKind::Tuple(exprs) => {
format_parens(self.fork(), exprs.len() == 1, exprs, span)
format_parens(None, self.fork(), exprs.len() == 1, exprs, span)
}
ExpressionKind::Literal(literal) => match literal {
Literal::Integer(_) | Literal::Bool(_) | Literal::Str(_) | Literal::FmtStr(_) => {
Expand All @@ -105,7 +117,7 @@ impl FmtVisitor<'_> {
format!("[{repeated}; {length}]")
}
Literal::Array(ArrayLiteral::Standard(exprs)) => {
format_brackets(self.fork(), false, exprs, span)
rewrite::array(self.fork(), exprs, span)
}
Literal::Unit => "()".to_string(),
},
Expand Down Expand Up @@ -370,7 +382,7 @@ fn format_expr_seq<T: Item>(

visitor.indent.block_unindent(visitor.config);

wrap_exprs(prefix, suffix, exprs, nested_indent, visitor.shape())
wrap_exprs(prefix, suffix, exprs, nested_indent, visitor.shape(), false)
}

fn format_brackets(
Expand All @@ -392,12 +404,14 @@ fn format_brackets(
}

fn format_parens(
max_width: Option<usize>,
visitor: FmtVisitor,
trailing_comma: bool,
exprs: Vec<Expression>,
span: Span,
) -> String {
format_expr_seq("(", ")", visitor, trailing_comma, exprs, span, Tactic::Horizontal)
let tactic = max_width.map(Tactic::LimitedHorizontalVertical).unwrap_or(Tactic::Horizontal);
format_expr_seq("(", ")", visitor, trailing_comma, exprs, span, tactic)
}

fn format_exprs(
Expand Down Expand Up @@ -483,16 +497,20 @@ fn format_exprs(
result
}

fn wrap_exprs(
pub(crate) fn wrap_exprs(
prefix: &str,
suffix: &str,
exprs: String,
nested_shape: Shape,
shape: Shape,
array: bool,
) -> String {
let first_line_width = first_line_width(&exprs);

if first_line_width <= shape.width {
let force_one_line =
if array { !exprs.contains('\n') } else { first_line_width <= shape.width };

if force_one_line {
let allow_trailing_newline = exprs
.lines()
.last()
Expand Down
20 changes: 18 additions & 2 deletions tooling/nargo_fmt/src/visitor/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,27 @@ use noirc_frontend::{
NoirFunction, ParsedModule,
};

use crate::utils::last_line_contains_single_line_comment;

impl super::FmtVisitor<'_> {
fn format_fn_before_block(&self, func: NoirFunction, start: u32) -> (String, bool) {
let slice = self.slice(start..func.span().start());
let force_brace_newline = slice.contains("//");
(slice.trim_end().to_string(), force_brace_newline)

let params_open = self
.span_before(func.name_ident().span().end()..func.span().start(), Token::LeftParen)
.start();

let last_span = if func.parameters().is_empty() {
params_open..func.span().start()
} else {
func.parameters().last().unwrap().1.span.unwrap().end()..func.span().start()
};

let params_end = self.span_after(last_span, Token::RightParen).start();

let maybe_comment = self.slice(params_end..func.span().start());

(slice.trim_end().to_string(), last_line_contains_single_line_comment(maybe_comment))
}

pub(crate) fn visit_file(&mut self, module: ParsedModule) {
Expand Down
Loading

0 comments on commit 543e3cc

Please sign in to comment.