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

chore!: Require types of globals to be specified #6592

Merged
merged 6 commits into from
Nov 22, 2024
Merged
Show file tree
Hide file tree
Changes from 3 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
20 changes: 20 additions & 0 deletions compiler/noirc_frontend/src/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
pub use type_alias::*;

use crate::{
elaborator::types::WILDCARD_TYPE,
node_interner::{InternedUnresolvedTypeData, QuotedTypeId},
parser::{ParserError, ParserErrorReason},
token::IntType,
Expand Down Expand Up @@ -160,12 +161,31 @@
Error,
}

impl UnresolvedTypeData {
pub(crate) fn is_unspecified(&self) -> bool {
match self {
UnresolvedTypeData::Unspecified => true,
// '_' is unspecified
UnresolvedTypeData::Named(path, _, _) => {
path.to_ident().map(|ident| ident.0.contents) == Some(WILDCARD_TYPE.to_string())
}
_ => false,
}
}
}

#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub struct UnresolvedType {
pub typ: UnresolvedTypeData,
pub span: Span,
}

impl UnresolvedType {
pub(crate) fn is_unspecified(&self) -> bool {
self.typ.is_unspecified()
}
}

/// An argument to a generic type or trait.
#[derive(Debug, PartialEq, Eq, Clone, Hash)]
pub enum GenericTypeArg {
Expand Down Expand Up @@ -534,7 +554,7 @@
Self::Public => write!(f, "pub"),
Self::Private => write!(f, "priv"),
Self::CallData(id) => write!(f, "calldata{id}"),
Self::ReturnData => write!(f, "returndata"),

Check warning on line 557 in compiler/noirc_frontend/src/ast/mod.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (returndata)
}
}
}
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,8 +76,17 @@ impl<'context> Elaborator<'context> {
) -> (HirStatement, Type) {
let expr_span = let_stmt.expression.span;
let (expression, expr_type) = self.elaborate_expression(let_stmt.expression);
let type_is_unspecified = let_stmt.r#type.is_unspecified();
let annotated_type = self.resolve_inferred_type(let_stmt.r#type);

// Require the top-level of a global's type to be specified
if type_is_unspecified && global_id.is_some() {
let span = expr_span;
let expected_type = annotated_type.clone();
let error = ResolverError::UnspecifiedGlobalType { span, expected_type };
self.push_err(error);
}

let definition = match global_id {
None => DefinitionKind::Local(Some(expression)),
Some(id) => DefinitionKind::Global(id),
Expand Down
9 changes: 9 additions & 0 deletions compiler/noirc_frontend/src/hir/resolution/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ pub enum ResolverError {
JumpOutsideLoop { is_break: bool, span: Span },
#[error("Only `comptime` globals can be mutable")]
MutableGlobal { span: Span },
#[error("Globals must have a specified type (RHS inferred to have type `{expected_type}`)")]
UnspecifiedGlobalType { span: Span, expected_type: Type },
#[error("Self-referential structs are not supported")]
SelfReferentialStruct { span: Span },
#[error("#[no_predicates] attribute is only allowed on constrained functions")]
Expand Down Expand Up @@ -431,6 +433,13 @@ impl<'a> From<&'a ResolverError> for Diagnostic {
*span,
)
},
ResolverError::UnspecifiedGlobalType { span, expected_type } => {
Diagnostic::simple_error(
format!("Globals must have a specified type (RHS inferred to have type `{expected_type}`)"),
String::new(),
*span,
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
)
},
ResolverError::SelfReferentialStruct { span } => {
Diagnostic::simple_error(
"Self-referential structs are not supported".into(),
Expand Down
66 changes: 62 additions & 4 deletions compiler/noirc_frontend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1304,7 +1304,21 @@
global B = A;
fn main() {}
"#;
assert_eq!(get_program_errors(src).len(), 1);

let errors = get_program_errors(src);
assert_eq!(errors.len(), 3);
assert!(matches!(
errors[0].0,
CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. })
));
assert!(matches!(
errors[1].0,
CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. })
));
assert!(matches!(
errors[2].0,
CompilationError::ResolverError(ResolverError::DependencyCycle { .. })
));
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}

#[test]
Expand Down Expand Up @@ -1996,7 +2010,7 @@
}

// TODO(https://github.com/noir-lang/noir/issues/6238):
// The EvaluatedGlobalIsntU32 warning is a stopgap

Check warning on line 2013 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
// (originally from https://github.com/noir-lang/noir/issues/6125)
#[test]
fn numeric_generic_field_larger_than_u32() {
Expand All @@ -2013,7 +2027,7 @@
assert_eq!(errors.len(), 2);
assert!(matches!(
errors[0].0,
CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),

Check warning on line 2030 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
));
assert!(matches!(
errors[1].0,
Expand All @@ -2022,7 +2036,7 @@
}

// TODO(https://github.com/noir-lang/noir/issues/6238):
// The EvaluatedGlobalIsntU32 warning is a stopgap

Check warning on line 2039 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
// (originally from https://github.com/noir-lang/noir/issues/6126)
#[test]
fn numeric_generic_field_arithmetic_larger_than_u32() {
Expand Down Expand Up @@ -2051,7 +2065,7 @@

assert!(matches!(
errors[0].0,
CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),

Check warning on line 2068 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
));

assert!(matches!(
Expand Down Expand Up @@ -2187,7 +2201,7 @@
assert_eq!(errors.len(), 3);

// TODO(https://github.com/noir-lang/noir/issues/6238):
// The EvaluatedGlobalIsntU32 warning is a stopgap

Check warning on line 2204 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (Isnt)
assert!(matches!(
errors[0].0,
CompilationError::TypeError(TypeCheckError::EvaluatedGlobalIsntU32 { .. }),
Expand Down Expand Up @@ -3210,10 +3224,10 @@
}

#[test]
fn infer_globals_to_u32_from_type_use() {
fn dont_infer_globals_to_u32_from_type_use() {

Check warning on line 3227 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (dont)
let src = r#"
global ARRAY_LEN = 3;
global STR_LEN = 2;
global STR_LEN: _ = 2;
global FMT_STR_LEN = 2;

fn main() {
Expand All @@ -3223,6 +3237,50 @@
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 3);
assert!(matches!(
errors[0].0,
CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. })
));
assert!(matches!(
errors[1].0,
CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. })
));
assert!(matches!(
errors[2].0,
CompilationError::ResolverError(ResolverError::UnspecifiedGlobalType { .. })
));
}

#[test]
fn infer_partial_global_types() {
let src = r#"
pub global ARRAY: [Field; _] = [0; 3];
pub global STR: str<_> = "hi";
pub global FMT_STR: fmtstr<_, _> = f"hi {ARRAY}";

fn main() { }
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 0);
}
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved

#[test]
fn u32_globals_as_sizes_in_types() {
let src = r#"
global ARRAY_LEN: u32 = 3;
global STR_LEN: u32 = 2;
global FMT_STR_LEN: u32 = 2;

fn main() {
let _a: [u32; ARRAY_LEN] = [1, 2, 3];
let _b: str<STR_LEN> = "hi";
let _c: fmtstr<FMT_STR_LEN, _> = f"hi";
}
"#;

let errors = get_program_errors(src);
assert_eq!(errors.len(), 0);
}
Expand Down Expand Up @@ -3428,7 +3486,7 @@

#[test]
fn unconditional_recursion_fail() {
let srcs = vec![

Check warning on line 3489 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (srcs)
r#"
fn main() {
main()
Expand Down Expand Up @@ -3490,7 +3548,7 @@
"#,
];

for src in srcs {

Check warning on line 3551 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (srcs)
let errors = get_program_errors(src);
assert!(
!errors.is_empty(),
Expand All @@ -3509,7 +3567,7 @@

#[test]
fn unconditional_recursion_pass() {
let srcs = vec![

Check warning on line 3570 in compiler/noirc_frontend/src/tests.rs

View workflow job for this annotation

GitHub Actions / Code

Unknown word (srcs)
r#"
fn main() {
if false { main(); }
Expand Down Expand Up @@ -3686,7 +3744,7 @@
x: [u64; N * 2],
}

global N = 9;
global N: u32 = 9;

fn main(_x: Foo<N * 2>) {}
"#;
Expand Down
6 changes: 3 additions & 3 deletions compiler/noirc_frontend/src/tests/unused_items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -191,8 +191,8 @@ fn errors_on_unused_type_alias() {
#[test]
fn warns_on_unused_global() {
let src = r#"
global foo = 1;
global bar = 1;
global foo: u32 = 1;
global bar: Field = 1;

fn main() {
let _ = bar;
Expand All @@ -216,7 +216,7 @@ fn does_not_warn_on_unused_global_if_it_has_an_abi_attribute() {
let src = r#"
contract foo {
#[abi(notes)]
global bar = 1;
global bar: u64 = 1;
}

fn main() {}
Expand Down
16 changes: 8 additions & 8 deletions docs/docs/noir/concepts/globals.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,12 @@ sidebar_position: 8
## Globals


Noir supports global variables. The global's type can be inferred by the compiler entirely:
Noir supports global variables. The global's type must be specified by the user:

```rust
global N = 5; // Same as `global N: Field = 5`
global N: Field = 5;

global TUPLE = (3, 2);
global TUPLE: (Field, Field) = (3, 2);

fn main() {
assert(N == 5);
Expand All @@ -28,7 +28,7 @@ fn main() {
Globals can be defined as any expression, so long as they don't depend on themselves - otherwise there would be a dependency cycle! For example:

```rust
global T = foo(T); // dependency error
global T: u32 = foo(T); // dependency error
```

:::
Expand All @@ -47,7 +47,7 @@ fn main(y : [Field; N]) {
A global from another module can be imported or referenced externally like any other name:

```rust
global N = 20;
global N: Field = 20;

fn main() {
assert(my_submodule::N != N);
Expand All @@ -62,7 +62,7 @@ When a global is used, Noir replaces the name with its definition on each occurr
This means globals defined using function calls will repeat the call each time they're used:

```rust
global RESULT = foo();
global RESULT: [Field; 100] = foo();

fn foo() -> [Field; 100] { ... }
```
Expand All @@ -78,5 +78,5 @@ to make the global public or `pub(crate)` to make it public to just its crate:

```rust
// This global is now public
pub global N = 5;
```
pub global N: u32 = 5;
```
12 changes: 6 additions & 6 deletions noir_stdlib/src/bigint.nr
Original file line number Diff line number Diff line change
@@ -1,27 +1,27 @@
use crate::cmp::Eq;
use crate::ops::{Add, Div, Mul, Sub};

global bn254_fq = &[
global bn254_fq: [u8] = &[
0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, 0x97,
0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, 0x72, 0x4e, 0x64, 0x30,
];
global bn254_fr = &[
global bn254_fr: [u8] = &[
1, 0, 0, 240, 147, 245, 225, 67, 145, 112, 185, 121, 72, 232, 51, 40, 93, 88, 129, 129, 182, 69,
80, 184, 41, 160, 49, 225, 114, 78, 100, 48,
];
global secpk1_fr = &[
global secpk1_fr: [u8] = &[
0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0x48, 0xAF, 0xE6, 0xDC, 0xAE, 0xBA,
0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
];
global secpk1_fq = &[
global secpk1_fq: [u8] = &[
0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF,
];
global secpr1_fq = &[
global secpr1_fq: [u8] = &[
0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, 0x00,
0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, 0xFF, 0xFF,
];
global secpr1_fr = &[
global secpr1_fr: [u8] = &[
81, 37, 99, 252, 194, 202, 185, 243, 132, 158, 23, 167, 173, 250, 230, 188, 255, 255, 255, 255,
255, 255, 255, 255, 0, 0, 0, 0, 255, 255, 255, 255,
];
Expand Down
4 changes: 2 additions & 2 deletions noir_stdlib/src/collections/map.nr
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@ use crate::option::Option;
// We use load factor alpha_max = 0.75.
// Upon exceeding it, assert will fail in order to inform the user
// about performance degradation, so that he can adjust the capacity.
global MAX_LOAD_FACTOR_NUMERATOR = 3;
global MAX_LOAD_FACTOR_DEN0MINATOR = 4;
global MAX_LOAD_FACTOR_NUMERATOR: u32 = 3;
global MAX_LOAD_FACTOR_DEN0MINATOR: u32 = 4;

/// `HashMap<Key, Value, MaxLen, Hasher>` is used to efficiently store and look up key-value pairs.
///
Expand Down
8 changes: 4 additions & 4 deletions noir_stdlib/src/ec/mod.nr
Original file line number Diff line number Diff line change
Expand Up @@ -122,12 +122,12 @@ pub mod consts; // Commonly used curve presets
// Field-dependent constant ZETA = a non-square element of Field
// Required for Elligator 2 map
// TODO: Replace with built-in constant.
global ZETA = 5;
global ZETA: Field = 5;
// Field-dependent constants for Tonelli-Shanks algorithm (see sqrt function below)
// TODO: Possibly make this built-in.
global C1 = 28;
global C3 = 40770029410420498293352137776570907027550720424234931066070132305055;
global C5 = 19103219067921713944291392827692070036145651957329286315305642004821462161904;
global C1: u32 = 28;
global C3: Field = 40770029410420498293352137776570907027550720424234931066070132305055;
global C5: Field = 19103219067921713944291392827692070036145651957329286315305642004821462161904;
// Higher-order version of scalar multiplication
// TODO: Make this work so that the submodules' bit_mul may be defined in terms of it.
//fn bit_mul<T,N>(add: fn(T,T) -> T, e: T, bits: [u1; N], p: T) -> T {
Expand Down
18 changes: 9 additions & 9 deletions noir_stdlib/src/hash/sha256.nr
Original file line number Diff line number Diff line change
Expand Up @@ -4,27 +4,27 @@ use crate::runtime::is_unconstrained;
// 32 bytes.

// A message block is up to 64 bytes taken from the input.
global BLOCK_SIZE = 64;
global BLOCK_SIZE: u32 = 64;

// The first index in the block where the 8 byte message size will be written.
global MSG_SIZE_PTR = 56;
global MSG_SIZE_PTR: u32 = 56;

// Size of the message block when packed as 4-byte integer array.
global INT_BLOCK_SIZE = 16;
global INT_BLOCK_SIZE: u32 = 16;

// A `u32` integer consists of 4 bytes.
global INT_SIZE = 4;
global INT_SIZE: u32 = 4;

// Index of the integer in the `INT_BLOCK` where the length is written.
global INT_SIZE_PTR = MSG_SIZE_PTR / INT_SIZE;
global INT_SIZE_PTR: u32 = MSG_SIZE_PTR / INT_SIZE;

// Magic numbers for bit shifting.
// Works with actual bit shifting as well as the compiler turns them into * and /
// but circuit execution appears to be 10% faster this way.
global TWO_POW_8 = 256;
global TWO_POW_16 = TWO_POW_8 * 256;
global TWO_POW_24 = TWO_POW_16 * 256;
global TWO_POW_32 = TWO_POW_24 as u64 * 256;
global TWO_POW_8: u32 = 256;
global TWO_POW_16: u32 = TWO_POW_8 * 256;
global TWO_POW_24: u32 = TWO_POW_16 * 256;
global TWO_POW_32: u64 = TWO_POW_24 as u64 * 256;

// Index of a byte in a 64 byte block; ie. 0..=63
type BLOCK_BYTE_PTR = u32;
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::hash::poseidon2;

global SIZE = 100;
global SIZE: u32 = 100;

fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] {
let mut results: [Field; SIZE] = [0; SIZE];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::hash::poseidon2;

global SIZE = 30;
global SIZE: u32 = 30;

fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] {
let mut results: [Field; SIZE] = [0; SIZE];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::hash;

global SIZE = 100;
global SIZE: u32 = 100;

fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] {
let mut results: [Field; SIZE] = [0; SIZE];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::hash;

global SIZE = 30;
global SIZE: u32 = 30;

fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] {
let mut results: [Field; SIZE] = [0; SIZE];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::hash;

global SIZE = 100;
global SIZE: u32 = 100;

fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] {
let mut results: [Field; SIZE] = [0; SIZE];
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use std::hash;

global SIZE = 30;
global SIZE: u32 = 30;

fn main(input: [[Field; 2]; SIZE]) -> pub [Field; SIZE] {
let mut results: [Field; SIZE] = [0; SIZE];
Expand Down
Loading
Loading