Skip to content

Commit

Permalink
feat: Remove comptime and warn upon usage (noir-lang#2178)
Browse files Browse the repository at this point in the history
* Remove comptime

* Remove comptime from stdlib and tests

* Deprecate comptime keyword

* Add unknown loop bound error

* Fix end location for for loops

* Add Location to Jmp

* Shorten error message

* Fix test

* Only abort_on_error for acir functions

* Remove comptime from frontend test

* Remove extra parameter

* Remove comptime more thoroughly
  • Loading branch information
jfecher authored Aug 8, 2023
1 parent 34be264 commit 98d0de3
Show file tree
Hide file tree
Showing 43 changed files with 415 additions and 1,035 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ struct MyStruct<S> {
}

impl<S> MyStruct<S> {
fn insert(mut self: Self, index: comptime Field, elem: Field) -> Self {
fn insert(mut self: Self, index: Field, elem: Field) -> Self {
// Regression test for numeric generics on impls
assert(index as u64 < S as u64);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ fn main(x: Field, len3: [u8; 3], len4: [Field; 4]) {
assert(add_lens(len3, len4) == 7);
assert(nested_call(len4) == 5);

// std::array::len returns a comptime value
// std::array::len returns a compile-time known value
assert(len4[len3.len()] == 4);

// Regression for #1023, ensure .len still works after calling to_le_bytes on a witness.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,12 @@ fn main(x: u64) {
let two: u64 = 2;
let three: u64 = 3;

// comptime shifts on comptime values
// shifts on constant values
assert(two << 2 == 8);
assert((two << 3) / 8 == two);
assert((three >> 1) == 1);

// comptime shifts on runtime values
// shifts on runtime values
assert(x << 1 == 128);
assert(x >> 2 == 16);
}
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
fn main(x: u64, y: u64) {
// runtime shifts on comptime values
// runtime shifts on compile-time known values
assert(64 << y == 128);
assert(64 >> y == 32);

// runtime shifts on runtime values
assert(x << y == 128);
assert(x >> y == 32);
}
}

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

Binary file not shown.
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ fn main(a: [Field; M + N - N], b: [Field; 30 + N / 2], c : pub [Field; foo::MAGI
let mut y = 5;
let mut x = M;
for i in 0..N*N {
let M: comptime Field = 10;
let M: Field = 10;
x = M;

y = i;
Expand Down Expand Up @@ -87,8 +87,8 @@ mod mysubmodule {
assert(x | y == 1);
}

fn my_helper() -> comptime Field {
let N: comptime Field = 15; // Like in Rust, local variables override globals
fn my_helper() -> Field {
let N: Field = 15; // Like in Rust, local variables override globals
let x = N;
x
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
global NIBBLE_LENGTH: comptime Field = 16;
global NIBBLE_LENGTH: Field = 16;

fn compact_decode<N>(input: [u8; N], length: Field) -> ([u4; NIBBLE_LENGTH], Field)
{
Expand Down Expand Up @@ -87,4 +87,4 @@ fn main(x: [u8; 5], z: Field)
assert(enc_val1.0 == [0x94,0xb8,0x8f,0x61,0xe6,0xfb,0xda,0x83,0xfb,0xff,0xfa,0xbe,0x36,0x41,0x12,0x13,0x74,0x80,0x39,0x80,0x18,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00]);
assert(enc_val1.1 == 21);

}
}
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ impl<'block> BrilligBlock<'block> {
self.create_block_label_for_current_function(*else_destination),
);
}
TerminatorInstruction::Jmp { destination, arguments } => {
TerminatorInstruction::Jmp { destination, arguments, location: _ } => {
let target = &dfg[*destination];
for (src, dest) in arguments.iter().zip(target.parameters()) {
// Destination variable might have already been created by another block that jumps to this target
Expand Down
41 changes: 20 additions & 21 deletions crates/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@ pub enum RuntimeError {
UnInitialized { name: String, location: Option<Location> },
#[error("Integer sized {num_bits:?} is over the max supported size of {max_num_bits:?}")]
UnsupportedIntegerSize { num_bits: u32, max_num_bits: u32, location: Option<Location> },
#[error("Could not determine loop bound at compile-time")]
UnknownLoopBound { location: Option<Location> },
}

#[derive(Debug, PartialEq, Eq, Clone, Error)]
Expand All @@ -50,34 +52,36 @@ pub enum InternalError {
UnExpected { expected: String, found: String, location: Option<Location> },
}

impl From<RuntimeError> for FileDiagnostic {
fn from(error: RuntimeError) -> Self {
match error {
RuntimeError::InternalError(ref ice_error) => match ice_error {
impl RuntimeError {
fn location(&self) -> Option<Location> {
match self {
RuntimeError::InternalError(
InternalError::DegreeNotReduced { location }
| InternalError::EmptyArray { location }
| InternalError::General { location, .. }
| InternalError::MissingArg { location, .. }
| InternalError::NotAConstant { location, .. }
| InternalError::UndeclaredAcirVar { location }
| InternalError::UnExpected { location, .. } => {
let file_id = location.map(|loc| loc.file).unwrap();
FileDiagnostic { file_id, diagnostic: error.into() }
}
},
RuntimeError::FailedConstraint { location, .. }
| InternalError::UnExpected { location, .. },
)
| RuntimeError::FailedConstraint { location, .. }
| RuntimeError::IndexOutOfBounds { location, .. }
| RuntimeError::InvalidRangeConstraint { location, .. }
| RuntimeError::TypeConversion { location, .. }
| RuntimeError::UnInitialized { location, .. }
| RuntimeError::UnsupportedIntegerSize { location, .. } => {
let file_id = location.map(|loc| loc.file).unwrap();
FileDiagnostic { file_id, diagnostic: error.into() }
}
| RuntimeError::UnknownLoopBound { location, .. }
| RuntimeError::UnsupportedIntegerSize { location, .. } => *location,
}
}
}

impl From<RuntimeError> for FileDiagnostic {
fn from(error: RuntimeError) -> Self {
let file_id = error.location().map(|loc| loc.file).unwrap();
FileDiagnostic { file_id, diagnostic: error.into() }
}
}

impl From<RuntimeError> for Diagnostic {
fn from(error: RuntimeError) -> Diagnostic {
match error {
Expand All @@ -87,13 +91,8 @@ impl From<RuntimeError> for Diagnostic {
"".to_string(),
noirc_errors::Span::new(0..0)
),
RuntimeError::FailedConstraint { location, .. }
| RuntimeError::IndexOutOfBounds { location, .. }
| RuntimeError::InvalidRangeConstraint { location, .. }
| RuntimeError::TypeConversion { location, .. }
| RuntimeError::UnInitialized { location, .. }
| RuntimeError::UnsupportedIntegerSize { location, .. } => {
let span = if let Some(loc) = location { loc.span } else { noirc_errors::Span::new(0..0) };
_ => {
let span = if let Some(loc) = error.location() { loc.span } else { noirc_errors::Span::new(0..0) };
Diagnostic::simple_error("".to_owned(), error.to_string(), span)
}
}
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ pub(crate) fn optimize_into_acir(
ssa = ssa
.inline_functions()
.print(print_ssa_passes, "After Inlining:")
.unroll_loops()
.unroll_loops()?
.print(print_ssa_passes, "After Unrolling:")
.simplify_cfg()
.print(print_ssa_passes, "After Simplifying:")
Expand Down
1 change: 1 addition & 0 deletions crates/noirc_evaluator/src/ssa/ir/cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ mod tests {
func.dfg[block2_id].set_terminator(TerminatorInstruction::Jmp {
destination: ret_block_id,
arguments: vec![],
location: None,
});
func.dfg[block0_id].set_terminator(TerminatorInstruction::JmpIf {
condition: cond,
Expand Down
4 changes: 2 additions & 2 deletions crates/noirc_evaluator/src/ssa/ir/dfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -408,11 +408,11 @@ impl DataFlowGraph {
}

pub(crate) fn get_location(&self, id: &InstructionId) -> Option<Location> {
self.locations.get(id).cloned()
self.locations.get(id).copied()
}

pub(crate) fn get_value_location(&self, id: &ValueId) -> Option<Location> {
match &self.values[*id] {
match &self.values[self.resolve(*id)] {
Value::Instruction { instruction, .. } => self.get_location(instruction),
_ => None,
}
Expand Down
15 changes: 10 additions & 5 deletions crates/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use acvm::{acir::BlackBoxFunc, FieldElement};
use iter_extended::vecmap;
use noirc_errors::Location;
use num_bigint::BigUint;

use super::{
Expand Down Expand Up @@ -419,8 +420,10 @@ pub(crate) enum TerminatorInstruction {

/// Unconditional Jump
///
/// Jumps to specified `destination` with `arguments`
Jmp { destination: BasicBlockId, arguments: Vec<ValueId> },
/// Jumps to specified `destination` with `arguments`.
/// The optional Location here is expected to be used to issue an error when the start range of
/// a for loop cannot be deduced at compile-time.
Jmp { destination: BasicBlockId, arguments: Vec<ValueId>, location: Option<Location> },

/// Return from the current function with the given return values.
///
Expand All @@ -445,9 +448,11 @@ impl TerminatorInstruction {
then_destination: *then_destination,
else_destination: *else_destination,
},
Jmp { destination, arguments } => {
Jmp { destination: *destination, arguments: vecmap(arguments, |value| f(*value)) }
}
Jmp { destination, arguments, location } => Jmp {
destination: *destination,
arguments: vecmap(arguments, |value| f(*value)),
location: *location,
},
Return { return_values } => {
Return { return_values: vecmap(return_values, |value| f(*value)) }
}
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/ir/printer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ pub(crate) fn display_terminator(
f: &mut Formatter,
) -> Result {
match terminator {
Some(TerminatorInstruction::Jmp { destination, arguments }) => {
Some(TerminatorInstruction::Jmp { destination, arguments, location: _ }) => {
writeln!(f, " jmp {}({})", destination, value_list(function, arguments))
}
Some(TerminatorInstruction::JmpIf { condition, then_destination, else_destination }) => {
Expand Down
2 changes: 1 addition & 1 deletion crates/noirc_evaluator/src/ssa/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -285,7 +285,7 @@ impl<'f> Context<'f> {
let end = self.branch_ends[&block];
self.inline_branch_end(end, then_branch, else_branch)
}
TerminatorInstruction::Jmp { destination, arguments } => {
TerminatorInstruction::Jmp { destination, arguments, location: _ } => {
if let Some((end_block, _)) = self.conditions.last() {
if destination == end_block {
return block;
Expand Down
8 changes: 6 additions & 2 deletions crates/noirc_evaluator/src/ssa/opt/inlining.rs
Original file line number Diff line number Diff line change
Expand Up @@ -433,10 +433,14 @@ impl<'function> PerFunctionContext<'function> {
block_queue: &mut Vec<BasicBlockId>,
) -> Option<(BasicBlockId, Vec<ValueId>)> {
match self.source_function.dfg[block_id].unwrap_terminator() {
TerminatorInstruction::Jmp { destination, arguments } => {
TerminatorInstruction::Jmp { destination, arguments, location } => {
let destination = self.translate_block(*destination, block_queue);
let arguments = vecmap(arguments, |arg| self.translate_value(*arg));
self.context.builder.terminate_with_jmp(destination, arguments);
self.context.builder.terminate_with_jmp_with_location(
destination,
arguments,
*location,
);
None
}
TerminatorInstruction::JmpIf { condition, then_destination, else_destination } => {
Expand Down
3 changes: 2 additions & 1 deletion crates/noirc_evaluator/src/ssa/opt/simplify_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,8 @@ fn check_for_constant_jmpif(
let destination =
if constant.is_zero() { *else_destination } else { *then_destination };

let jmp = TerminatorInstruction::Jmp { destination, arguments: Vec::new() };
let arguments = Vec::new();
let jmp = TerminatorInstruction::Jmp { destination, arguments, location: None };
function.dfg[block].set_terminator(jmp);
cfg.recompute_block(function, block);
}
Expand Down
Loading

0 comments on commit 98d0de3

Please sign in to comment.