From 1a4050d21d880351da84e6cf884f00596f09da93 Mon Sep 17 00:00:00 2001 From: Elias Castro <127804425+eliascxstro@users.noreply.github.com> Date: Thu, 16 May 2024 14:59:35 -0400 Subject: [PATCH] Metadata (#2022) * Basic scaffolding for metadeta * Implemented new_metadata.pset and new_parser.rs * Added new_parser and test cases * Enabled debugger to debug any .futil file * Clippy complaints * Adjusted code to PR suggestions and added some comments --- cider-dap/src/adapter.rs | 38 ++++----- cider-dap/src/main.rs | 5 +- interp/src/debugger/cidr.rs | 25 ++++-- interp/src/debugger/mod.rs | 1 + interp/src/debugger/new_metadata.pest | 13 +++ interp/src/debugger/new_parser.rs | 101 +++++++++++++++++++++++ interp/src/debugger/source/metadata.pest | 20 ++--- interp/src/debugger/source/structures.rs | 18 +++- interp/src/errors.rs | 9 ++ 9 files changed, 181 insertions(+), 49 deletions(-) create mode 100644 interp/src/debugger/new_metadata.pest create mode 100644 interp/src/debugger/new_parser.rs diff --git a/cider-dap/src/adapter.rs b/cider-dap/src/adapter.rs index 780b3073e1..1ece2c6411 100644 --- a/cider-dap/src/adapter.rs +++ b/cider-dap/src/adapter.rs @@ -2,7 +2,6 @@ use crate::error::AdapterResult; use dap::types::{Breakpoint, Source, SourceBreakpoint, StackFrame, Thread}; use interp::debugger::source::structures::NewSourceMap; use interp::debugger::Debugger; -use std::collections::HashMap; use std::path::PathBuf; pub struct MyAdapter { @@ -18,13 +17,12 @@ pub struct MyAdapter { ids: NewSourceMap, } -// New metadata in interp/debugger - impl MyAdapter { pub fn new(path: &str, std_path: PathBuf) -> AdapterResult { + let (debugger, metadata) = + Debugger::from_file(&PathBuf::from(path), &std_path).unwrap(); Ok(MyAdapter { - debugger: Debugger::from_file(&PathBuf::from(path), &std_path) - .unwrap(), + debugger, break_count: Counter::new(), thread_count: Counter::new(), stack_count: Counter::new(), @@ -32,7 +30,7 @@ impl MyAdapter { stack_frames: Vec::new(), threads: Vec::new(), source: path.to_string(), - ids: create_map(), + ids: metadata, }) } ///Set breakpoints for adapter @@ -80,8 +78,8 @@ impl MyAdapter { pub fn create_stack(&mut self) -> Vec { let frame = StackFrame { id: self.stack_count.increment(), - // TODO: edit name field - name: String::from("Hi"), + // Maybe automate the name in the future? + name: String::from("Frame"), source: Some(Source { name: None, path: Some(self.source.clone()), @@ -111,22 +109,25 @@ impl MyAdapter { } pub fn next_line(&mut self, _thread: i64) -> bool { + // Step through once let status = self.debugger.step(1).unwrap(); // Check if done: if status.get_done() { + // Give bool to exit the debugger true } else { - let map = status.get_status().clone(); - // Declare line number beforehand + let map = status.get_status(); let mut line_number = 0; - // Return -1 should a lookup not be found. This really shouldn't - // happen though + // Implemented for loop for when more than 1 group is running, + // the code for now goes to the line of the last group running in the map, should deal + // with this in the future for when groups run in parallel. for id in map { - let value = *self.ids.lookup(id.to_string()).unwrap_or(&-1); + let value = self.ids.lookup(id.to_string()).unwrap().line; line_number = value; } - self.stack_frames[0].line = line_number; + // Set line of the stack frame and tell debugger we're not finished. + self.stack_frames[0].line = line_number as i64; false } } @@ -173,12 +174,3 @@ pub fn make_breakpoint( offset: None, } } - -// Hardcode mapping for now, this mapping is for reg_seq.futil -fn create_map() -> NewSourceMap { - let mut hashmap = HashMap::new(); - // Hardcode - hashmap.insert(String::from("wr_reg0"), 10); - hashmap.insert(String::from("wr_reg1"), 15); - NewSourceMap::from(hashmap) -} diff --git a/cider-dap/src/main.rs b/cider-dap/src/main.rs index 511be48db0..44598b87d8 100644 --- a/cider-dap/src/main.rs +++ b/cider-dap/src/main.rs @@ -148,7 +148,8 @@ where // Construct the adapter let mut adapter = MyAdapter::new(program_path, std_path)?; - //Make two threads to make threads visible on call stack, subject to change. + // Currently, we need two threads to run the debugger and step through, + // not sure why but would be good to look into for the future. let thread = &adapter.create_thread(String::from("Main")); let thread2 = &adapter.create_thread(String::from("Thread 1")); @@ -199,7 +200,6 @@ fn run_server( } } - // TODO: Implement this request fully when adapter becomes functional Command::SetExceptionBreakpoints(_) => { let rsp = req.success(ResponseBody::SetExceptionBreakpoints( SetExceptionBreakpointsResponse { @@ -313,7 +313,6 @@ fn run_server( } Command::Scopes(_) => { let rsp = req.success(ResponseBody::Scopes(ScopesResponse { - // TODO: Understand vectors scopes: vec![], })); server.respond(rsp)?; diff --git a/interp/src/debugger/cidr.rs b/interp/src/debugger/cidr.rs index 416a6eac9f..b8409fca63 100644 --- a/interp/src/debugger/cidr.rs +++ b/interp/src/debugger/cidr.rs @@ -3,6 +3,8 @@ use super::{ context::DebuggingContext, interactive_errors::DebuggerError, io_utils::Input, + new_parser::parse_metadata, + source::structures::NewSourceMap, }; use crate::interpreter::{ComponentInterpreter, ConstCell, Interpreter}; use crate::structures::names::{CompGroupName, ComponentQualifiedInstanceName}; @@ -78,7 +80,10 @@ pub struct Debugger { impl Debugger { /// construct a debugger instance from the target calyx file - pub fn from_file(file: &Path, lib_path: &Path) -> InterpreterResult { + pub fn from_file( + file: &Path, + lib_path: &Path, + ) -> InterpreterResult<(Self, NewSourceMap)> { // create a workspace using the file and lib_path, run the standard // passes (see main.rs). Construct the initial environment then use that // to create a new debugger instance with new @@ -122,7 +127,16 @@ impl Debugger { &config, )?; - Debugger::new(&components, main_component, None, env) + // Make NewSourceMap, if we can't then we explode + let mapping = ctx + .metadata + .map(|metadata| parse_metadata(&metadata)) + .unwrap_or_else(|| Err(InterpreterError::MissingMetaData.into()))?; + + Ok(( + Debugger::new(&components, main_component, None, env).unwrap(), + mapping, + )) } pub fn new( @@ -150,8 +164,7 @@ impl Debugger { }) } - // probably want a different return type - // Return InterpreterResult of Program Status, new struct + // Go to next step pub fn step(&mut self, n: u64) -> InterpreterResult { for _ in 0..n { self.interpreter.step()?; @@ -165,10 +178,6 @@ impl Debugger { )) } - /// continue the execution until a breakpoint is hit, needs a different - /// return type or we need a different "update status" method to communicate - /// with the adapter. The latter might be more ergonomic, though potentially - /// less efficient pub fn cont(&mut self) -> InterpreterResult<()> { self.debugging_ctx .set_current_time(self.interpreter.currently_executing_group()); diff --git a/interp/src/debugger/mod.rs b/interp/src/debugger/mod.rs index a8fd079274..d7b1e1644b 100644 --- a/interp/src/debugger/mod.rs +++ b/interp/src/debugger/mod.rs @@ -4,6 +4,7 @@ mod context; mod interactive_errors; mod io_utils; pub(crate) mod name_tree; +pub(crate) mod new_parser; pub(crate) mod parser; pub mod source; pub use commands::PrintCode; diff --git a/interp/src/debugger/new_metadata.pest b/interp/src/debugger/new_metadata.pest new file mode 100644 index 0000000000..b1c72cbac2 --- /dev/null +++ b/interp/src/debugger/new_metadata.pest @@ -0,0 +1,13 @@ +WHITE_SPACE = _{ " " | "\t" | NEWLINE } +dot = _{ "." } +colon = _{ ":" } +ident_syms = _{ "_" | "-" | "'" } +num = @{ ASCII_DIGIT+ } +group_name = @{ (ASCII_ALPHA | ASCII_DIGIT | ident_syms | dot)+ } +path = @{ (ASCII_ALPHA | ASCII_DIGIT | ident_syms | dot | "/")+ } + +entry = { group_name ~ colon ~ WHITE_SPACE ~ path ~ WHITE_SPACE ~ num ~ (",")* ~ WHITE_SPACE* } + +metadata = { + SOI ~ entry* ~ EOI +} diff --git a/interp/src/debugger/new_parser.rs b/interp/src/debugger/new_parser.rs new file mode 100644 index 0000000000..dbc780846e --- /dev/null +++ b/interp/src/debugger/new_parser.rs @@ -0,0 +1,101 @@ +use super::source::structures::{GroupContents, NewSourceMap}; +use crate::errors::InterpreterResult; +use pest_consume::{match_nodes, Error, Parser}; +use std::collections::HashMap; +type ParseResult = std::result::Result>; +type Node<'i> = pest_consume::Node<'i, Rule, ()>; + +// include the grammar file so that Cargo knows to rebuild this file on grammar changes +const _GRAMMAR: &str = include_str!("new_metadata.pest"); + +#[derive(Parser)] +#[grammar = "debugger/new_metadata.pest"] +pub struct MetadataParser; + +#[pest_consume::parser] +impl MetadataParser { + fn EOI(_input: Node) -> ParseResult<()> { + Ok(()) + } + pub fn num(input: Node) -> ParseResult { + input + .as_str() + .parse::() + .map_err(|_| input.error("Expected number")) + } + fn group_name(input: Node) -> ParseResult { + Ok(input.as_str().to_string()) + } + + fn path(input: Node) -> ParseResult { + Ok(input.as_str().to_string()) + } + + fn entry(input: Node) -> ParseResult<(String, GroupContents)> { + Ok(match_nodes!(input.into_children(); + [group_name(g), path(p),num(n)] => (g,GroupContents {path:p, line: n}) + )) + } + + fn metadata(input: Node) -> ParseResult { + Ok(match_nodes!(input.into_children(); + [entry(e).., EOI(_)] => { + let map: HashMap = e.collect(); + map.into() + } + )) + } +} + +pub fn parse_metadata(input_str: &str) -> InterpreterResult { + let inputs = MetadataParser::parse(Rule::metadata, input_str)?; + let input = inputs.single()?; + Ok(MetadataParser::metadata(input)?) +} + +// Meta is expected as the following format, this is an example for reg_seq.futil + +// metadata #{ +// wr_reg0: /path/to/file 10 +// wr_reg1: /path/to/file 15 +// }# + +#[cfg(test)] +#[test] +fn one_entry() { + let entry = parse_metadata("hello: your/mom 5").unwrap(); + dbg!(&entry); + let tup = entry.lookup(String::from("hello")); + assert_eq!( + tup.unwrap().clone(), + GroupContents { + path: String::from("your/mom"), + line: 5 + } + ) +} + +#[test] +fn mult_entires() { + let entry = parse_metadata( + "wr_reg0: calyx/interp/test/control/reg_seq.futil 10, + wr_reg1: calyx/interp/test/control/reg_seq.futil 15", + ) + .unwrap(); + let tup = entry.lookup(String::from("wr_reg0")); + let tup2 = entry.lookup(String::from("wr_reg1")); + assert_eq!( + tup.unwrap().clone(), + GroupContents { + path: String::from("calyx/interp/test/control/reg_seq.futil"), + line: 10 + } + ); + assert_eq!( + tup2.unwrap().clone(), + GroupContents { + path: String::from("calyx/interp/test/control/reg_seq.futil"), + line: 15 + } + ) +} diff --git a/interp/src/debugger/source/metadata.pest b/interp/src/debugger/source/metadata.pest index 2eb8a222ca..ab3dddad38 100644 --- a/interp/src/debugger/source/metadata.pest +++ b/interp/src/debugger/source/metadata.pest @@ -1,27 +1,25 @@ WHITESPACE = _{ " " | "\t" } -dot = _{"."} +dot = _{ "." } ident_syms = _{ "_" | "-" | "'" } -quote = _{"\""} -num = @{ASCII_DIGIT+} +quote = _{ "\"" } +num = @{ ASCII_DIGIT+ } -id_string = @{ ("_" | ASCII_ALPHA)+ ~ (ident_syms | ASCII_ALPHA | ASCII_DIGIT | "::" |".")* } +id_string = @{ ("_" | ASCII_ALPHA)+ ~ (ident_syms | ASCII_ALPHA | ASCII_DIGIT | "::" | ".")* } named_tag = { "(" ~ num ~ "," ~ id_string ~ ")" } tag = { named_tag | num } escaped_newline = @{ "\\" ~ NEWLINE } -string_char = ${ !(NEWLINE | EOI) ~ ANY } -source_char = { escaped_newline | string_char } +string_char = ${ !(NEWLINE | EOI) ~ ANY } +source_char = { escaped_newline | string_char } -inner_position_string = ${ source_char* } +inner_position_string = ${ source_char* } position_string = { inner_position_string ~ (NEWLINE | &EOI) } -entry = { tag ~ ":" ~ position_string} +entry = { tag ~ ":" ~ position_string } metadata = { - SOI ~ - entry+ ~ - EOI + SOI ~ entry+ ~ EOI } diff --git a/interp/src/debugger/source/structures.rs b/interp/src/debugger/source/structures.rs index d89045ba09..70e984788e 100644 --- a/interp/src/debugger/source/structures.rs +++ b/interp/src/debugger/source/structures.rs @@ -17,20 +17,30 @@ impl From<(u64, String)> for NamedTag { Self(i.0, i.1) } } + +/// GroupContents contains the file path of the group and the line number the +/// group is on. +#[derive(Debug, Clone, PartialEq)] +pub struct GroupContents { + pub path: String, + pub line: u64, +} + +/// impl struct with path and number #[derive(Debug, Clone)] /// NewSourceMap contains the group name as the key and the line it lies on with /// as respect to its corresponding .futil file -pub struct NewSourceMap(HashMap); +pub struct NewSourceMap(HashMap); impl NewSourceMap { /// look up group name, if not present, return None - pub fn lookup(&self, key: String) -> Option<&i64> { + pub fn lookup(&self, key: String) -> Option<&GroupContents> { self.0.get(&key) } } -impl From> for NewSourceMap { - fn from(i: HashMap) -> Self { +impl From> for NewSourceMap { + fn from(i: HashMap) -> Self { Self(i) } } diff --git a/interp/src/errors.rs b/interp/src/errors.rs index 27e443b80b..4180b7bad1 100644 --- a/interp/src/errors.rs +++ b/interp/src/errors.rs @@ -70,6 +70,15 @@ pub enum InterpreterError { #[from] pest_consume::Error, ), + // Unable to parse metadata + #[error(transparent)] + NewMetadataParseError( + #[from] pest_consume::Error, + ), + + // Missing metadata + #[error("missing metadata")] + MissingMetaData, /// Wrapper for errors coming from the interactive CLI #[error(transparent)]