Skip to content

Commit

Permalink
Merge branch 'master' into boolean-and
Browse files Browse the repository at this point in the history
* master:
  fix: flattening pass no longer overwrites previously mapped condition values (#2117)
  chore(noirc_driver): Unify crate preparation (#2119)
  • Loading branch information
TomAFrench committed Aug 2, 2023
2 parents 41ca50d + f7742ab commit d445952
Show file tree
Hide file tree
Showing 10 changed files with 65 additions and 45 deletions.
6 changes: 3 additions & 3 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ use lsp_types::{
InitializeParams, InitializeResult, InitializedParams, Position, PublishDiagnosticsParams,
Range, ServerCapabilities, TextDocumentSyncOptions,
};
use noirc_driver::{check_crate, create_local_crate};
use noirc_driver::{check_crate, prepare_crate};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::{
graph::{CrateGraph, CrateType},
Expand Down Expand Up @@ -190,7 +190,7 @@ fn on_code_lens_request(
}
};

let crate_id = create_local_crate(&mut context, file_path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, file_path, CrateType::Binary);

// We ignore the warnings and errors produced by compilation for producing codelenses
// because we can still get the test functions even if compilation fails
Expand Down Expand Up @@ -283,7 +283,7 @@ fn on_did_save_text_document(
}
};

let crate_id = create_local_crate(&mut context, file_path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, file_path, CrateType::Binary);

let mut diagnostics = Vec::new();

Expand Down
7 changes: 3 additions & 4 deletions crates/lsp/src/lib_hacky.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ use lsp_types::{
InitializedParams, Position, PublishDiagnosticsParams, Range, ServerCapabilities,
TextDocumentSyncOptions,
};
use noirc_driver::{check_crate, create_local_crate, create_non_local_crate, propagate_dep};
use noirc_driver::{check_crate, prepare_crate, propagate_dep};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::{
graph::{CrateGraph, CrateId, CrateType},
Expand Down Expand Up @@ -286,7 +286,7 @@ fn create_context_at_path(
}
let nargo_toml_path = find_nearest_parent_file(&file_path, &["Nargo.toml"]);

let current_crate_id = create_local_crate(&mut context, &file_path, CrateType::Binary);
let current_crate_id = prepare_crate(&mut context, &file_path, CrateType::Binary);

// TODO(AD): undo hacky dependency resolution
if let Some(nargo_toml_path) = nargo_toml_path {
Expand All @@ -297,8 +297,7 @@ fn create_context_at_path(
.parent()
.unwrap() // TODO
.join(PathBuf::from(&dependency_path).join("src").join("lib.nr"));
let library_crate =
create_non_local_crate(&mut context, &path_to_lib, CrateType::Library);
let library_crate = prepare_crate(&mut context, &path_to_lib, CrateType::Library);
propagate_dep(&mut context, library_crate, &crate_name.parse().unwrap());
}
}
Expand Down
4 changes: 2 additions & 2 deletions crates/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ pub fn start_cli() -> eyre::Result<()> {
#[cfg(test)]
mod tests {
use fm::FileManager;
use noirc_driver::{check_crate, create_local_crate};
use noirc_driver::{check_crate, prepare_crate};
use noirc_errors::reporter;
use noirc_frontend::{
graph::{CrateGraph, CrateType},
Expand All @@ -110,7 +110,7 @@ mod tests {
let fm = FileManager::new(root_dir);
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);
let crate_id = create_local_crate(&mut context, root_file, CrateType::Binary);
let crate_id = prepare_crate(&mut context, root_file, CrateType::Binary);

let result = check_crate(&mut context, crate_id, false);
let success = result.is_ok();
Expand Down
7 changes: 3 additions & 4 deletions crates/nargo_cli/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
use fm::FileManager;
use nargo::package::{Dependency, Package};
use noirc_driver::{add_dep, create_local_crate, create_non_local_crate};
use noirc_driver::{add_dep, prepare_crate};
use noirc_frontend::{
graph::{CrateGraph, CrateId, CrateName, CrateType},
hir::Context,
Expand Down Expand Up @@ -107,8 +107,7 @@ fn prepare_dependencies(
for (dep_name, dep) in dependencies.into_iter() {
match dep {
Dependency::Remote { package } | Dependency::Local { package } => {
let crate_id =
create_non_local_crate(context, &package.entry_path, package.crate_type);
let crate_id = prepare_crate(context, &package.entry_path, package.crate_type);
add_dep(context, parent_crate, crate_id, dep_name);
prepare_dependencies(context, crate_id, package.dependencies.to_owned());
}
Expand All @@ -121,7 +120,7 @@ fn prepare_package(package: &Package) -> (Context, CrateId) {
let graph = CrateGraph::default();
let mut context = Context::new(fm, graph);

let crate_id = create_local_crate(&mut context, &package.entry_path, package.crate_type);
let crate_id = prepare_crate(&mut context, &package.entry_path, package.crate_type);

prepare_dependencies(&mut context, crate_id, package.dependencies.to_owned());

Expand Down
6 changes: 6 additions & 0 deletions crates/nargo_cli/tests/test_data/regression_2099/Nargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[package]
name = "regression_2099"
authors = [""]
compiler_version = "0.9.0"

[dependencies]
37 changes: 37 additions & 0 deletions crates/nargo_cli/tests/test_data/regression_2099/src/main.nr
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
use dep::std::ec::tecurve::affine::Curve as AffineCurve;
use dep::std::ec::tecurve::affine::Point as Gaffine;
use dep::std::ec::tecurve::curvegroup::Curve;
use dep::std::ec::tecurve::curvegroup::Point as G;

use dep::std::ec::swcurve::affine::Point as SWGaffine;
use dep::std::ec::swcurve::curvegroup::Point as SWG;

use dep::std::ec::montcurve::affine::Point as MGaffine;
use dep::std::ec::montcurve::curvegroup::Point as MG;

fn main() {
// Define Baby Jubjub (ERC-2494) parameters in affine representation
let bjj_affine = AffineCurve::new(168700, 168696, Gaffine::new(995203441582195749578291179787384436505546430278305826713579947235728471134,5472060717959818805561601436314318772137091100104008585924551046643952123905));

// Test addition
let p1_affine = Gaffine::new(17777552123799933955779906779655732241715742912184938656739573121738514868268, 2626589144620713026669568689430873010625803728049924121243784502389097019475);
let p2_affine = Gaffine::new(16540640123574156134436876038791482806971768689494387082833631921987005038935, 20819045374670962167435360035096875258406992893633759881276124905556507972311);
let _p3_affine = bjj_affine.add(p1_affine, p2_affine);

// Test SWCurve equivalents of the above
// First the affine representation
let bjj_swcurve_affine = bjj_affine.into_swcurve();

let p1_swcurve_affine = bjj_affine.map_into_swcurve(p1_affine);
let p2_swcurve_affine = bjj_affine.map_into_swcurve(p2_affine);

let _p3_swcurve_affine_from_add = bjj_swcurve_affine.add(
p1_swcurve_affine,
p2_swcurve_affine
);

// Check that these points are on the curve
assert(
bjj_swcurve_affine.contains(p1_swcurve_affine)
);
}
29 changes: 3 additions & 26 deletions crates/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,40 +52,17 @@ pub fn compile_file(
context: &mut Context,
root_file: &Path,
) -> Result<(CompiledProgram, Warnings), ErrorsAndWarnings> {
let crate_id = create_local_crate(context, root_file, CrateType::Binary);
let crate_id = prepare_crate(context, root_file, CrateType::Binary);
compile_main(context, crate_id, &CompileOptions::default())
}

/// Adds the File with the local crate root to the file system
/// and adds the local crate to the graph
/// XXX: This may pose a problem with workspaces, where you can change the local crate and where
/// we have multiple binaries in one workspace
/// A Fix would be for the driver instance to store the local crate id.
// Granted that this is the only place which relies on the local crate being first
pub fn create_local_crate(
context: &mut Context,
file_name: &Path,
crate_type: CrateType,
) -> CrateId {
/// Adds the file from the file system at `Path` to the crate graph
pub fn prepare_crate(context: &mut Context, file_name: &Path, crate_type: CrateType) -> CrateId {
let root_file_id = context.file_manager.add_file(file_name).unwrap();

context.crate_graph.add_crate_root(crate_type, root_file_id)
}

/// Creates a Non Local Crate. A Non Local Crate is any crate which is the not the crate that
/// the compiler is compiling.
pub fn create_non_local_crate(
context: &mut Context,
file_name: &Path,
crate_type: CrateType,
) -> CrateId {
let root_file_id = context.file_manager.add_file(file_name).unwrap();

// You can add any crate type to the crate graph
// but you cannot depend on Binaries
context.crate_graph.add_crate_root(crate_type, root_file_id)
}

/// Adds a edge in the crate graph for two crates
pub fn add_dep(
context: &mut Context,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ impl<'f> FunctionInserter<'f> {
let old_parameters = self.function.dfg.block_parameters(block);

for (param, new_param) in old_parameters.iter().zip(new_values) {
// Don't overwrite any existing entries to avoid overwriting the induction variable
self.values.entry(*param).or_insert(*new_param);
}
}
Expand Down
5 changes: 4 additions & 1 deletion crates/noirc_evaluator/src/ssa_refactor/opt/flatten_cfg.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,10 @@ impl<'f> Context<'f> {
// end, in addition to resetting the value of old_condition since it is set to
// known to be true/false within the then/else branch respectively.
self.insert_current_side_effects_enabled();
self.inserter.map_value(old_condition, old_condition);

// We must map back to `then_condition` here. Mapping `old_condition` to itself would
// lose any previous mappings.
self.inserter.map_value(old_condition, then_condition);

// While there is a condition on the stack we don't compile outside the condition
// until it is popped. This ensures we inline the full then and else branches
Expand Down
8 changes: 4 additions & 4 deletions crates/wasm/src/compile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ use fm::FileManager;
use gloo_utils::format::JsValueSerdeExt;
use log::debug;
use noirc_driver::{
check_crate, compile_contracts, compile_no_check, create_local_crate, create_non_local_crate,
propagate_dep, CompileOptions, CompiledContract,
check_crate, compile_contracts, compile_no_check, prepare_crate, propagate_dep, CompileOptions,
CompiledContract,
};
use noirc_frontend::{
graph::{CrateGraph, CrateType},
Expand Down Expand Up @@ -63,7 +63,7 @@ impl Default for WASMCompileOptions {

fn add_noir_lib(context: &mut Context, crate_name: &str) {
let path_to_lib = Path::new(&crate_name).join("lib.nr");
let library_crate = create_non_local_crate(context, &path_to_lib, CrateType::Library);
let library_crate = prepare_crate(context, &path_to_lib, CrateType::Library);

propagate_dep(context, library_crate, &crate_name.parse().unwrap());
}
Expand All @@ -87,7 +87,7 @@ pub fn compile(args: JsValue) -> JsValue {
let mut context = Context::new(fm, graph);

let path = Path::new(&options.entry_point);
let crate_id = create_local_crate(&mut context, path, CrateType::Binary);
let crate_id = prepare_crate(&mut context, path, CrateType::Binary);

for dependency in options.optional_dependencies_set {
add_noir_lib(&mut context, dependency.as_str());
Expand Down

0 comments on commit d445952

Please sign in to comment.