Skip to content

Commit

Permalink
Merge branch 'master' into remove-backend-solver
Browse files Browse the repository at this point in the history
* master:
  fix: Require package names to be non-empty (#2293)
  fix(nargo)!: Remove `-p` short flag from the `--program-dir` flag (#2300)
  feat: optionally output a debug artifact on compile (#2260)
  • Loading branch information
TomAFrench committed Aug 14, 2023
2 parents f0f3c19 + 87174ac commit ecfba96
Show file tree
Hide file tree
Showing 26 changed files with 259 additions and 70 deletions.
4 changes: 3 additions & 1 deletion crates/fm/src/file_map.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,9 @@ impl From<&PathBuf> for PathString {
pub struct FileMap(SimpleFiles<PathString, String>);

// XXX: Note that we derive Default here due to ModuleOrigin requiring us to set a FileId
#[derive(Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize)]
#[derive(
Default, Debug, Clone, PartialEq, Eq, Copy, Hash, Serialize, Deserialize, PartialOrd, Ord,
)]
pub struct FileId(usize);

impl FileId {
Expand Down
2 changes: 1 addition & 1 deletion crates/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ impl FileManager {
assert!(old_value.is_none(), "ice: the same path was inserted into the file manager twice");
}

pub fn fetch_file(&mut self, file_id: FileId) -> File {
pub fn fetch_file(&self, file_id: FileId) -> File {
// Unwrap as we ensure that all file_id's map to a corresponding file in the file map
self.file_map.get_file(file_id).unwrap()
}
Expand Down
50 changes: 50 additions & 0 deletions crates/nargo/src/artifacts/debug.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
use serde::{Deserialize, Serialize};
use std::{
collections::{BTreeMap, BTreeSet},
path::PathBuf,
};

use fm::FileId;
use noirc_errors::debug_info::DebugInfo;
use noirc_frontend::hir::Context;

/// For a given file, we store the source code and the path to the file
/// so consumers of the debug artifact can reconstruct the original source code structure.
#[derive(Debug, Serialize, Deserialize)]
pub struct DebugFile {
pub source: String,
pub path: PathBuf,
}

/// A Debug Artifact stores, for a given program, the debug info for every function
/// along with a map of file Id to the source code so locations in debug info can be mapped to source code they point to.
#[derive(Debug, Serialize, Deserialize)]
pub struct DebugArtifact {
pub debug_symbols: Vec<DebugInfo>,
pub file_map: BTreeMap<FileId, DebugFile>,
}

impl DebugArtifact {
pub fn new(debug_symbols: Vec<DebugInfo>, compilation_context: &Context) -> Self {
let mut file_map = BTreeMap::new();

let files_with_debug_symbols: BTreeSet<FileId> = debug_symbols
.iter()
.flat_map(|function_symbols| function_symbols.locations.values().map(|loc| loc.file))
.collect();

for file_id in files_with_debug_symbols {
let file_source = compilation_context.file_manager.fetch_file(file_id).source();

file_map.insert(
file_id,
DebugFile {
source: file_source.to_string(),
path: compilation_context.file_manager.path(file_id).to_path_buf(),
},
);
}

Self { debug_symbols, file_map }
}
}
1 change: 1 addition & 0 deletions crates/nargo/src/artifacts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ use base64::Engine;
use serde::{Deserializer, Serializer};

pub mod contract;
pub mod debug;
pub mod program;

// TODO: move these down into ACVM.
Expand Down
23 changes: 13 additions & 10 deletions crates/nargo/src/ops/preprocess.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
include_keys: bool,
common_reference_string: &[u8],
func: ContractFunction,
) -> Result<PreprocessedContractFunction, B::Error> {
) -> Result<(PreprocessedContractFunction, DebugInfo), B::Error> {
// TODO: currently `func`'s bytecode is already optimized for the backend.
// In future we'll need to apply those optimizations here.
let optimized_bytecode = func.bytecode;
Expand All @@ -54,14 +54,17 @@ pub fn preprocess_contract_function<B: ProofSystemCompiler>(
(None, None)
};

Ok(PreprocessedContractFunction {
name: func.name,
function_type: func.function_type,
is_internal: func.is_internal,
abi: func.abi,
Ok((
PreprocessedContractFunction {
name: func.name,
function_type: func.function_type,
is_internal: func.is_internal,
abi: func.abi,

bytecode: optimized_bytecode,
proving_key,
verification_key,
})
bytecode: optimized_bytecode,
proving_key,
verification_key,
},
func.debug,
))
}
97 changes: 68 additions & 29 deletions crates/nargo_cli/src/cli/compile_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,16 @@ use acvm::acir::circuit::OpcodeLabel;
use acvm::{acir::circuit::Circuit, Backend};
use iter_extended::try_vecmap;
use iter_extended::vecmap;
use nargo::artifacts::debug::DebugArtifact;
use nargo::package::Package;
use nargo::prepare_package;
use nargo::{artifacts::contract::PreprocessedContract, NargoError};
use nargo_toml::{find_package_manifest, resolve_workspace_from_toml};
use noirc_driver::{
compile_contracts, compile_main, CompileOptions, CompiledContract, CompiledProgram,
ContractFunction, ErrorsAndWarnings, Warnings,
ErrorsAndWarnings, Warnings,
};
use noirc_errors::debug_info::DebugInfo;
use noirc_frontend::graph::CrateName;
use noirc_frontend::hir::Context;

Expand All @@ -19,6 +21,7 @@ use nargo::ops::{preprocess_contract_function, preprocess_program};

use crate::errors::{CliError, CompileError};

use super::fs::program::save_debug_artifact_to_file;
use super::fs::{
common_reference_string::{
read_cached_common_reference_string, update_common_reference_string,
Expand All @@ -38,6 +41,10 @@ pub(crate) struct CompileCommand {
#[arg(long)]
include_keys: bool,

/// Output debug files
#[arg(long, hide = true)]
output_debug: bool,

/// The name of the package to compile
#[clap(long)]
package: Option<CrateName>,
Expand Down Expand Up @@ -70,49 +77,72 @@ pub(crate) fn run<B: Backend>(
// As can be seen here, It seems like a leaky abstraction where ContractFunctions (essentially CompiledPrograms)
// are compiled via nargo-core and then the PreprocessedContract is constructed here.
// This is due to EACH function needing it's own CRS, PKey, and VKey from the backend.
let preprocessed_contracts: Result<Vec<PreprocessedContract>, CliError<B>> =
try_vecmap(optimized_contracts, |contract| {
let preprocessed_contract_functions = try_vecmap(contract.functions, |func| {
common_reference_string = update_common_reference_string(
backend,
&common_reference_string,
&func.bytecode,
)
.map_err(CliError::CommonReferenceStringError)?;

preprocess_contract_function(
backend,
args.include_keys,
&common_reference_string,
func,
)
.map_err(CliError::ProofSystemCompilerError)
})?;

Ok(PreprocessedContract {
let preprocessed_contracts: Result<
Vec<(PreprocessedContract, Vec<DebugInfo>)>,
CliError<B>,
> = try_vecmap(optimized_contracts, |contract| {
let preprocess_result = try_vecmap(contract.functions, |func| {
common_reference_string = update_common_reference_string(
backend,
&common_reference_string,
&func.bytecode,
)
.map_err(CliError::CommonReferenceStringError)?;

preprocess_contract_function(
backend,
args.include_keys,
&common_reference_string,
func,
)
.map_err(CliError::ProofSystemCompilerError)
})?;

let (preprocessed_contract_functions, debug_infos): (Vec<_>, Vec<_>) =
preprocess_result.into_iter().unzip();

Ok((
PreprocessedContract {
name: contract.name,
backend: String::from(BACKEND_IDENTIFIER),
functions: preprocessed_contract_functions,
})
});
for contract in preprocessed_contracts? {
},
debug_infos,
))
});
for (contract, debug_infos) in preprocessed_contracts? {
save_contract_to_file(
&contract,
&format!("{}-{}", package.name, contract.name),
&circuit_dir,
);

if args.output_debug {
let debug_artifact = DebugArtifact::new(debug_infos, &context);
save_debug_artifact_to_file(
&debug_artifact,
&format!("{}-{}", package.name, contract.name),
&circuit_dir,
);
}
}
} else {
let (_, program) = compile_package(backend, package, &args.compile_options)?;
let (context, program) = compile_package(backend, package, &args.compile_options)?;

common_reference_string =
update_common_reference_string(backend, &common_reference_string, &program.circuit)
.map_err(CliError::CommonReferenceStringError)?;

let (preprocessed_program, _) =
let (preprocessed_program, debug_info) =
preprocess_program(backend, args.include_keys, &common_reference_string, program)
.map_err(CliError::ProofSystemCompilerError)?;
save_program_to_file(&preprocessed_program, &package.name, &circuit_dir);

if args.output_debug {
let debug_artifact = DebugArtifact::new(vec![debug_info], &context);
let circuit_name: String = (&package.name).into();
save_debug_artifact_to_file(&debug_artifact, &circuit_name, &circuit_dir);
}
}
}

Expand Down Expand Up @@ -167,9 +197,18 @@ pub(super) fn optimize_contract<B: Backend>(
backend: &B,
contract: CompiledContract,
) -> Result<CompiledContract, CliError<B>> {
let functions = try_vecmap(contract.functions, |func| {
let optimized_bytecode = optimize_circuit(backend, func.bytecode)?.0;
Ok::<_, CliError<B>>(ContractFunction { bytecode: optimized_bytecode, ..func })
let functions = try_vecmap(contract.functions, |mut func| {
let (optimized_bytecode, opcode_labels) = optimize_circuit(backend, func.bytecode)?;
let opcode_ids = vecmap(opcode_labels, |label| match label {
OpcodeLabel::Unresolved => {
unreachable!("Compiled circuit opcodes must resolve to some index")
}
OpcodeLabel::Resolved(index) => index as usize,
});

func.bytecode = optimized_bytecode;
func.debug.update_acir(opcode_ids);
Ok::<_, CliError<B>>(func)
})?;

Ok(CompiledContract { functions, ..contract })
Expand Down
19 changes: 16 additions & 3 deletions crates/nargo_cli/src/cli/fs/program.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use std::path::{Path, PathBuf};

use nargo::artifacts::{contract::PreprocessedContract, program::PreprocessedProgram};
use nargo::artifacts::{
contract::PreprocessedContract, debug::DebugArtifact, program::PreprocessedProgram,
};
use noirc_frontend::graph::CrateName;

use crate::errors::FilesystemError;
Expand All @@ -15,20 +17,31 @@ pub(crate) fn save_program_to_file<P: AsRef<Path>>(
let circuit_name: String = crate_name.into();
save_build_artifact_to_file(compiled_program, &circuit_name, circuit_dir)
}

pub(crate) fn save_contract_to_file<P: AsRef<Path>>(
compiled_contract: &PreprocessedContract,
circuit_name: &str,
circuit_dir: P,
) -> PathBuf {
save_build_artifact_to_file(compiled_contract, circuit_name, circuit_dir)
}

pub(crate) fn save_debug_artifact_to_file<P: AsRef<Path>>(
debug_artifact: &DebugArtifact,
circuit_name: &str,
circuit_dir: P,
) -> PathBuf {
let artifact_name = format!("debug_{}", circuit_name);
save_build_artifact_to_file(debug_artifact, &artifact_name, circuit_dir)
}

fn save_build_artifact_to_file<P: AsRef<Path>, T: ?Sized + serde::Serialize>(
build_artifact: &T,
circuit_name: &str,
artifact_name: &str,
circuit_dir: P,
) -> PathBuf {
create_named_dir(circuit_dir.as_ref(), "target");
let circuit_path = circuit_dir.as_ref().join(circuit_name).with_extension("json");
let circuit_path = circuit_dir.as_ref().join(artifact_name).with_extension("json");

write_to_file(&serde_json::to_vec(build_artifact).unwrap(), &circuit_path);

Expand Down
17 changes: 11 additions & 6 deletions crates/nargo_cli/src/cli/init_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,15 @@ use acvm::Backend;
use clap::Args;
use nargo::constants::{PKG_FILE, SRC_DIR};
use nargo::package::PackageType;
use noirc_frontend::graph::CrateName;
use std::path::PathBuf;

/// Create a Noir project in the current directory.
#[derive(Debug, Clone, Args)]
pub(crate) struct InitCommand {
/// Name of the package [default: current directory name]
#[clap(long)]
name: Option<String>,
name: Option<CrateName>,

/// Use a library template
#[arg(long, conflicts_with = "bin", conflicts_with = "contract")]
Expand Down Expand Up @@ -67,9 +68,13 @@ pub(crate) fn run<B: Backend>(
args: InitCommand,
config: NargoConfig,
) -> Result<(), CliError<B>> {
let package_name = args
.name
.unwrap_or_else(|| config.program_dir.file_name().unwrap().to_str().unwrap().to_owned());
let package_name = match args.name {
Some(name) => name,
None => {
let name = config.program_dir.file_name().unwrap().to_str().unwrap();
name.parse().map_err(|_| CliError::InvalidPackageName(name.into()))?
}
};

let package_type = if args.lib {
PackageType::Library
Expand All @@ -78,14 +83,14 @@ pub(crate) fn run<B: Backend>(
} else {
PackageType::Binary
};
initialize_project(config.program_dir, &package_name, package_type);
initialize_project(config.program_dir, package_name, package_type);
Ok(())
}

/// Initializes a new Noir project in `package_dir`.
pub(crate) fn initialize_project(
package_dir: PathBuf,
package_name: &str,
package_name: CrateName,
package_type: PackageType,
) {
let src_dir = package_dir.join(SRC_DIR);
Expand Down
2 changes: 1 addition & 1 deletion crates/nargo_cli/src/cli/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ struct NargoCli {
#[derive(Args, Clone, Debug)]
pub(crate) struct NargoConfig {
// REMINDER: Also change this flag in the LSP test lens if renamed
#[arg(short, long, hide=true, global=true, default_value_os_t = std::env::current_dir().unwrap())]
#[arg(long, hide=true, global=true, default_value_os_t = std::env::current_dir().unwrap())]
program_dir: PathBuf,
}

Expand Down
Loading

0 comments on commit ecfba96

Please sign in to comment.