Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Instrument AST in debug mode to track variable state #3523

Closed
wants to merge 11 commits into from
1 change: 1 addition & 0 deletions compiler/noirc_errors/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,6 @@ codespan-reporting.workspace = true
codespan.workspace = true
fm.workspace = true
chumsky.workspace = true
noirc_printable_type.workspace = true
serde.workspace = true
serde_with = "3.2.0"
29 changes: 25 additions & 4 deletions compiler/noirc_errors/src/debug_info.rs
Original file line number Diff line number Diff line change
@@ -1,15 +1,20 @@
use acvm::acir::circuit::OpcodeLocation;
use acvm::compiler::AcirTransformationMap;
use fm::FileId;

use serde_with::serde_as;
use serde_with::DisplayFromStr;
use std::collections::BTreeMap;
use std::collections::HashMap;
use std::collections::{BTreeMap, HashMap};
use std::mem;

use crate::Location;
use noirc_printable_type::PrintableType;
use serde::{Deserialize, Serialize};

pub type Variables = Vec<(u32, (String, u32))>;
pub type Types = Vec<(u32, PrintableType)>;
pub type VariableTypes = (Variables, Types);

#[serde_as]
#[derive(Default, Debug, Clone, Deserialize, Serialize)]
pub struct DebugInfo {
Expand All @@ -18,6 +23,8 @@ pub struct DebugInfo {
/// that they should be serialized to/from strings.
#[serde_as(as = "BTreeMap<DisplayFromStr, _>")]
pub locations: BTreeMap<OpcodeLocation, Vec<Location>>,
pub variables: HashMap<u32, (String, u32)>, // var_id => (name, type_id)
pub types: HashMap<u32, PrintableType>, // type_id => printable type
Comment on lines +26 to +27
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note to self to look at how this acts when we're not in a debug context. Should we make this optional?

}

/// Holds OpCodes Counts for Acir and Brillig Opcodes
Expand All @@ -29,8 +36,15 @@ pub struct OpCodesCount {
}

impl DebugInfo {
pub fn new(locations: BTreeMap<OpcodeLocation, Vec<Location>>) -> Self {
DebugInfo { locations }
pub fn new(
locations: BTreeMap<OpcodeLocation, Vec<Location>>,
var_types: VariableTypes,
) -> Self {
Self {
locations,
variables: var_types.0.into_iter().collect(),
types: var_types.1.into_iter().collect(),
}
}

/// Updates the locations map when the [`Circuit`][acvm::acir::circuit::Circuit] is modified.
Expand Down Expand Up @@ -85,4 +99,11 @@ impl DebugInfo {

counted_opcodes
}

pub fn get_file_ids(&self) -> Vec<FileId> {
self.locations
.values()
.filter_map(|call_stack| call_stack.last().map(|location| location.file))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty sure we can't have empty callstacks so this filtering may be unnecessary.

.collect()
}
}
4 changes: 4 additions & 0 deletions compiler/noirc_errors/src/position.rs
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,10 @@ impl Span {
pub fn end(&self) -> u32 {
self.0.end().into()
}

pub fn from_str(s: &str) -> Span {
Span(ByteSpan::from_str(s))
}
}

impl From<Span> for Range<usize> {
Expand Down
3 changes: 2 additions & 1 deletion compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,7 @@ pub fn create_circuit(
enable_ssa_logging: bool,
enable_brillig_logging: bool,
) -> Result<(Circuit, DebugInfo, Abi, Vec<SsaReport>), RuntimeError> {
let debug_var_types = program.debug_var_types.clone();
let func_sig = program.main_function_signature.clone();
let mut generated_acir =
optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?;
Expand Down Expand Up @@ -126,7 +127,7 @@ pub fn create_circuit(
.map(|(index, locations)| (index, locations.into_iter().collect()))
.collect();

let mut debug_info = DebugInfo::new(locations);
let mut debug_info = DebugInfo::new(locations, debug_var_types);

// Perform any ACIR-level optimizations
let (optimized_circuit, transformation_map) = acvm::compiler::optimize(circuit);
Expand Down
Loading
Loading