Skip to content

Commit

Permalink
fix: remove compiler_version from new Nargo.toml (noir-lang/noir#…
Browse files Browse the repository at this point in the history
…6590)

feat: Avoid incrementing reference counts in some cases (noir-lang/noir#6568)
chore: fix typo in test name (noir-lang/noir#6589)
fix: consider prereleases to be compatible with pre-1.0.0 releases (noir-lang/noir#6580)
feat: try to inline brillig calls with all constant arguments  (noir-lang/noir#6548)
fix: correct type when simplifying `derive_pedersen_generators` (noir-lang/noir#6579)
feat: Sync from aztec-packages (noir-lang/noir#6576)
  • Loading branch information
AztecBot committed Nov 22, 2024
2 parents 9c56e4d + 2505ae4 commit 8e90562
Show file tree
Hide file tree
Showing 10 changed files with 67 additions and 22 deletions.
2 changes: 1 addition & 1 deletion .noir-sync-commit
Original file line number Diff line number Diff line change
@@ -1 +1 @@
e4c66b91d42b20d17837fe5e7c32c9a83b6ab354
df8f2eee5c27d3cd4b6128056afdd9bd4a0322fe
16 changes: 10 additions & 6 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/mem2reg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,13 @@ impl<'f> PerFunctionContext<'f> {
let address = self.inserter.function.dfg.resolve(*address);
let value = self.inserter.function.dfg.resolve(*value);

// FIXME: This causes errors in the sha256 tests
//
// If there was another store to this instruction without any (unremoved) loads or
// function calls in-between, we can remove the previous store.
if let Some(last_store) = references.last_stores.get(&address) {
self.instructions_to_remove.insert(*last_store);
}
// if let Some(last_store) = references.last_stores.get(&address) {
// self.instructions_to_remove.insert(*last_store);
// }

if self.inserter.function.dfg.value_is_reference(value) {
if let Some(expression) = references.expressions.get(&value) {
Expand Down Expand Up @@ -899,12 +901,14 @@ mod tests {
// in the same block, and the store is not needed before the later store.
// The rest of the stores are also removed as no loads are done within any blocks
// to the stored values.
assert_eq!(count_stores(b1, &main.dfg), 0);
//
// NOTE: This store is not removed due to the FIXME when handling Instruction::Store.
assert_eq!(count_stores(b1, &main.dfg), 1);

let b1_instructions = main.dfg[b1].instructions();

// We expect the last eq to be optimized out
assert_eq!(b1_instructions.len(), 0);
// We expect the last eq to be optimized out, only the store from above remains
assert_eq!(b1_instructions.len(), 1);
}

#[test]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -172,6 +172,7 @@ impl<'a> FunctionContext<'a> {
/// Always returns a Value::Mutable wrapping the allocate instruction.
pub(super) fn new_mutable_variable(&mut self, value_to_store: ValueId) -> Value {
let element_type = self.builder.current_function.dfg.type_of_value(value_to_store);
self.builder.increment_array_reference_count(value_to_store);
let alloc = self.builder.insert_allocate(element_type);
self.builder.insert_store(alloc, value_to_store);
let typ = self.builder.type_of_value(value_to_store);
Expand Down Expand Up @@ -735,7 +736,6 @@ impl<'a> FunctionContext<'a> {
// Reference counting in brillig relies on us incrementing reference
// counts when arrays/slices are constructed or indexed.
// Thus, if we dereference an lvalue which happens to be array/slice we should increment its reference counter.
self.builder.increment_array_reference_count(reference);
self.builder.insert_load(reference, element_type).into()
})
}
Expand Down Expand Up @@ -916,7 +916,10 @@ impl<'a> FunctionContext<'a> {
let parameters = self.builder.current_function.dfg.block_parameters(entry).to_vec();

for parameter in parameters {
self.builder.increment_array_reference_count(parameter);
// Avoid reference counts for immutable arrays that aren't behind references.
if self.builder.current_function.dfg.value_is_reference(parameter) {
self.builder.increment_array_reference_count(parameter);
}
}

entry
Expand All @@ -933,7 +936,9 @@ impl<'a> FunctionContext<'a> {
dropped_parameters.retain(|parameter| !terminator_args.contains(parameter));

for parameter in dropped_parameters {
self.builder.decrement_array_reference_count(parameter);
if self.builder.current_function.dfg.value_is_reference(parameter) {
self.builder.decrement_array_reference_count(parameter);
}
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -665,12 +665,11 @@ impl<'a> FunctionContext<'a> {
values = values.map(|value| {
let value = value.eval(self);

// Make sure to increment array reference counts on each let binding
self.builder.increment_array_reference_count(value);

Tree::Leaf(if let_expr.mutable {
self.new_mutable_variable(value)
} else {
// `new_mutable_variable` already increments rcs internally
self.builder.increment_array_reference_count(value);
value::Value::Normal(value)
})
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
[package]
name = "brillig_unitialised_arrays"
name = "brillig_uninitialized_arrays"
type = "bin"
authors = [""]

Expand Down
2 changes: 0 additions & 2 deletions noir/noir-repo/tooling/nargo_cli/src/cli/init_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ use super::NargoConfig;
use clap::Args;
use nargo::constants::{PKG_FILE, SRC_DIR};
use nargo::package::{CrateName, PackageType};
use noirc_driver::NOIRC_VERSION;
use std::path::PathBuf;

/// Create a Noir project in the current directory.
Expand Down Expand Up @@ -66,7 +65,6 @@ pub(crate) fn initialize_project(
name = "{package_name}"
type = "{package_type}"
authors = [""]
compiler_version = ">={NOIRC_VERSION}"
[dependencies]"#
);
Expand Down
2 changes: 2 additions & 0 deletions noir/noir-repo/tooling/nargo_toml/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ pub enum ManifestError {
#[allow(clippy::enum_variant_names)]
#[derive(Error, Debug, PartialEq, Eq, Clone)]
pub enum SemverError {
#[error("Invalid value for `compiler_version` in package {package_name}. Requirements may only refer to full releases")]
InvalidCompilerVersionRequirement { package_name: CrateName, required_compiler_version: String },
#[error("Incompatible compiler version in package {package_name}. Required compiler version is {required_compiler_version} but the compiler version is {compiler_version_found}.\n Update the compiler_version field in Nargo.toml to >={required_compiler_version} or compile this project with version {required_compiler_version}")]
IncompatibleVersion {
package_name: CrateName,
Expand Down
49 changes: 43 additions & 6 deletions noir/noir-repo/tooling/nargo_toml/src/semver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ use nargo::{
package::{Dependency, Package},
workspace::Workspace,
};
use semver::{Error, Version, VersionReq};
use noirc_driver::CrateName;
use semver::{Error, Prerelease, Version, VersionReq};

// Parse a semver compatible version string
pub(crate) fn parse_semver_compatible_version(version: &str) -> Result<Version, Error> {
Version::parse(version)
let mut version = Version::parse(version)?;
version.pre = Prerelease::EMPTY;
Ok(version)
}

// Check that all of the packages in the workspace are compatible with the current compiler version
Expand All @@ -25,10 +28,7 @@ pub(crate) fn semver_check_workspace(
}

// Check that a package and all of its dependencies are compatible with the current compiler version
pub(crate) fn semver_check_package(
package: &Package,
compiler_version: &Version,
) -> Result<(), SemverError> {
fn semver_check_package(package: &Package, compiler_version: &Version) -> Result<(), SemverError> {
// Check that this package's compiler version requirements are satisfied
if let Some(version) = &package.compiler_required_version {
let version_req = match VersionReq::parse(version) {
Expand All @@ -40,6 +40,9 @@ pub(crate) fn semver_check_package(
})
}
};

validate_compiler_version_requirement(&package.name, &version_req)?;

if !version_req.matches(compiler_version) {
return Err(SemverError::IncompatibleVersion {
package_name: package.name.clone(),
Expand All @@ -61,6 +64,20 @@ pub(crate) fn semver_check_package(
Ok(())
}

fn validate_compiler_version_requirement(
package_name: &CrateName,
required_compiler_version: &VersionReq,
) -> Result<(), SemverError> {
if required_compiler_version.comparators.iter().any(|comparator| !comparator.pre.is_empty()) {
return Err(SemverError::InvalidCompilerVersionRequirement {
package_name: package_name.clone(),
required_compiler_version: required_compiler_version.to_string(),
});
}

Ok(())
}

// Strip the build meta data from the version string since it is ignored by semver.
fn strip_build_meta_data(version: &Version) -> String {
let version_string = version.to_string();
Expand Down Expand Up @@ -191,6 +208,26 @@ mod tests {
};
}

#[test]
fn test_semver_prerelease() {
let compiler_version = parse_semver_compatible_version("1.0.0-beta.0").unwrap();

let package = Package {
compiler_required_version: Some(">=0.1.0".to_string()),
root_dir: PathBuf::new(),
package_type: PackageType::Library,
entry_path: PathBuf::new(),
name: CrateName::from_str("test").unwrap(),
dependencies: BTreeMap::new(),
version: Some("1.0".to_string()),
expression_width: None,
};

if let Err(err) = semver_check_package(&package, &compiler_version) {
panic!("{err}");
};
}

#[test]
fn test_semver_build_data() {
let compiler_version = Version::parse("0.1.0+this-is-ignored-by-semver").unwrap();
Expand Down

0 comments on commit 8e90562

Please sign in to comment.