Skip to content

Commit

Permalink
Merge branch 'master' into aztec-packages
Browse files Browse the repository at this point in the history
* master:
  fix(lsp): replace panics with errors (#4209)
  feat: Improve Error Handling for Cargo in Bootstrap Script (#4211)
  fix: prevent declarations of blackbox functions outside of the stdlib (#4177)
  feat: disable unused variable checks on low-level and oracle functions (#4179)
  chore: Rename acir_docs.md to README.md (#4208)
  feat: remove replacement of boolean range opcodes with `AssertZero` opcodes (#4107)
  chore(docs): updating docs to match new recursion interfacee (#4187)
  feat!: Sync commits from `aztec-packages` (#4144)
  • Loading branch information
TomAFrench committed Jan 31, 2024
2 parents caf0b25 + 26e9618 commit b38ffdd
Show file tree
Hide file tree
Showing 31 changed files with 290 additions and 240 deletions.
File renamed without changes.
72 changes: 26 additions & 46 deletions acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,7 @@ use acir::{
opcodes::{BlackBoxFuncCall, FunctionInput},
Circuit, Opcode,
},
native_types::{Expression, Witness},
FieldElement,
native_types::Witness,
};
use std::collections::{BTreeMap, HashSet};

Expand Down Expand Up @@ -105,9 +104,11 @@ impl RangeOptimizer {
let mut new_order_list = Vec::with_capacity(order_list.len());
let mut optimized_opcodes = Vec::with_capacity(self.circuit.opcodes.len());
for (idx, opcode) in self.circuit.opcodes.into_iter().enumerate() {
let (witness, num_bits) = match extract_range_opcode(&opcode) {
Some(range_opcode) => range_opcode,
None => {
let (witness, num_bits) = match &opcode {
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => {
(input.witness, input.num_bits)
}
_ => {
// If its not the range opcode, add it to the opcode
// list and continue;
optimized_opcodes.push(opcode);
Expand All @@ -131,44 +132,19 @@ impl RangeOptimizer {
if is_lowest_bit_size {
already_seen_witness.insert(witness);
new_order_list.push(order_list[idx]);
optimized_opcodes.push(optimized_range_opcode(witness, num_bits));
optimized_opcodes.push(opcode);
}
}

(Circuit { opcodes: optimized_opcodes, ..self.circuit }, new_order_list)
}
}

/// Extract the range opcode from the `Opcode` enum
/// Returns None, if `Opcode` is not the range opcode.
fn extract_range_opcode(opcode: &Opcode) -> Option<(Witness, u32)> {
match opcode {
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => {
Some((input.witness, input.num_bits))
}
_ => None,
}
}

fn optimized_range_opcode(witness: Witness, num_bits: u32) -> Opcode {
if num_bits == 1 {
Opcode::AssertZero(Expression {
mul_terms: vec![(FieldElement::one(), witness, witness)],
linear_combinations: vec![(-FieldElement::one(), witness)],
q_c: FieldElement::zero(),
})
} else {
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
input: FunctionInput { witness, num_bits },
})
}
}

#[cfg(test)]
mod tests {
use std::collections::BTreeSet;

use crate::compiler::optimizers::redundant_range::{extract_range_opcode, RangeOptimizer};
use crate::compiler::optimizers::redundant_range::RangeOptimizer;
use acir::{
circuit::{
opcodes::{BlackBoxFuncCall, FunctionInput},
Expand Down Expand Up @@ -218,11 +194,12 @@ mod tests {
let (optimized_circuit, _) = optimizer.replace_redundant_ranges(acir_opcode_positions);
assert_eq!(optimized_circuit.opcodes.len(), 1);

let (witness, num_bits) =
extract_range_opcode(&optimized_circuit.opcodes[0]).expect("expected one range opcode");

assert_eq!(witness, Witness(1));
assert_eq!(num_bits, 16);
assert_eq!(
optimized_circuit.opcodes[0],
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
input: FunctionInput { witness: Witness(1), num_bits: 16 }
})
);
}

#[test]
Expand All @@ -240,15 +217,18 @@ mod tests {
let (optimized_circuit, _) = optimizer.replace_redundant_ranges(acir_opcode_positions);
assert_eq!(optimized_circuit.opcodes.len(), 2);

let (witness_a, num_bits_a) =
extract_range_opcode(&optimized_circuit.opcodes[0]).expect("expected two range opcode");
let (witness_b, num_bits_b) =
extract_range_opcode(&optimized_circuit.opcodes[1]).expect("expected two range opcode");

assert_eq!(witness_a, Witness(1));
assert_eq!(witness_b, Witness(2));
assert_eq!(num_bits_a, 16);
assert_eq!(num_bits_b, 23);
assert_eq!(
optimized_circuit.opcodes[0],
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
input: FunctionInput { witness: Witness(1), num_bits: 16 }
})
);
assert_eq!(
optimized_circuit.opcodes[1],
Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
input: FunctionInput { witness: Witness(2), num_bits: 23 }
})
);
}

#[test]
Expand Down
86 changes: 65 additions & 21 deletions aztec_macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,12 @@ impl MacroProcessor for AztecMacro {
transform(ast, crate_id, context)
}

fn process_typed_ast(&self, crate_id: &CrateId, context: &mut HirContext) {
transform_hir(crate_id, context)
fn process_typed_ast(
&self,
crate_id: &CrateId,
context: &mut HirContext,
) -> Result<(), (MacroError, FileId)> {
transform_hir(crate_id, context).map_err(|(err, file_id)| (err.into(), file_id))
}
}

Expand All @@ -41,6 +45,7 @@ pub enum AztecMacroError {
ContractHasTooManyFunctions { span: Span },
ContractConstructorMissing { span: Span },
UnsupportedFunctionArgumentType { span: Span, typ: UnresolvedTypeData },
EventError { span: Span, message: String },
}

impl From<AztecMacroError> for MacroError {
Expand Down Expand Up @@ -71,6 +76,11 @@ impl From<AztecMacroError> for MacroError {
secondary_message: None,
span: Some(span),
},
AztecMacroError::EventError { span, message } => MacroError {
primary_message: message,
secondary_message: None,
span: Some(span),
},
}
}
}
Expand Down Expand Up @@ -237,8 +247,11 @@ fn transform(
//

/// Completes the Hir with data gathered from type resolution
fn transform_hir(crate_id: &CrateId, context: &mut HirContext) {
transform_events(crate_id, context);
fn transform_hir(
crate_id: &CrateId,
context: &mut HirContext,
) -> Result<(), (AztecMacroError, FileId)> {
transform_events(crate_id, context)
}

/// Includes an import to the aztec library if it has not been included yet
Expand Down Expand Up @@ -472,19 +485,30 @@ fn collect_crate_structs(crate_id: &CrateId, context: &HirContext) -> Vec<Struct
}

/// Substitutes the signature literal that was introduced in the selector method previously with the actual signature.
fn transform_event(struct_id: StructId, interner: &mut NodeInterner) {
fn transform_event(
struct_id: StructId,
interner: &mut NodeInterner,
) -> Result<(), (AztecMacroError, FileId)> {
let struct_type = interner.get_struct(struct_id);
let selector_id = interner
.lookup_method(&Type::Struct(struct_type, vec![]), struct_id, "selector", false)
.expect("Selector method not found");
.lookup_method(&Type::Struct(struct_type.clone(), vec![]), struct_id, "selector", false)
.ok_or_else(|| {
let error = AztecMacroError::EventError {
span: struct_type.borrow().location.span,
message: "Selector method not found".to_owned(),
};
(error, struct_type.borrow().location.file)
})?;
let selector_function = interner.function(&selector_id);

let compute_selector_statement = interner.statement(
selector_function
.block(interner)
.statements()
.first()
.expect("Compute selector statement not found"),
selector_function.block(interner).statements().first().ok_or_else(|| {
let error = AztecMacroError::EventError {
span: struct_type.borrow().location.span,
message: "Compute selector statement not found".to_owned(),
};
(error, struct_type.borrow().location.file)
})?,
);

let compute_selector_expression = match compute_selector_statement {
Expand All @@ -494,12 +518,21 @@ fn transform_event(struct_id: StructId, interner: &mut NodeInterner) {
},
_ => None,
}
.expect("Compute selector statement is not a call expression");

let first_arg_id = compute_selector_expression
.arguments
.first()
.expect("Missing argument for compute selector");
.ok_or_else(|| {
let error = AztecMacroError::EventError {
span: struct_type.borrow().location.span,
message: "Compute selector statement is not a call expression".to_owned(),
};
(error, struct_type.borrow().location.file)
})?;

let first_arg_id = compute_selector_expression.arguments.first().ok_or_else(|| {
let error = AztecMacroError::EventError {
span: struct_type.borrow().location.span,
message: "Compute selector statement is not a call expression".to_owned(),
};
(error, struct_type.borrow().location.file)
})?;

match interner.expression(first_arg_id) {
HirExpression::Literal(HirLiteral::Str(signature))
Expand All @@ -518,18 +551,29 @@ fn transform_event(struct_id: StructId, interner: &mut NodeInterner) {
selector_literal_id,
Type::String(Box::new(Type::Constant(signature.len() as u64))),
);
Ok(())
}
_ => unreachable!("Signature placeholder literal does not match"),
_ => Err((
AztecMacroError::EventError {
span: struct_type.borrow().location.span,
message: "Signature placeholder literal does not match".to_owned(),
},
struct_type.borrow().location.file,
)),
}
}

fn transform_events(crate_id: &CrateId, context: &mut HirContext) {
fn transform_events(
crate_id: &CrateId,
context: &mut HirContext,
) -> Result<(), (AztecMacroError, FileId)> {
for struct_id in collect_crate_structs(crate_id, context) {
let attributes = context.def_interner.struct_attributes(&struct_id);
if attributes.iter().any(|attr| matches!(attr, SecondaryAttribute::Event)) {
transform_event(struct_id, &mut context.def_interner);
transform_event(struct_id, &mut context.def_interner)?;
}
}
Ok(())
}

const SIGNATURE_PLACEHOLDER: &str = "SIGNATURE_PLACEHOLDER";
Expand Down
14 changes: 12 additions & 2 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ use crate::hir::resolution::{
use crate::hir::type_check::{type_check_func, TypeCheckError, TypeChecker};
use crate::hir::Context;

use crate::macros_api::MacroProcessor;
use crate::macros_api::{MacroError, MacroProcessor};
use crate::node_interner::{FuncId, NodeInterner, StmtId, StructId, TraitId, TypeAliasId};

use crate::parser::{ParserError, SortedModule};
Expand Down Expand Up @@ -155,6 +155,12 @@ impl From<CompilationError> for CustomDiagnostic {
}
}

impl From<MacroError> for CompilationError {
fn from(value: MacroError) -> Self {
CompilationError::DefinitionError(DefCollectorErrorKind::MacroError(value))
}
}

impl From<ParserError> for CompilationError {
fn from(value: ParserError) -> Self {
CompilationError::ParseError(value)
Expand Down Expand Up @@ -359,7 +365,11 @@ impl DefCollector {
errors.extend(resolved_globals.errors);

for macro_processor in macro_processors {
macro_processor.process_typed_ast(&crate_id, context);
macro_processor.process_typed_ast(&crate_id, context).unwrap_or_else(
|(macro_err, file_id)| {
errors.push((macro_err.into(), file_id));
},
);
}
errors.extend(type_check_globals(&mut context.def_interner, resolved_globals.globals));

Expand Down
7 changes: 7 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,8 @@ pub enum ResolverError {
InvalidTypeForEntryPoint { span: Span },
#[error("Nested slices are not supported")]
NestedSlices { span: Span },
#[error("Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library")]
LowLevelFunctionOutsideOfStdlib { ident: Ident },
}

impl ResolverError {
Expand Down Expand Up @@ -311,6 +313,11 @@ impl From<ResolverError> for Diagnostic {
"Try to use a constant sized array instead".into(),
span,
),
ResolverError::LowLevelFunctionOutsideOfStdlib { ident } => Diagnostic::simple_error(
"Definition of low-level function outside of standard library".into(),
"Usage of the `#[foreign]` or `#[builtin]` function attributes are not allowed outside of the Noir standard library".into(),
ident.span(),
),
}
}
}
17 changes: 16 additions & 1 deletion compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,10 +191,18 @@ impl<'a> Resolver<'a> {
self.add_generics(&func.def.generics);
self.trait_bounds = func.def.where_clause.clone();

let is_low_level_or_oracle = func
.attributes()
.function
.as_ref()
.map_or(false, |func| func.is_low_level() || func.is_oracle());
let (hir_func, func_meta) = self.intern_function(func, func_id);
let func_scope_tree = self.scopes.end_function();

self.check_for_unused_variables_in_scope_tree(func_scope_tree);
// The arguments to low-level and oracle functions are always unused so we do not produce warnings for them.
if !is_low_level_or_oracle {
self.check_for_unused_variables_in_scope_tree(func_scope_tree);
}

self.trait_bounds.clear();
(hir_func, func_meta, self.errors)
Expand Down Expand Up @@ -900,6 +908,13 @@ impl<'a> Resolver<'a> {
position: PubPosition::ReturnType,
});
}
let is_low_level_function =
func.attributes().function.as_ref().map_or(false, |func| func.is_low_level());
if !self.path_resolver.module_id().krate.is_stdlib() && is_low_level_function {
let error =
ResolverError::LowLevelFunctionOutsideOfStdlib { ident: func.name_ident().clone() };
self.push_err(error);
}

// 'pub' is required on return types for entry point functions
if self.is_entry_point_function(func)
Expand Down
4 changes: 4 additions & 0 deletions compiler/noirc_frontend/src/lexer/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -562,6 +562,10 @@ impl FunctionAttribute {
matches!(self, FunctionAttribute::Foreign(_))
}

pub fn is_oracle(&self) -> bool {
matches!(self, FunctionAttribute::Oracle(_))
}

pub fn is_low_level(&self) -> bool {
matches!(self, FunctionAttribute::Foreign(_) | FunctionAttribute::Builtin(_))
}
Expand Down
6 changes: 5 additions & 1 deletion compiler/noirc_frontend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,10 @@ pub mod macros_api {
) -> Result<SortedModule, (MacroError, FileId)>;
/// Function to manipulate the AST after type checking has been completed.
/// The AST after type checking has been done is called the HIR.
fn process_typed_ast(&self, crate_id: &CrateId, context: &mut HirContext);
fn process_typed_ast(
&self,
crate_id: &CrateId,
context: &mut HirContext,
) -> Result<(), (MacroError, FileId)>;
}
}
2 changes: 2 additions & 0 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,10 +52,12 @@ mod test {
) -> (ParsedModule, Context, Vec<(CompilationError, FileId)>) {
let root = std::path::Path::new("/");
let fm = FileManager::new(root);

let mut context = Context::new(fm, Default::default());
context.def_interner.populate_dummy_operator_traits();
let root_file_id = FileId::dummy();
let root_crate_id = context.crate_graph.add_crate_root(root_file_id);

let (program, parser_errors) = parse_program(src);
let mut errors = vecmap(parser_errors, |e| (e.into(), root_file_id));
remove_experimental_warnings(&mut errors);
Expand Down

0 comments on commit b38ffdd

Please sign in to comment.