Skip to content

Commit

Permalink
feat(driver): Remove Driver struct and refactor functions to take `…
Browse files Browse the repository at this point in the history
…Context` (#1867)

feat(driver): Remove Driver struct and refactor functions to take context
  • Loading branch information
phated authored Jul 6, 2023
1 parent 38e2c18 commit 8895853
Show file tree
Hide file tree
Showing 13 changed files with 529 additions and 529 deletions.
81 changes: 39 additions & 42 deletions crates/lsp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use std::{
future::Future,
ops::{self, ControlFlow},
pin::Pin,
task::{Context, Poll},
task::{self, Poll},
};

use async_lsp::{
Expand All @@ -17,24 +17,25 @@ use lsp_types::{
InitializeParams, InitializeResult, InitializedParams, Position, PublishDiagnosticsParams,
Range, ServerCapabilities, TextDocumentSyncOptions,
};
use noirc_driver::Driver;
use noirc_driver::{check_crate, create_local_crate};
use noirc_errors::{DiagnosticKind, FileDiagnostic};
use noirc_frontend::graph::CrateType;
use noirc_frontend::{graph::CrateType, hir::Context};
use serde_json::Value as JsonValue;
use tower::Service;

const TEST_COMMAND: &str = "nargo.test";
const TEST_CODELENS_TITLE: &str = "▶\u{fe0e} Run Test";

// State for the LSP gets implemented on this struct and is internal to the implementation
#[derive(Debug)]
struct LspState {
context: Context,
client: ClientSocket,
}

impl LspState {
fn new(client: &ClientSocket) -> Self {
Self { client: client.clone() }
// TODO: Do we want to build the Context here or when we get the initialize message?
Self { client: client.clone(), context: Context::default() }
}
}

Expand Down Expand Up @@ -68,7 +69,7 @@ impl Service<AnyRequest> for NargoLspService {
type Error = ResponseError;
type Future = Pin<Box<dyn Future<Output = Result<Self::Response, Self::Error>> + Send>>;

fn poll_ready(&mut self, cx: &mut Context<'_>) -> Poll<Result<(), Self::Error>> {
fn poll_ready(&mut self, cx: &mut task::Context<'_>) -> Poll<Result<(), Self::Error>> {
self.router.poll_ready(cx)
}

Expand Down Expand Up @@ -129,49 +130,47 @@ fn on_shutdown(
}

fn on_code_lens_request(
_state: &mut LspState,
state: &mut LspState,
params: CodeLensParams,
) -> impl Future<Output = Result<Option<Vec<CodeLens>>, ResponseError>> {
async move {
let mut driver = Driver::new();
let file_path = &params.text_document.uri.to_file_path().unwrap();

let file_path = &params.text_document.uri.to_file_path().unwrap();
let crate_id = create_local_crate(&mut state.context, file_path, CrateType::Binary);

driver.create_local_crate(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
let _ = check_crate(&mut state.context, false);

// We ignore the warnings and errors produced by compilation for producing codelenses
// because we can still get the test functions even if compilation fails
let _ = driver.check_crate(false);
let fm = &state.context.file_manager;
let files = fm.as_simple_files();
let tests = state.context.get_all_test_functions_in_crate_matching(&crate_id, "");

let fm = driver.file_manager();
let files = fm.as_simple_files();
let tests = driver.get_all_test_functions_in_crate_matching("");

let mut lenses: Vec<CodeLens> = vec![];
for func_id in tests {
let location = driver.function_meta(&func_id).name.location;
let file_id = location.file;
// TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file
if file_id.as_usize() != 0 {
continue;
}
let mut lenses: Vec<CodeLens> = vec![];
for func_id in tests {
let location = state.context.function_meta(&func_id).name.location;
let file_id = location.file;
// TODO(#1681): This file_id never be 0 because the "path" where it maps is the directory, not a file
if file_id.as_usize() != 0 {
continue;
}

let func_name = driver.function_name(func_id);
let func_name = state.context.function_name(&func_id);

let range = byte_span_to_range(files, file_id.as_usize(), location.span.into())
.unwrap_or_default();
let range =
byte_span_to_range(files, file_id.as_usize(), location.span.into()).unwrap_or_default();

let command = Command {
title: TEST_CODELENS_TITLE.into(),
command: TEST_COMMAND.into(),
arguments: Some(vec![func_name.into()]),
};
let command = Command {
title: TEST_CODELENS_TITLE.into(),
command: TEST_COMMAND.into(),
arguments: Some(vec![func_name.into()]),
};

let lens = CodeLens { range, command: command.into(), data: None };
let lens = CodeLens { range, command: command.into(), data: None };

lenses.push(lens);
}
lenses.push(lens);
}

async move {
if lenses.is_empty() {
Ok(None)
} else {
Expand Down Expand Up @@ -219,21 +218,19 @@ fn on_did_save_text_document(
state: &mut LspState,
params: DidSaveTextDocumentParams,
) -> ControlFlow<Result<(), async_lsp::Error>> {
let mut driver = Driver::new();

let file_path = &params.text_document.uri.to_file_path().unwrap();

driver.create_local_crate(file_path, CrateType::Binary);
create_local_crate(&mut state.context, file_path, CrateType::Binary);

let mut diagnostics = Vec::new();

let file_diagnostics = match driver.check_crate(false) {
let file_diagnostics = match check_crate(&mut state.context, false) {
Ok(warnings) => warnings,
Err(errors_and_warnings) => errors_and_warnings,
};

if !file_diagnostics.is_empty() {
let fm = driver.file_manager();
let fm = &state.context.file_manager;
let files = fm.as_simple_files();

for FileDiagnostic { file_id, diagnostic } in file_diagnostics {
Expand Down
17 changes: 9 additions & 8 deletions crates/nargo_cli/src/cli/check_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
use crate::{errors::CliError, resolver::Resolver};
use crate::{errors::CliError, resolver::resolve_root_manifest};
use acvm::Backend;
use clap::Args;
use iter_extended::btree_map;
use noirc_abi::{AbiParameter, AbiType, MAIN_RETURN_NAME};
use noirc_driver::CompileOptions;
use noirc_driver::{check_crate, compute_function_signature, CompileOptions};
use noirc_errors::reporter::ReportedErrors;
use noirc_frontend::hir::Context;
use std::path::{Path, PathBuf};

use super::fs::write_to_file;
Expand Down Expand Up @@ -35,11 +36,11 @@ fn check_from_path<B: Backend>(
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut driver = Resolver::resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut driver, compile_options.deny_warnings)?;
let mut context = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings)?;

// XXX: We can have a --overwrite flag to determine if you want to overwrite the Prover/Verifier.toml files
if let Some((parameters, return_type)) = driver.compute_function_signature() {
if let Some((parameters, return_type)) = compute_function_signature(&context) {
// XXX: The root config should return an enum to determine if we are looking for .json or .toml
// For now it is hard-coded to be toml.
//
Expand Down Expand Up @@ -211,9 +212,9 @@ d2 = ["", "", ""]
/// Run the lexing, parsing, name resolution, and type checking passes and report any warnings
/// and errors found.
pub(crate) fn check_crate_and_report_errors(
driver: &mut noirc_driver::Driver,
context: &mut Context,
deny_warnings: bool,
) -> Result<(), ReportedErrors> {
let result = driver.check_crate(deny_warnings).map(|warnings| ((), warnings));
super::compile_cmd::report_errors(result, driver, deny_warnings)
let result = check_crate(context, deny_warnings).map(|warnings| ((), warnings));
super::compile_cmd::report_errors(result, context, deny_warnings)
}
27 changes: 16 additions & 11 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
use acvm::Backend;
use iter_extended::try_vecmap;
use nargo::artifacts::contract::PreprocessedContract;
use noirc_driver::{CompileOptions, CompiledProgram, Driver, ErrorsAndWarnings, Warnings};
use noirc_driver::{
compile_contracts, compile_main, CompileOptions, CompiledProgram, ErrorsAndWarnings, Warnings,
};
use noirc_errors::reporter::ReportedErrors;
use noirc_frontend::hir::Context;
use std::path::Path;

use clap::Args;

use nargo::ops::{preprocess_contract_function, preprocess_program};

use crate::{constants::TARGET_DIR, errors::CliError, resolver::Resolver};
use crate::{constants::TARGET_DIR, errors::CliError, resolver::resolve_root_manifest};

use super::fs::{
common_reference_string::{
Expand Down Expand Up @@ -48,14 +51,15 @@ pub(crate) fn run<B: Backend>(

// If contracts is set we're compiling every function in a 'contract' rather than just 'main'.
if args.contracts {
let mut driver = Resolver::resolve_root_manifest(&config.program_dir)?;
let mut context = resolve_root_manifest(&config.program_dir)?;

let result = driver.compile_contracts(
let result = compile_contracts(
&mut context,
backend.np_language(),
&|op| backend.supports_opcode(op),
&args.compile_options,
);
let contracts = report_errors(result, &driver, args.compile_options.deny_warnings)?;
let contracts = report_errors(result, &context, args.compile_options.deny_warnings)?;

// TODO(#1389): I wonder if it is incorrect for nargo-core to know anything about contracts.
// As can be seen here, It seems like a leaky abstraction where ContractFunctions (essentially CompiledPrograms)
Expand Down Expand Up @@ -109,26 +113,27 @@ pub(crate) fn compile_circuit<B: Backend>(
program_dir: &Path,
compile_options: &CompileOptions,
) -> Result<CompiledProgram, CliError<B>> {
let mut driver = Resolver::resolve_root_manifest(program_dir)?;
let result = driver.compile_main(
let mut context = resolve_root_manifest(program_dir)?;
let result = compile_main(
&mut context,
backend.np_language(),
&|op| backend.supports_opcode(op),
compile_options,
);
report_errors(result, &driver, compile_options.deny_warnings).map_err(Into::into)
report_errors(result, &context, compile_options.deny_warnings).map_err(Into::into)
}

/// Helper function for reporting any errors in a Result<(T, Warnings), ErrorsAndWarnings>
/// structure that is commonly used as a return result in this file.
pub(crate) fn report_errors<T>(
result: Result<(T, Warnings), ErrorsAndWarnings>,
driver: &Driver,
context: &Context,
deny_warnings: bool,
) -> Result<T, ReportedErrors> {
let (t, warnings) = result.map_err(|errors| {
noirc_errors::reporter::report_all(driver.file_manager(), &errors, deny_warnings)
noirc_errors::reporter::report_all(&context.file_manager, &errors, deny_warnings)
})?;

noirc_errors::reporter::report_all(driver.file_manager(), &warnings, deny_warnings);
noirc_errors::reporter::report_all(&context.file_manager, &warnings, deny_warnings);
Ok(t)
}
12 changes: 6 additions & 6 deletions crates/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -117,9 +117,9 @@ pub fn prove_and_verify(program_dir: &Path, experimental_ssa: bool) -> bool {
// FIXME: I not sure that this is the right place for this tests.
#[cfg(test)]
mod tests {
use noirc_driver::Driver;
use noirc_driver::{check_crate, create_local_crate};
use noirc_errors::reporter;
use noirc_frontend::graph::CrateType;
use noirc_frontend::{graph::CrateType, hir::Context};

use std::path::{Path, PathBuf};

Expand All @@ -129,18 +129,18 @@ mod tests {
///
/// This is used for tests.
fn file_compiles<P: AsRef<Path>>(root_file: P) -> bool {
let mut driver = Driver::new();
driver.create_local_crate(&root_file, CrateType::Binary);
let mut context = Context::default();
create_local_crate(&mut context, &root_file, CrateType::Binary);

let result = driver.check_crate(false);
let result = check_crate(&mut context, false);
let success = result.is_ok();

let errors = match result {
Ok(warnings) => warnings,
Err(errors) => errors,
};

reporter::report_all(driver.file_manager(), &errors, false);
reporter::report_all(&context.file_manager, &errors, false);
success
}

Expand Down
28 changes: 16 additions & 12 deletions crates/nargo_cli/src/cli/test_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,14 @@ use std::{io::Write, path::Path};
use acvm::{acir::native_types::WitnessMap, Backend};
use clap::Args;
use nargo::ops::execute_circuit;
use noirc_driver::{CompileOptions, Driver};
use noirc_frontend::node_interner::FuncId;
use noirc_driver::{compile_no_check, CompileOptions};
use noirc_frontend::{graph::LOCAL_CRATE, hir::Context, node_interner::FuncId};
use termcolor::{Color, ColorChoice, ColorSpec, StandardStream, WriteColor};

use crate::{cli::check_cmd::check_crate_and_report_errors, errors::CliError, resolver::Resolver};
use crate::{
cli::check_cmd::check_crate_and_report_errors, errors::CliError,
resolver::resolve_root_manifest,
};

use super::NargoConfig;

Expand Down Expand Up @@ -37,22 +40,22 @@ fn run_tests<B: Backend>(
test_name: &str,
compile_options: &CompileOptions,
) -> Result<(), CliError<B>> {
let mut driver = Resolver::resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut driver, compile_options.deny_warnings)?;
let mut context = resolve_root_manifest(program_dir)?;
check_crate_and_report_errors(&mut context, compile_options.deny_warnings)?;

let test_functions = driver.get_all_test_functions_in_crate_matching(test_name);
let test_functions = context.get_all_test_functions_in_crate_matching(&LOCAL_CRATE, test_name);
println!("Running {} test functions...", test_functions.len());
let mut failing = 0;

let writer = StandardStream::stderr(ColorChoice::Always);
let mut writer = writer.lock();

for test_function in test_functions {
let test_name = driver.function_name(test_function);
let test_name = context.function_name(&test_function);
writeln!(writer, "Testing {test_name}...").expect("Failed to write to stdout");
writer.flush().ok();

match run_test(backend, test_name, test_function, &driver, compile_options) {
match run_test(backend, test_name, test_function, &context, compile_options) {
Ok(_) => {
writer.set_color(ColorSpec::new().set_fg(Some(Color::Green))).ok();
writeln!(writer, "ok").ok();
Expand All @@ -79,12 +82,13 @@ fn run_test<B: Backend>(
backend: &B,
test_name: &str,
main: FuncId,
driver: &Driver,
context: &Context,
config: &CompileOptions,
) -> Result<(), CliError<B>> {
let program = driver
.compile_no_check(config, main, backend.np_language(), &|op| backend.supports_opcode(op))
.map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?;
let program = compile_no_check(context, config, main, backend.np_language(), &|op| {
backend.supports_opcode(op)
})
.map_err(|_| CliError::Generic(format!("Test '{test_name}' failed to compile")))?;

// Run the backend to ensure the PWG evaluates functions like std::hash::pedersen,
// otherwise constraints involving these expressions will not error.
Expand Down
Loading

0 comments on commit 8895853

Please sign in to comment.