Skip to content

Commit

Permalink
feat: disable unused variable checks on low-level and oracle functions (
Browse files Browse the repository at this point in the history
#4179)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## Summary\*

Any low-level functions don't have a function implementation in Noir so
all inputs will appear unused. We currently prefix these arguments with
underscores to silence this but this leaks through into the docs where
preferably we'd have non-underscored variables.

This also affects any oracle functions which may exist in user code,
requiring them to use underscores in the same way which isn't ideal.

This PR then disables unused variable checks on any function with the
`#[foreign]`, `#[builtin]` or `#[oracle]` attributes.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[Exceptional Case]** Documentation to be submitted in a separate
PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.
  • Loading branch information
TomAFrench authored Jan 30, 2024
1 parent 2d93169 commit 8f70e57
Show file tree
Hide file tree
Showing 13 changed files with 62 additions and 50 deletions.
10 changes: 9 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
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
4 changes: 2 additions & 2 deletions noir_stdlib/src/array.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,10 @@
// by the methods in the `slice` module
impl<T, N> [T; N] {
#[builtin(array_len)]
pub fn len(_self: Self) -> Field {}
pub fn len(self) -> Field {}

#[builtin(arraysort)]
pub fn sort(_self: Self) -> Self {}
pub fn sort(self) -> Self {}

// Sort with a custom sorting function.
pub fn sort_via<Env>(mut a: Self, ordering: fn[Env](T, T) -> bool) -> Self {
Expand Down
10 changes: 5 additions & 5 deletions noir_stdlib/src/ecdsa_secp256k1.nr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#[foreign(ecdsa_secp256k1)]
// docs:start:ecdsa_secp256k1
pub fn verify_signature<N>(
_public_key_x: [u8; 32],
_public_key_y: [u8; 32],
_signature: [u8; 64],
_message_hash: [u8; N]
public_key_x: [u8; 32],
public_key_y: [u8; 32],
signature: [u8; 64],
message_hash: [u8; N]
) -> bool
// docs:end:ecdsa_secp256k1
{}
{}
10 changes: 5 additions & 5 deletions noir_stdlib/src/ecdsa_secp256r1.nr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#[foreign(ecdsa_secp256r1)]
// docs:start:ecdsa_secp256r1
pub fn verify_signature<N>(
_public_key_x: [u8; 32],
_public_key_y: [u8; 32],
_signature: [u8; 64],
_message_hash: [u8; N]
public_key_x: [u8; 32],
public_key_y: [u8; 32],
signature: [u8; 64],
message_hash: [u8; N]
) -> bool
// docs:end:ecdsa_secp256r1
{}
{}
10 changes: 5 additions & 5 deletions noir_stdlib/src/field.nr
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ impl Field {
}

#[builtin(to_le_bits)]
fn __to_le_bits(_self: Self, _bit_size: u32) -> [u1] {}
fn __to_le_bits(self, _bit_size: u32) -> [u1] {}

#[builtin(to_be_bits)]
fn __to_be_bits(_self: Self, _bit_size: u32) -> [u1] {}
fn __to_be_bits(self, bit_size: u32) -> [u1] {}

#[builtin(apply_range_constraint)]
fn __assert_max_bit_size(_self: Self, _bit_size: u32) {}
fn __assert_max_bit_size(self, bit_size: u32) {}

pub fn assert_max_bit_size(self: Self, bit_size: u32) {
crate::assert_constant(bit_size);
Expand Down Expand Up @@ -53,10 +53,10 @@ impl Field {
// decompose `_self` into a `_result_len` vector over the `_radix` basis
// `_radix` must be less than 256
#[builtin(to_le_radix)]
fn __to_le_radix(_self: Self, _radix: u32, _result_len: u32) -> [u8] {}
fn __to_le_radix(self, radix: u32, result_len: u32) -> [u8] {}

#[builtin(to_be_radix)]
fn __to_be_radix(_self: Self, _radix: u32, _result_len: u32) -> [u8] {}
fn __to_be_radix(self, radix: u32, result_len: u32) -> [u8] {}


// Returns self to the power of the given exponent value.
Expand Down
16 changes: 8 additions & 8 deletions noir_stdlib/src/hash.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ mod mimc;

#[foreign(sha256)]
// docs:start:sha256
pub fn sha256<N>(_input: [u8; N]) -> [u8; 32]
pub fn sha256<N>(input: [u8; N]) -> [u8; 32]
// docs:end:sha256
{}

#[foreign(blake2s)]
// docs:start:blake2s
pub fn blake2s<N>(_input: [u8; N]) -> [u8; 32]
pub fn blake2s<N>(input: [u8; N]) -> [u8; 32]
// docs:end:blake2s
{}

#[foreign(blake3)]
// docs:start:blake3
pub fn blake3<N>(_input: [u8; N]) -> [u8; 32]
pub fn blake3<N>(input: [u8; N]) -> [u8; 32]
// docs:end:blake3
{}

Expand All @@ -32,7 +32,7 @@ pub fn pedersen_commitment<N>(input: [Field; N]) -> PedersenPoint
}

#[foreign(pedersen_commitment)]
pub fn __pedersen_commitment_with_separator<N>(_input: [Field; N], _separator: u32) -> [Field; 2] {}
pub fn __pedersen_commitment_with_separator<N>(input: [Field; N], separator: u32) -> [Field; 2] {}

pub fn pedersen_commitment_with_separator<N>(input: [Field; N], separator: u32) -> PedersenPoint {
let values = __pedersen_commitment_with_separator(input, separator);
Expand All @@ -47,13 +47,13 @@ pub fn pedersen_hash<N>(input: [Field; N]) -> Field
}

#[foreign(pedersen_hash)]
pub fn pedersen_hash_with_separator<N>(_input: [Field; N], _separator: u32) -> Field {}
pub fn pedersen_hash_with_separator<N>(input: [Field; N], separator: u32) -> Field {}

pub fn hash_to_field<N>(_input: [Field; N]) -> Field {
pub fn hash_to_field<N>(input: [Field; N]) -> Field {
let mut inputs_as_bytes = [];

for i in 0..N {
let input_bytes = _input[i].to_le_bytes(32);
let input_bytes = input[i].to_le_bytes(32);
for i in 0..32 {
inputs_as_bytes = inputs_as_bytes.push_back(input_bytes[i]);
}
Expand All @@ -65,7 +65,7 @@ pub fn hash_to_field<N>(_input: [Field; N]) -> Field {

#[foreign(keccak256)]
// docs:start:keccak256
pub fn keccak256<N>(_input: [u8; N], _message_size: u32) -> [u8; 32]
pub fn keccak256<N>(input: [u8; N], message_size: u32) -> [u8; 32]
// docs:end:keccak256
{}

Expand Down
10 changes: 5 additions & 5 deletions noir_stdlib/src/lib.nr
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ mod uint128;
// Oracle calls are required to be wrapped in an unconstrained function
// Thus, the only argument to the `println` oracle is expected to always be an ident
#[oracle(print)]
unconstrained fn print_oracle<T>(_with_newline: bool, _input: T) {}
unconstrained fn print_oracle<T>(with_newline: bool, input: T) {}

unconstrained pub fn print<T>(input: T) {
print_oracle(false, input);
Expand All @@ -41,20 +41,20 @@ unconstrained pub fn println<T>(input: T) {
}

#[foreign(recursive_aggregation)]
pub fn verify_proof<N>(_verification_key: [Field], _proof: [Field], _public_inputs: [Field], _key_hash: Field) {}
pub fn verify_proof<N>(verification_key: [Field], proof: [Field], public_inputs: [Field], key_hash: Field) {}

// Asserts that the given value is known at compile-time.
// Useful for debugging for-loop bounds.
#[builtin(assert_constant)]
pub fn assert_constant<T>(_x: T) {}
pub fn assert_constant<T>(x: T) {}
// from_field and as_field are private since they are not valid for every type.
// `as` should be the default for users to cast between primitive types, and in the future
// traits can be used to work with generic types.
#[builtin(from_field)]
fn from_field<T>(_x: Field) -> T {}
fn from_field<T>(x: Field) -> T {}

#[builtin(as_field)]
fn as_field<T>(_x: T) -> Field {}
fn as_field<T>(x: T) -> Field {}

pub fn wrapping_add<T>(x: T, y: T) -> T {
crate::from_field(crate::as_field(x) + crate::as_field(y))
Expand Down
4 changes: 2 additions & 2 deletions noir_stdlib/src/scalar_mul.nr
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,8 @@ impl Add for EmbeddedCurvePoint {
#[foreign(fixed_base_scalar_mul)]
// docs:start:fixed_base_embedded_curve
pub fn fixed_base_embedded_curve(
_low: Field,
_high: Field
low: Field,
high: Field
) -> [Field; 2]
// docs:end:fixed_base_embedded_curve
{}
Expand Down
10 changes: 5 additions & 5 deletions noir_stdlib/src/schnorr.nr
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
#[foreign(schnorr_verify)]
// docs:start:schnorr_verify
pub fn verify_signature<N>(
_public_key_x: Field,
_public_key_y: Field,
_signature: [u8; 64],
_message: [u8; N]
public_key_x: Field,
public_key_y: Field,
signature: [u8; 64],
message: [u8; N]
) -> bool
// docs:end:schnorr_verify
{}
{}
12 changes: 6 additions & 6 deletions noir_stdlib/src/slice.nr
Original file line number Diff line number Diff line change
Expand Up @@ -3,34 +3,34 @@ impl<T> [T] {
/// new slice with a length one greater than the
/// original unmodified slice.
#[builtin(slice_push_back)]
pub fn push_back(_self: Self, _elem: T) -> Self { }
pub fn push_back(self, elem: T) -> Self { }

/// Push a new element to the front of the slice, returning a
/// new slice with a length one greater than the
/// original unmodified slice.
#[builtin(slice_push_front)]
pub fn push_front(_self: Self, _elem: T) -> Self { }
pub fn push_front(self, elem: T) -> Self { }

/// Remove the last element of the slice, returning the
/// popped slice and the element in a tuple
#[builtin(slice_pop_back)]
pub fn pop_back(_self: Self) -> (Self, T) { }
pub fn pop_back(self) -> (Self, T) { }

/// Remove the first element of the slice, returning the
/// element and the popped slice in a tuple
#[builtin(slice_pop_front)]
pub fn pop_front(_self: Self) -> (T, Self) { }
pub fn pop_front(self) -> (T, Self) { }

/// Insert an element at a specified index, shifting all elements
/// after it to the right
#[builtin(slice_insert)]
pub fn insert(_self: Self, _index: Field, _elem: T) -> Self { }
pub fn insert(self, index: Field, elem: T) -> Self { }

/// Remove an element at a specified index, shifting all elements
/// after it to the left, returning the altered slice and
/// the removed element
#[builtin(slice_remove)]
pub fn remove(_self: Self, _index: Field) -> (Self, T) { }
pub fn remove(self, index: Field) -> (Self, T) { }

// Append each element of the `other` slice to the end of `self`.
// This returns a new slice and leaves both input slices unchanged.
Expand Down
2 changes: 1 addition & 1 deletion noir_stdlib/src/string.nr
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::collections::vec::Vec;
impl<N> str<N> {
/// Converts the given string into a byte array
#[builtin(str_as_bytes)]
pub fn as_bytes(_self: Self) -> [u8; N] { }
pub fn as_bytes(self) -> [u8; N] { }

/// return a byte vector of the str content
pub fn as_bytes_vec(self: Self) -> Vec<u8> {
Expand Down
10 changes: 5 additions & 5 deletions noir_stdlib/src/test.nr
Original file line number Diff line number Diff line change
@@ -1,17 +1,17 @@
#[oracle(create_mock)]
unconstrained fn create_mock_oracle<N>(_name: str<N>) -> Field {}
unconstrained fn create_mock_oracle<N>(name: str<N>) -> Field {}

#[oracle(set_mock_params)]
unconstrained fn set_mock_params_oracle<P>(_id: Field, _params: P) {}
unconstrained fn set_mock_params_oracle<P>(id: Field, params: P) {}

#[oracle(set_mock_returns)]
unconstrained fn set_mock_returns_oracle<R>(_id: Field, _returns: R) {}
unconstrained fn set_mock_returns_oracle<R>(id: Field, returns: R) {}

#[oracle(set_mock_times)]
unconstrained fn set_mock_times_oracle(_id: Field, _times: u64) {}
unconstrained fn set_mock_times_oracle(id: Field, times: u64) {}

#[oracle(clear_mock)]
unconstrained fn clear_mock_oracle(_id: Field) {}
unconstrained fn clear_mock_oracle(id: Field) {}

struct OracleMock {
id: Field,
Expand Down

0 comments on commit 8f70e57

Please sign in to comment.