Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat!: remove concept of noir fallbacks for foreign functions #1371

Merged
merged 1 commit into from
May 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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