Skip to content

Commit

Permalink
Merge branch 'master' into tf/optimize-boolean-ranges
Browse files Browse the repository at this point in the history
* master:
  chore(docs): Rearrange NoirJS section (#3260)
  chore(docs): Document `nargo fmt` (#3262)
  feat(debugger): Print limited source code context (#3217)
  fix: recompile artefacts from a different noir version (#3248)
  chore: Update ACIR artifacts (#3263)
  fix: Show println output before an error occurs in `nargo execute` (#3211)
  chore(docs): document info codelens (#3252)
  fix: Impl methods are no longer placed in contracts (#3255)
  feat(stdlib): optimize constraint counts in sha256/sha512 (#3253)
  chore: enhance code formatting for If expressions (#3246)
  feat: Cache debug artifacts  (#3133)
  • Loading branch information
TomAFrench committed Oct 24, 2023
2 parents b05ef73 + 031bd4d commit 0dc9608
Show file tree
Hide file tree
Showing 57 changed files with 720 additions and 303 deletions.
2 changes: 2 additions & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions compiler/noirc_driver/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ license.workspace = true

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[build-dependencies]
build-data = "0.1.3"

[dependencies]
clap.workspace = true
noirc_errors.workspace = true
Expand Down
14 changes: 14 additions & 0 deletions compiler/noirc_driver/build.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
const GIT_COMMIT: &&str = &"GIT_COMMIT";

fn main() {
// Rebuild if the tests have changed
println!("cargo:rerun-if-changed=tests");

// Only use build_data if the environment variable isn't set
// The environment variable is always set when working via Nix
if std::env::var(GIT_COMMIT).is_err() {
build_data::set_GIT_COMMIT();
build_data::set_GIT_DIRTY();
build_data::no_debug_rebuilds();
}
}
2 changes: 2 additions & 0 deletions compiler/noirc_driver/src/contract.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ pub enum ContractFunctionType {

#[derive(Serialize, Deserialize)]
pub struct CompiledContract {
pub noir_version: String,

/// The name of the contract.
pub name: String,
/// Each of the contract's functions are compiled into a separate `CompiledProgram`
Expand Down
19 changes: 18 additions & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,15 @@ pub use program::CompiledProgram;

const STD_CRATE_NAME: &str = "std";

pub const GIT_COMMIT: &str = env!("GIT_COMMIT");
pub const GIT_DIRTY: &str = env!("GIT_DIRTY");
pub const NOIRC_VERSION: &str = env!("CARGO_PKG_VERSION");

/// Version string that gets placed in artifacts that Noir builds. This is semver compatible.
/// Note: You can't directly use the value of a constant produced with env! inside a concat! macro.
pub const NOIR_ARTIFACT_VERSION_STRING: &str =
concat!(env!("CARGO_PKG_VERSION"), "+", env!("GIT_COMMIT"));

#[derive(Args, Clone, Debug, Default, Serialize, Deserialize)]
pub struct CompileOptions {
/// Emit debug information for the intermediate SSA IR
Expand Down Expand Up @@ -305,6 +314,7 @@ fn compile_contract_inner(
.collect(),
functions,
file_map,
noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(),
})
} else {
Err(errors)
Expand Down Expand Up @@ -342,5 +352,12 @@ pub fn compile_no_check(

let file_map = filter_relevant_files(&[debug.clone()], &context.file_manager);

Ok(CompiledProgram { hash, circuit, debug, abi, file_map })
Ok(CompiledProgram {
hash,
circuit,
debug,
abi,
file_map,
noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(),
})
}
1 change: 1 addition & 0 deletions compiler/noirc_driver/src/program.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use super::debug::DebugFile;

#[derive(Debug, Serialize, Deserialize, Clone)]
pub struct CompiledProgram {
pub noir_version: String,
/// Hash of the [`Program`][noirc_frontend::monomorphization::ast::Program] from which this [`CompiledProgram`]
/// was compiled.
///
Expand Down
18 changes: 4 additions & 14 deletions compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -427,20 +427,10 @@ impl AcirContext {
let diff_expr = &lhs_expr - &rhs_expr;

// Check to see if equality can be determined at compile-time.
if diff_expr.is_const() {
if diff_expr.is_zero() {
// Constraint is always true - assertion is unnecessary.
self.mark_variables_equivalent(lhs, rhs)?;
return Ok(());
} else {
// Constraint is always false - this program is unprovable.
return Err(RuntimeError::FailedConstraint {
lhs: Box::new(lhs_expr),
rhs: Box::new(rhs_expr),
call_stack: self.get_call_stack(),
assert_message,
});
};
if diff_expr.is_zero() {
// Constraint is always true - assertion is unnecessary.
self.mark_variables_equivalent(lhs, rhs)?;
return Ok(());
}

self.acir_ir.assert_is_zero(diff_expr);
Expand Down
5 changes: 5 additions & 0 deletions compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1173,6 +1173,11 @@ fn resolve_function_set(
resolver.set_self_type(self_type.clone());
resolver.set_trait_id(unresolved_functions.trait_id);

// Without this, impl methods can accidentally be placed in contracts. See #3254
if self_type.is_some() {
resolver.set_in_contract(false);
}

let (hir_func, func_meta, errs) = resolver.resolve_function(func, func_id);
interner.push_fn_meta(func_meta, func_id);
interner.update_fn(func_id, hir_func);
Expand Down
34 changes: 24 additions & 10 deletions compiler/noirc_frontend/src/hir/resolution/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,14 @@ pub struct Resolver<'a> {
/// Set to the current type if we're resolving an impl
self_type: Option<Type>,

/// True if the current module is a contract.
/// This is usually determined by self.path_resolver.module_id(), but it can
/// be overriden for impls. Impls are an odd case since the methods within resolve
/// as if they're in the parent module, but should be placed in a child module.
/// Since they should be within a child module, in_contract is manually set to false
/// for these so we can still resolve them in the parent module without them being in a contract.
in_contract: bool,

/// Contains a mapping of the current struct or functions's generics to
/// unique type variables if we're resolving a struct. Empty otherwise.
/// This is a Vec rather than a map to preserve the order a functions generics
Expand Down Expand Up @@ -119,6 +127,9 @@ impl<'a> Resolver<'a> {
def_maps: &'a BTreeMap<CrateId, CrateDefMap>,
file: FileId,
) -> Resolver<'a> {
let module_id = path_resolver.module_id();
let in_contract = module_id.module(def_maps).is_contract;

Self {
path_resolver,
def_maps,
Expand All @@ -131,6 +142,7 @@ impl<'a> Resolver<'a> {
errors: Vec::new(),
lambda_stack: Vec::new(),
file,
in_contract,
}
}

Expand Down Expand Up @@ -805,17 +817,24 @@ impl<'a> Resolver<'a> {
}
}

/// Override whether this name resolver is within a contract or not.
/// This will affect which types are allowed as parameters to methods as well
/// as which modifiers are allowed on a function.
pub(crate) fn set_in_contract(&mut self, in_contract: bool) {
self.in_contract = in_contract;
}

/// True if the 'pub' keyword is allowed on parameters in this function
fn pub_allowed(&self, func: &NoirFunction) -> bool {
if self.in_contract() {
if self.in_contract {
!func.def.is_unconstrained
} else {
func.name() == MAIN_FUNCTION
}
}

fn is_entry_point_function(&self, func: &NoirFunction) -> bool {
if self.in_contract() {
if self.in_contract {
func.attributes().is_contract_entry_point()
} else {
func.name() == MAIN_FUNCTION
Expand All @@ -824,7 +843,7 @@ impl<'a> Resolver<'a> {

/// True if the `distinct` keyword is allowed on a function's return type
fn distinct_allowed(&self, func: &NoirFunction) -> bool {
if self.in_contract() {
if self.in_contract {
// "open" and "unconstrained" functions are compiled to brillig and thus duplication of
// witness indices in their abis is not a concern.
!func.def.is_unconstrained && !func.def.is_open
Expand All @@ -836,15 +855,15 @@ impl<'a> Resolver<'a> {
fn handle_function_type(&mut self, function: &FuncId) {
let function_type = self.interner.function_modifiers(function).contract_function_type;

if !self.in_contract() && function_type == Some(ContractFunctionType::Open) {
if !self.in_contract && function_type == Some(ContractFunctionType::Open) {
let span = self.interner.function_ident(function).span();
self.errors.push(ResolverError::ContractFunctionTypeInNormalFunction { span });
self.interner.function_modifiers_mut(function).contract_function_type = None;
}
}

fn handle_is_function_internal(&mut self, function: &FuncId) {
if !self.in_contract() {
if !self.in_contract {
if self.interner.function_modifiers(function).is_internal == Some(true) {
let span = self.interner.function_ident(function).span();
self.push_err(ResolverError::ContractFunctionInternalInNormalFunction { span });
Expand Down Expand Up @@ -1605,11 +1624,6 @@ impl<'a> Resolver<'a> {
}
}

fn in_contract(&self) -> bool {
let module_id = self.path_resolver.module_id();
module_id.module(self.def_maps).is_contract
}

fn resolve_fmt_str_literal(&mut self, str: String, call_expr_span: Span) -> HirLiteral {
let re = Regex::new(r"\{([a-zA-Z0-9_]+)\}")
.expect("ICE: an invalid regex pattern was used for checking format strings");
Expand Down
10 changes: 0 additions & 10 deletions compiler/wasm/build.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,4 @@
const GIT_COMMIT: &&str = &"GIT_COMMIT";

fn main() {
// Only use build_data if the environment variable isn't set
// The environment variable is always set when working via Nix
if std::env::var(GIT_COMMIT).is_err() {
build_data::set_GIT_COMMIT();
build_data::set_GIT_DIRTY();
build_data::no_debug_rebuilds();
}

build_data::set_SOURCE_TIMESTAMP();
build_data::no_debug_rebuilds();
}
4 changes: 3 additions & 1 deletion compiler/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use nargo::artifacts::{
};
use noirc_driver::{
add_dep, compile_contract, compile_main, prepare_crate, prepare_dependency, CompileOptions,
CompiledContract, CompiledProgram,
CompiledContract, CompiledProgram, NOIR_ARTIFACT_VERSION_STRING,
};
use noirc_frontend::{graph::CrateGraph, hir::Context};
use std::path::Path;
Expand Down Expand Up @@ -118,6 +118,7 @@ fn preprocess_program(program: CompiledProgram) -> PreprocessedProgram {
hash: program.hash,
backend: String::from(BACKEND_IDENTIFIER),
abi: program.abi,
noir_version: NOIR_ARTIFACT_VERSION_STRING.to_string(),
bytecode: program.circuit,
}
}
Expand All @@ -136,6 +137,7 @@ fn preprocess_contract(contract: CompiledContract) -> PreprocessedContract {
.collect();

PreprocessedContract {
noir_version: String::from(NOIR_ARTIFACT_VERSION_STRING),
name: contract.name,
backend: String::from(BACKEND_IDENTIFIER),
functions: preprocessed_functions,
Expand Down
8 changes: 3 additions & 5 deletions compiler/wasm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ use getrandom as _;

use gloo_utils::format::JsValueSerdeExt;
use log::Level;
use noirc_driver::{GIT_COMMIT, GIT_DIRTY, NOIRC_VERSION};
use serde::{Deserialize, Serialize};
use std::str::FromStr;
use wasm_bindgen::prelude::*;
Expand Down Expand Up @@ -37,11 +38,8 @@ pub fn init_log_level(level: String) {
});
}

const BUILD_INFO: BuildInfo = BuildInfo {
git_hash: env!("GIT_COMMIT"),
version: env!("CARGO_PKG_VERSION"),
dirty: env!("GIT_DIRTY"),
};
const BUILD_INFO: BuildInfo =
BuildInfo { git_hash: GIT_COMMIT, version: NOIRC_VERSION, dirty: GIT_DIRTY };

#[wasm_bindgen]
pub fn build_info() -> JsValue {
Expand Down
10 changes: 6 additions & 4 deletions docs/docs/language_concepts/01_functions.md
Original file line number Diff line number Diff line change
Expand Up @@ -148,19 +148,21 @@ See [Lambdas](./08_lambdas.md) for more details.
Attributes are metadata that can be applied to a function, using the following syntax: `#[attribute(value)]`.

Supported attributes include:

- **builtin**: the function is implemented by the compiler, for efficiency purposes.
- **deprecated**: mark the function as *deprecated*. Calling the function will generate a warning: `warning: use of deprecated function`
- **deprecated**: mark the function as _deprecated_. Calling the function will generate a warning: `warning: use of deprecated function`
- **field**: Used to enable conditional compilation of code depending on the field size. See below for more details
- **oracle**: mark the function as *oracle*; meaning it is an external unconstrained function, implemented in noir_js. See [Unconstrained](./05_unconstrained.md) and [Noir js](../noir_js/noir_js.md) for more details.
- **oracle**: mark the function as _oracle_; meaning it is an external unconstrained function, implemented in noir_js. See [Unconstrained](./05_unconstrained.md) and [NoirJS](../noir_js/noir_js.md) for more details.
- **test**: mark the function as unit tests. See [Tests](../nargo/02_testing.md) for more details

### Field Attribute

The field attribute defines which field the function is compatible for. The function is conditionally compiled, under the condition that the field attribute matches the Noir native field.
The field can be defined implicitly, by using the name of the elliptic curve usually associated to it - for instance bn254, bls12_381 - or explicitly by using the field (prime) order, in decimal or hexadecimal form.
As a result, it is possible to define multiple versions of a function with each version specialized for a different field attribute. This can be useful when a function requires different parameters depending on the underlying elliptic curve.


Example: we define the function `foo()` three times below. Once for the default Noir bn254 curve, once for the field $\mathbb F_{23}$, which will normally never be used by Noir, and once again for the bls12_381 curve.

```rust
#[field(bn254)]
fn foo() -> u32 {
Expand All @@ -184,4 +186,4 @@ fn foo() -> u32 {
}
```

If the field name is not known to Noir, it will discard the function. Field names are case insensitive.
If the field name is not known to Noir, it will discard the function. Field names are case insensitive.
Loading

0 comments on commit 0dc9608

Please sign in to comment.