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: add CLI argument for debugging comptime blocks #5192

Merged
merged 27 commits into from
Jul 10, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
33ee349
feat: add CLI argument for debugging comptime blocks for a particular…
michaeljklein Jun 6, 2024
695194c
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jun 6, 2024
53b0a8b
basic debugging working w/o filtering, added handling for each of the…
michaeljklein Jun 7, 2024
289afaa
add method to find FileId by path suffix, cleanup and make methods fo…
michaeljklein Jun 7, 2024
1d8ba31
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jun 7, 2024
ebed5f8
cargo clippy/fmt
michaeljklein Jun 7, 2024
1bc05ea
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jun 7, 2024
8e518c3
cleanup TODO's
michaeljklein Jun 7, 2024
f1bdb57
patch change from merging master
michaeljklein Jun 7, 2024
f7f01b3
add missing arg
michaeljklein Jun 7, 2024
52bcd04
add missing args in tests
michaeljklein Jun 7, 2024
375229e
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jun 10, 2024
6351085
Update compiler/noirc_frontend/src/hir/comptime/errors.rs
michaeljklein Jun 12, 2024
60d9ef6
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jun 12, 2024
427b1ca
Update compiler/noirc_errors/src/reporter.rs
michaeljklein Jun 14, 2024
2ae971f
Update compiler/noirc_frontend/src/elaborator/mod.rs
michaeljklein Jun 14, 2024
d225099
fix span todo
michaeljklein Jun 17, 2024
f030f38
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jul 8, 2024
32fe0ab
add debug_comptime_scope parameter where missing, use newer NamedGene…
michaeljklein Jul 8, 2024
0c76abe
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jul 8, 2024
b9f6745
make find_by_path_suffix less generic, use as_deref instead of clone,…
michaeljklein Jul 9, 2024
7658908
make functions for checking debug scope and debugging current expression
michaeljklein Jul 9, 2024
0bc2931
refactor setting up interpreter, fix triple-backtick automatic test e…
michaeljklein Jul 9, 2024
2e1cc5e
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jul 9, 2024
71ea123
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jul 9, 2024
90cab04
rename debug_comptime_scope -> debug_comptime_in_file, pass interpret…
michaeljklein Jul 9, 2024
92dc357
Merge branch 'master' into michaeljklein/debug-comptime-cli
michaeljklein Jul 9, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions compiler/fm/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ license.workspace = true

[dependencies]
codespan-reporting.workspace = true
iter-extended.workspace = true
serde.workspace = true

[dev-dependencies]
Expand Down
25 changes: 25 additions & 0 deletions compiler/fm/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ mod file_map;

pub use file_map::{File, FileId, FileMap, PathString};

use iter_extended::vecmap;
// Re-export for the lsp
pub use codespan_reporting::files as codespan_files;

use std::{
collections::HashMap,
ffi::OsStr,
path::{Component, Path, PathBuf},
};

Expand Down Expand Up @@ -103,6 +105,29 @@ impl FileManager {
pub fn name_to_id(&self, file_name: PathBuf) -> Option<FileId> {
self.file_map.get_file_id(&PathString::from_path(file_name))
}

/// Find a file by its path suffix, e.g. "src/main.nr" is a suffix of
/// "some_dir/package_name/src/main.nr"`
pub fn find_by_path_suffix<S: AsRef<OsStr> + ?Sized>(
&self,
suffix: &S,
) -> Result<Option<FileId>, Vec<PathBuf>> {
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
let suffix_path: Vec<_> = Path::new(suffix).components().rev().collect();
let results: Vec<_> = self
.path_to_id
.iter()
.filter(|(path, _id)| {
path.components().rev().zip(suffix_path.iter()).all(|(x, y)| &x == y)
})
.collect();
if results.is_empty() {
Ok(None)
} else if results.len() == 1 {
Ok(Some(*results[0].1))
} else {
Err(vecmap(results, |(path, _id)| path.clone()))
}
}
}

pub trait NormalizePath {
Expand Down
11 changes: 10 additions & 1 deletion compiler/noirc_driver/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,11 @@ pub struct CompileOptions {
#[arg(long, hide = true)]
pub use_elaborator: bool,

/// Enable printing results of comptime evaluation: provide a path suffix
/// for the module to debug, e.g. "package_name/src/main.nr"
#[arg(long)]
pub debug_comptime_scope: Option<String>,
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved

/// Outputs the paths to any modified artifacts
#[arg(long, hide = true)]
pub show_artifact_paths: bool,
Expand Down Expand Up @@ -258,12 +263,14 @@ pub fn check_crate(
deny_warnings: bool,
disable_macros: bool,
use_elaborator: bool,
debug_comptime_scope: Option<String>,
) -> CompilationResult<()> {
let macros: &[&dyn MacroProcessor] =
if disable_macros { &[] } else { &[&aztec_macros::AztecMacro as &dyn MacroProcessor] };

let mut errors = vec![];
let diagnostics = CrateDefMap::collect_defs(crate_id, context, use_elaborator, macros);
let diagnostics =
CrateDefMap::collect_defs(crate_id, context, use_elaborator, debug_comptime_scope, macros);
errors.extend(diagnostics.into_iter().map(|(error, file_id)| {
let diagnostic = CustomDiagnostic::from(&error);
diagnostic.in_file(file_id)
Expand Down Expand Up @@ -301,6 +308,7 @@ pub fn compile_main(
options.deny_warnings,
options.disable_macros,
options.use_elaborator,
options.debug_comptime_scope.clone(),
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
)?;

let main = context.get_main_function(&crate_id).ok_or_else(|| {
Expand Down Expand Up @@ -342,6 +350,7 @@ pub fn compile_contract(
options.deny_warnings,
options.disable_macros,
options.use_elaborator,
options.debug_comptime_scope.clone(),
)?;

// TODO: We probably want to error if contracts is empty
Expand Down
2 changes: 1 addition & 1 deletion compiler/noirc_driver/tests/stdlib_warnings.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ fn stdlib_does_not_produce_constant_warnings() -> Result<(), ErrorsAndWarnings>
let root_crate_id = prepare_crate(&mut context, file_name);

let ((), warnings) =
noirc_driver::check_crate(&mut context, root_crate_id, false, false, false)?;
noirc_driver::check_crate(&mut context, root_crate_id, false, false, false, None)?;

assert_eq!(warnings, Vec::new(), "stdlib is producing warnings");

Expand Down
49 changes: 41 additions & 8 deletions compiler/noirc_errors/src/reporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ pub struct CustomDiagnostic {
pub enum DiagnosticKind {
Error,
Warning,
Debug,
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}

/// A count of errors that have been already reported to stderr
Expand All @@ -34,30 +35,57 @@ impl CustomDiagnostic {
}
}

pub fn simple_error(
fn simple_with_kind(
primary_message: String,
secondary_message: String,
secondary_span: Span,
kind: DiagnosticKind,
) -> CustomDiagnostic {
CustomDiagnostic {
message: primary_message,
secondaries: vec![CustomLabel::new(secondary_message, secondary_span)],
notes: Vec::new(),
kind: DiagnosticKind::Error,
kind,
}
}

pub fn simple_error(
primary_message: String,
secondary_message: String,
secondary_span: Span,
) -> CustomDiagnostic {
Self::simple_with_kind(
primary_message,
secondary_message,
secondary_span,
DiagnosticKind::Error,
)
}

pub fn simple_warning(
primary_message: String,
secondary_message: String,
secondary_span: Span,
) -> CustomDiagnostic {
CustomDiagnostic {
message: primary_message,
secondaries: vec![CustomLabel::new(secondary_message, secondary_span)],
notes: Vec::new(),
kind: DiagnosticKind::Warning,
}
Self::simple_with_kind(
primary_message,
secondary_message,
secondary_span,
DiagnosticKind::Warning,
)
}

pub fn simple_debug(
primary_message: String,
secondary_message: String,
secondary_span: Span,
) -> CustomDiagnostic {
Self::simple_with_kind(
primary_message,
secondary_message,
secondary_span,
DiagnosticKind::Debug,
)
}

pub fn in_file(self, file_id: fm::FileId) -> FileDiagnostic {
Expand All @@ -79,6 +107,10 @@ impl CustomDiagnostic {
pub fn is_warning(&self) -> bool {
matches!(self.kind, DiagnosticKind::Warning)
}

pub fn is_debug(&self) -> bool {
matches!(self.kind, DiagnosticKind::Warning)
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}
}

impl std::fmt::Display for CustomDiagnostic {
Expand Down Expand Up @@ -166,6 +198,7 @@ fn convert_diagnostic(
) -> Diagnostic<fm::FileId> {
let diagnostic = match (cd.kind, deny_warnings) {
(DiagnosticKind::Warning, false) => Diagnostic::warning(),
(DiagnosticKind::Debug, _) => Diagnostic::note(),
_ => Diagnostic::error(),
};

Expand Down
33 changes: 26 additions & 7 deletions compiler/noirc_frontend/src/elaborator/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,15 +16,15 @@ use crate::{
hir_def::{
expr::{
HirArrayLiteral, HirBinaryOp, HirBlockExpression, HirCallExpression, HirCastExpression,
HirConstructorExpression, HirIfExpression, HirIndexExpression, HirInfixExpression,
HirLambda, HirMemberAccess, HirMethodCallExpression, HirMethodReference,
HirPrefixExpression,
HirConstructorExpression, HirExpression, HirIfExpression, HirIndexExpression,
HirInfixExpression, HirLambda, HirMemberAccess, HirMethodCallExpression,
HirMethodReference, HirPrefixExpression,
},
traits::TraitConstraint,
},
macros_api::{
BlockExpression, CallExpression, CastExpression, Expression, ExpressionKind, HirExpression,
HirLiteral, HirStatement, Ident, IndexExpression, Literal, MemberAccessExpression,
BlockExpression, CallExpression, CastExpression, Expression, ExpressionKind, HirLiteral,
HirStatement, Ident, IndexExpression, Literal, MemberAccessExpression,
MethodCallExpression, PrefixExpression,
},
node_interner::{DefinitionKind, ExprId, FuncId},
Expand Down Expand Up @@ -633,9 +633,28 @@ impl<'context> Elaborator<'context> {

fn elaborate_comptime_block(&mut self, block: BlockExpression, span: Span) -> (ExprId, Type) {
let (block, _typ) = self.elaborate_block_expression(block);
let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter_errors = vec![];
let mut interpreter = Interpreter::new(
self.interner,
&mut self.comptime_scopes,
self.debug_comptime_scope,
&mut interpreter_errors,
);
let value = interpreter.evaluate_block(block);
self.inline_comptime_value(value, span)
self.include_interpreter_errors(interpreter_errors);
let (id, typ) = self.inline_comptime_value(value, span);

let location = self.interner.id_location(id);
if Some(location.file) == self.debug_comptime_scope {
let new_expr =
self.interner.expression(&id).to_display_ast(self.interner, location.span).kind;
self.errors.push((
InterpreterError::debug_evaluate_comptime(new_expr, location).into(),
location.file,
));
}

(id, typ)
}

pub(super) fn inline_comptime_value(
Expand Down
45 changes: 40 additions & 5 deletions compiler/noirc_frontend/src/elaborator/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use std::{
use crate::{
ast::{FunctionKind, UnresolvedTraitConstraint},
hir::{
comptime::{self, Interpreter},
comptime::{self, Interpreter, InterpreterError},
def_collector::{
dc_crate::{
filter_literal_globals, CompilationError, ImplMap, UnresolvedGlobal,
Expand Down Expand Up @@ -166,14 +166,21 @@ pub struct Elaborator<'context> {
/// up all currently visible definitions. The first scope is always the global scope.
comptime_scopes: Vec<HashMap<DefinitionId, comptime::Value>>,

/// The scope of --debug-comptime, or None if unset
debug_comptime_scope: Option<FileId>,

/// These are the globals that have yet to be elaborated.
/// This map is used to lazily evaluate these globals if they're encountered before
/// they are elaborated (e.g. in a function's type or another global's RHS).
unresolved_globals: BTreeMap<GlobalId, UnresolvedGlobal>,
}

impl<'context> Elaborator<'context> {
pub fn new(context: &'context mut Context, crate_id: CrateId) -> Self {
pub fn new(
context: &'context mut Context,
crate_id: CrateId,
debug_comptime_scope: Option<FileId>,
) -> Self {
Self {
scopes: ScopeForest::default(),
errors: Vec::new(),
Expand All @@ -197,6 +204,7 @@ impl<'context> Elaborator<'context> {
trait_constraints: Vec::new(),
current_trait_impl: None,
comptime_scopes: vec![HashMap::default()],
debug_comptime_scope,
unresolved_globals: BTreeMap::new(),
}
}
Expand All @@ -205,8 +213,9 @@ impl<'context> Elaborator<'context> {
context: &'context mut Context,
crate_id: CrateId,
mut items: CollectedItems,
debug_comptime_scope: Option<FileId>,
) -> Vec<(CompilationError, FileId)> {
let mut this = Self::new(context, crate_id);
let mut this = Self::new(context, crate_id, debug_comptime_scope);

// We must first resolve and intern the globals before we can resolve any stmts inside each function.
// Each function uses its own resolver with a newly created ScopeForest, and must be resolved again to be within a function's scope
Expand Down Expand Up @@ -1181,8 +1190,13 @@ impl<'context> Elaborator<'context> {
let global = self.interner.get_global(global_id);
let definition_id = global.definition_id;
let location = global.location;

let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter_errors = vec![];
let mut interpreter = Interpreter::new(
self.interner,
&mut self.comptime_scopes,
self.debug_comptime_scope,
&mut interpreter_errors,
);

if let Err(error) = interpreter.evaluate_let(let_statement) {
self.errors.push(error.into_compilation_error_pair());
Expand All @@ -1191,8 +1205,22 @@ impl<'context> Elaborator<'context> {
.lookup_id(definition_id, location)
.expect("The global should be defined since evaluate_let did not error");

if Some(location.file) == self.debug_comptime_scope {
let new_stmt = self
.interner
.get_global(global_id)
.let_statement
.to_display_ast(self.interner)
.kind;
self.errors.push((
InterpreterError::debug_evaluate_comptime(new_stmt, location).into(),
location.file,
));
}

self.interner.get_global_mut(global_id).value = Some(value);
}
self.include_interpreter_errors(interpreter_errors);
}

fn define_function_metas(
Expand Down Expand Up @@ -1258,4 +1286,11 @@ impl<'context> Elaborator<'context> {
});
}
}

fn include_interpreter_errors(&mut self, errors: Vec<InterpreterError>) {
self.errors.extend(errors.into_iter().map(|error| {
let file_id = error.get_location().file;
(error.into(), file_id)
}));
michaeljklein marked this conversation as resolved.
Show resolved Hide resolved
}
}
21 changes: 19 additions & 2 deletions compiler/noirc_frontend/src/elaborator/statements.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use noirc_errors::{Location, Span};
use crate::{
ast::{AssignStatement, ConstrainStatement, LValue},
hir::{
comptime::Interpreter,
comptime::{Interpreter, InterpreterError},
resolution::errors::ResolverError,
type_check::{Source, TypeCheckError},
},
Expand Down Expand Up @@ -432,9 +432,26 @@ impl<'context> Elaborator<'context> {
fn elaborate_comptime_statement(&mut self, statement: Statement) -> (HirStatement, Type) {
let span = statement.span;
let (hir_statement, _typ) = self.elaborate_statement(statement);
let mut interpreter = Interpreter::new(self.interner, &mut self.comptime_scopes);
let mut interpreter_errors = vec![];
let mut interpreter = Interpreter::new(
self.interner,
&mut self.comptime_scopes,
self.debug_comptime_scope,
&mut interpreter_errors,
);
jfecher marked this conversation as resolved.
Show resolved Hide resolved
let value = interpreter.evaluate_statement(hir_statement);
let (expr, typ) = self.inline_comptime_value(value, span);
self.include_interpreter_errors(interpreter_errors);

let location = self.interner.id_location(hir_statement);
if Some(location.file) == self.debug_comptime_scope {
let new_expr = expr.to_display_ast(self.interner).kind;
self.errors.push((
InterpreterError::debug_evaluate_comptime(new_expr, location).into(),
location.file,
));
}

(HirStatement::Expression(expr), typ)
}
}
Loading
Loading