Skip to content

Commit

Permalink
feat!: remove concept of noir fallbacks for foreign functions (#1371)
Browse files Browse the repository at this point in the history
  • Loading branch information
TomAFrench authored May 18, 2023
1 parent 5d1efd5 commit dbec6f2
Show file tree
Hide file tree
Showing 9 changed files with 9 additions and 79 deletions.
7 changes: 4 additions & 3 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,9 @@ pub use program::CompiledProgram;
pub struct Driver {
context: Context,
language: Language,
// We retain this as we need to pass this into `create_circuit` once signature is updated to allow.
#[allow(dead_code)]
is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>,
}

#[derive(Args, Clone, Debug, Serialize, Deserialize)]
Expand Down Expand Up @@ -68,9 +71,7 @@ impl Default for CompileOptions {

impl Driver {
pub fn new(language: &Language, is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>) -> Self {
let mut driver = Driver { context: Context::default(), language: language.clone() };
driver.context.def_interner.set_opcode_support(is_opcode_supported);
driver
Driver { context: Context::default(), language: language.clone(), is_opcode_supported }
}

// This is here for backwards compatibility
Expand Down
1 change: 0 additions & 1 deletion crates/noirc_frontend/src/ast/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ impl From<FunctionDefinition> for NoirFunction {
let kind = match fd.attribute {
Some(Attribute::Builtin(_)) => FunctionKind::Builtin,
Some(Attribute::Foreign(_)) => FunctionKind::LowLevel,
Some(Attribute::Alternative(_)) => FunctionKind::Normal,
Some(Attribute::Test) => FunctionKind::Normal,
None => FunctionKind::Normal,
};
Expand Down
13 changes: 3 additions & 10 deletions crates/noirc_frontend/src/hir/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ pub mod type_check;

use crate::graph::{CrateGraph, CrateId};
use crate::node_interner::NodeInterner;
use acvm::acir::circuit::Opcode;
use def_map::CrateDefMap;
use fm::FileManager;
use std::collections::HashMap;
Expand All @@ -29,20 +28,14 @@ pub struct Context {
pub type StorageSlot = u32;

impl Context {
pub fn new(
file_manager: FileManager,
crate_graph: CrateGraph,
is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>,
) -> Context {
let mut ctx = Context {
pub fn new(file_manager: FileManager, crate_graph: CrateGraph) -> Context {
Context {
def_interner: NodeInterner::default(),
def_maps: HashMap::new(),
crate_graph,
file_manager,
storage_slots: HashMap::new(),
};
ctx.def_interner.set_opcode_support(is_opcode_supported);
ctx
}
}

/// Returns the CrateDefMap for a given CrateId.
Expand Down
18 changes: 1 addition & 17 deletions crates/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1149,23 +1149,7 @@ impl<'a> Resolver<'a> {
let span = path.span();
let id = self.resolve_path(path)?;

if let Some(mut function) = TryFromModuleDefId::try_from(id) {
// Check if this is an unsupported low level opcode. If so, replace it with
// an alternative in the stdlib.
if let Some(meta) = self.interner.try_function_meta(&function) {
if meta.kind == crate::FunctionKind::LowLevel {
let attribute = meta.attributes.expect("all low level functions must contain an attribute which contains the opcode which it links to");
let opcode = attribute.foreign().expect(
"ice: function marked as foreign, but attribute kind does not match this",
);
if !self.interner.foreign(&opcode) {
if let Some(new_id) = self.interner.get_alt(opcode) {
function = new_id;
}
}
}
}

if let Some(function) = TryFromModuleDefId::try_from(id) {
return Ok(self.interner.function_definition_id(function));
}

Expand Down
4 changes: 0 additions & 4 deletions crates/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -324,7 +324,6 @@ impl IntType {
pub enum Attribute {
Foreign(String),
Builtin(String),
Alternative(String),
Test,
}

Expand All @@ -333,7 +332,6 @@ impl fmt::Display for Attribute {
match *self {
Attribute::Foreign(ref k) => write!(f, "#[foreign({k})]"),
Attribute::Builtin(ref k) => write!(f, "#[builtin({k})]"),
Attribute::Alternative(ref k) => write!(f, "#[alternative({k})]"),
Attribute::Test => write!(f, "#[test]"),
}
}
Expand Down Expand Up @@ -365,7 +363,6 @@ impl Attribute {
let tok = match attribute_type {
"foreign" => Token::Attribute(Attribute::Foreign(attribute_name.to_string())),
"builtin" => Token::Attribute(Attribute::Builtin(attribute_name.to_string())),
"alternative" => Token::Attribute(Attribute::Alternative(attribute_name.to_string())),
_ => {
return Err(LexerErrorKind::MalformedFuncAttribute { span, found: word.to_owned() })
}
Expand Down Expand Up @@ -401,7 +398,6 @@ impl AsRef<str> for Attribute {
match self {
Attribute::Foreign(string) => string,
Attribute::Builtin(string) => string,
Attribute::Alternative(string) => string,
Attribute::Test => "",
}
}
Expand Down
7 changes: 1 addition & 6 deletions crates/noirc_frontend/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,7 @@ fn main() {
let crate_id = crate_graph.add_crate_root(CrateType::Library, root_file_id);

// initiate context with file manager and crate graph
let mut context = Context::new(
fm,
crate_graph,
#[allow(deprecated)]
Box::new(acvm::default_is_opcode_supported(acvm::Language::R1CS)),
);
let mut context = Context::new(fm, crate_graph);

// Now create the CrateDefMap
// This is preamble for analysis
Expand Down
36 changes: 0 additions & 36 deletions crates/noirc_frontend/src/node_interner.rs
Original file line number Diff line number Diff line change
@@ -1,8 +1,5 @@
use std::collections::{BTreeMap, HashMap};

use acvm::acir::circuit::opcodes::BlackBoxFuncCall;
use acvm::acir::circuit::Opcode;
use acvm::Language;
use arena::{Arena, Index};
use fm::FileId;
use iter_extended::vecmap;
Expand Down Expand Up @@ -69,9 +66,6 @@ pub struct NodeInterner {

next_type_variable_id: usize,

//used for fallback mechanism
is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>,

delayed_type_checks: Vec<TypeCheckFn>,

/// A map from a struct type and method name to a function id for the method.
Expand Down Expand Up @@ -258,8 +252,6 @@ impl Default for NodeInterner {
field_indices: HashMap::new(),
next_type_variable_id: 0,
globals: HashMap::new(),
#[allow(deprecated)]
is_opcode_supported: Box::new(acvm::default_is_opcode_supported(Language::R1CS)),
delayed_type_checks: vec![],
struct_methods: HashMap::new(),
primitive_methods: HashMap::new(),
Expand Down Expand Up @@ -396,17 +388,6 @@ impl NodeInterner {
self.func_meta.insert(func_id, func_data);
}

pub fn get_alt(&self, opcode: String) -> Option<FuncId> {
for (func_id, meta) in &self.func_meta {
if let Some(crate::token::Attribute::Alternative(name)) = &meta.attributes {
if *name == opcode {
return Some(*func_id);
}
}
}
None
}

pub fn push_definition(
&mut self,
name: String,
Expand Down Expand Up @@ -580,23 +561,6 @@ impl NodeInterner {
self.function_definition_ids[&function]
}

pub fn set_opcode_support(&mut self, is_opcode_supported: Box<dyn Fn(&Opcode) -> bool>) {
self.is_opcode_supported = is_opcode_supported;
}

#[allow(deprecated)]
pub fn foreign(&self, opcode: &str) -> bool {
let black_box_func = match acvm::acir::BlackBoxFunc::lookup(opcode) {
Some(black_box_func) => black_box_func,
None => return false,
};
(self.is_opcode_supported)(&Opcode::BlackBoxFuncCall(BlackBoxFuncCall {
name: black_box_func,
inputs: Vec::new(),
outputs: Vec::new(),
}))
}

pub fn push_delayed_type_check(&mut self, f: TypeCheckFn) {
self.delayed_type_checks.push(f);
}
Expand Down
1 change: 0 additions & 1 deletion noir_stdlib/src/merkle.nr
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ fn check_membership(_root : Field, _leaf : Field, _index : Field, _hash_path: [F
fn compute_merkle_root(_leaf : Field, _index : Field, _hash_path: [Field]) -> Field {}

// Returns the root of the tree from the provided leaf and its hashpath, using pedersen hash
#[alternative(compute_merkle_root)]
fn compute_root_from_leaf(leaf : Field, index : Field, hash_path: [Field]) -> Field {
let n = hash_path.len();
let index_bits = index.to_le_bits(n as u32);
Expand Down
1 change: 0 additions & 1 deletion noir_stdlib/src/sha256.nr
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,6 @@ fn msg_u8_to_u32(msg: [u8; 64]) -> [u32; 16]
}

// SHA-256 hash function
#[alternative(sha256)]
fn digest<N>(msg: [u8; N]) -> [u8; 32] {
let mut msg_block: [u8; 64] = [0; 64];
let mut h: [u32; 8] = [1779033703,3144134277,1013904242,2773480762,1359893119,2600822924,528734635,1541459225]; // Intermediate hash, starting with the canonical initial value
Expand Down

0 comments on commit dbec6f2

Please sign in to comment.