Skip to content

Commit

Permalink
Merge pull request #221 from urbit/msl/perform-flogging
Browse files Browse the repository at this point in the history
debug: use `flog!` macro instead of `eprintln!` sometimes
  • Loading branch information
eamsden authored Mar 18, 2024
2 parents 4ee1cb1 + bf3b644 commit f96297d
Show file tree
Hide file tree
Showing 5 changed files with 42 additions and 22 deletions.
2 changes: 1 addition & 1 deletion rust/ares/src/flog.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,6 @@ pub fn flog_fmt(context: &mut Context, fmt: Arguments<'_>) -> Result<()> {
#[macro_export]
macro_rules! flog {
($ctx:expr, $($arg:tt)*) => {
$crate::flog::flog_fmt($ctx, std::format_args!($($arg)*))
let _ = $crate::flog::flog_fmt($ctx, std::format_args!($($arg)*));
}
}
12 changes: 8 additions & 4 deletions rust/ares/src/interpreter.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use crate::assert_acyclic;
use crate::assert_no_forwarding_pointers;
use crate::assert_no_junior_pointers;
use crate::flog;
use crate::guard::call_with_guard;
use crate::hamt::Hamt;
use crate::jets::cold;
Expand Down Expand Up @@ -390,7 +391,7 @@ pub fn interpret(context: &mut Context, mut subject: Noun, formula: Noun) -> Res
*(context.stack.push()) = NockWork::Done;
};

// DO NOT REMOVE THIS ASSERTION
// DO NOT REMOVE THIS COMMENT
//
// If you need to allocate for debugging, wrap the debugging code in
//
Expand Down Expand Up @@ -1339,8 +1340,11 @@ unsafe fn write_trace(context: &mut Context) {
// Abort writing to trace file if we encountered an error. This should
// result in a well-formed partial trace file.
if let Err(_e) = write_nock_trace(&mut context.stack, info, trace_stack) {
// XX: need NockStack allocated string interpolation
// eprintln!("\rserf: error writing nock trace to file: {:?}", e);
flog!(
context,
"\rserf: error writing nock trace to file: {:?}",
_e
);
context.trace_info = None;
}
}
Expand Down Expand Up @@ -1387,7 +1391,7 @@ mod hint {
// XX: what is the head here?
let jet_name = jet_formula.tail();

if let Some(jet) = jets::get_jet(jet_name) {
if let Some(jet) = jets::get_jet(context, jet_name) {
match jet(context, subject) {
Ok(mut jet_res) => {
// XX: simplify this by moving jet test mode into the 11 code in interpret, or into its own function?
Expand Down
6 changes: 3 additions & 3 deletions rust/ares/src/jets.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ pub mod serial;
pub mod sort;
pub mod tree;

use crate::flog;
use crate::interpreter::{Context, Error, Mote};
use crate::jets::bits::*;
use crate::jets::cold::Cold;
Expand Down Expand Up @@ -96,7 +97,7 @@ impl From<JetErr> for Error {
}
}

pub fn get_jet(jet_name: Noun) -> Option<Jet> {
pub fn get_jet(context: &mut Context, jet_name: Noun) -> Option<Jet> {
match jet_name.as_direct().ok()?.data() {
tas!(b"add") => Some(jet_add),
tas!(b"dec") => Some(jet_dec),
Expand Down Expand Up @@ -166,8 +167,7 @@ pub fn get_jet(jet_name: Noun) -> Option<Jet> {
tas!(b"sivc_de") => Some(jet_sivc_de),
//
_ => {
// XX: need NockStack allocated string interpolation
// eprintln!("Unknown jet: {:?}", jet_name);
flog!(context, "unknown jet: {:?}", jet_name);
None
}
}
Expand Down
30 changes: 20 additions & 10 deletions rust/ares/src/serf.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use crate::hamt::Hamt;
use crate::interpreter;
use crate::interpreter::{inc, interpret, Error, Mote};
use crate::jets::cold::Cold;
use crate::jets::hot::{Hot, HotEntry};
Expand All @@ -13,6 +12,7 @@ use crate::noun::{Atom, Cell, DirectAtom, Noun, Slots, D, T};
use crate::persist::pma_meta_set;
use crate::persist::{pma_meta_get, pma_open, pma_sync, Persist};
use crate::trace::*;
use crate::{flog, interpreter};
use ares_macros::tas;
use signal_hook;
use signal_hook::consts::SIGINT;
Expand Down Expand Up @@ -332,6 +332,8 @@ pub fn serf(constant_hot_state: &[HotEntry]) -> io::Result<()> {
if let Some(ref mut info) = trace_info.as_mut() {
if let Err(_e) = write_metadata(info) {
// XX: need NockStack allocated string interpolation
// XX: chicken/egg problem with flog bc it requires context
// before we've initialized it, and context needs trace_info
// eprintln!("\rError initializing trace file: {:?}", e);
trace_info = None;
}
Expand All @@ -351,18 +353,26 @@ pub fn serf(constant_hot_state: &[HotEntry]) -> io::Result<()> {
tas!(b"live") => {
let inner = slot(writ, 6)?.as_direct().unwrap();
match inner.data() {
tas!(b"cram") => eprintln!("\r %cram: not implemented"),
tas!(b"cram") => {
flog!(&mut context.nock_context, "\r %cram: not implemented");
}
tas!(b"exit") => {
eprintln!("\r %exit");
flog!(&mut context.nock_context, "\r %exit");
std::process::exit(0);
}
tas!(b"save") => {
// XX what is eve for?
pma_sync();
}
tas!(b"meld") => eprintln!("\r %meld: not implemented"),
tas!(b"pack") => eprintln!("\r %pack: not implemented"),
_ => eprintln!("unknown live"),
tas!(b"meld") => {
flog!(&mut context.nock_context, "\r %meld: not implemented");
}
tas!(b"pack") => {
flog!(&mut context.nock_context, "\r %pack: not implemented");
}
_ => {
flog!(&mut context.nock_context, "unknown live");
}
}
context.live();
}
Expand Down Expand Up @@ -411,7 +421,7 @@ fn peek(context: &mut Context, ovo: Noun) -> Noun {
let trace_name = "peek";
let start = Instant::now();
let slam_res = slam(context, PEEK_AXIS, ovo);
write_serf_trace_safe(&mut context.nock_context.trace_info, trace_name, start);
write_serf_trace_safe(&mut context.nock_context, trace_name, start);

slam_res.expect("peek error handling unimplemented")
} else {
Expand All @@ -436,7 +446,7 @@ fn soft(context: &mut Context, ovo: Noun, trace_name: Option<String>) -> Result<
let start = Instant::now();
let slam_res = slam(context, POKE_AXIS, ovo);
write_serf_trace_safe(
&mut context.nock_context.trace_info,
&mut context.nock_context,
trace_name.as_ref().unwrap(),
start,
);
Expand Down Expand Up @@ -467,7 +477,7 @@ fn play_life(context: &mut Context, eve: Noun) {
let trace_name = "boot";
let start = Instant::now();
let boot_res = interpret(&mut context.nock_context, eve, lyf);
write_serf_trace_safe(&mut context.nock_context.trace_info, trace_name, start);
write_serf_trace_safe(&mut context.nock_context, trace_name, start);

boot_res
} else {
Expand Down Expand Up @@ -605,7 +615,7 @@ fn work_swap(context: &mut Context, job: Noun, goof: Noun) {
context.work_swap(ovo, fec);
}
Err(goof_crud) => {
eprintln!("\rserf: bail");
flog!(&mut context.nock_context, "\rserf: bail");
let stack = &mut context.nock_context.stack;
let lud = T(stack, &[goof_crud, goof, D(0)]);
context.work_bail(lud);
Expand Down
14 changes: 10 additions & 4 deletions rust/ares/src/trace.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
use crate::flog;
use crate::interpreter::Context;
use crate::jets::bits::util::rap;
use crate::jets::form::util::scow;
use crate::mem::NockStack;
Expand Down Expand Up @@ -97,10 +99,14 @@ pub fn write_metadata(info: &mut TraceInfo) -> Result<(), Error> {
/// Abort writing to trace file if an error is encountered.
///
/// This should result in a well-formed partial trace file.
pub fn write_serf_trace_safe(info: &mut Option<TraceInfo>, name: &str, start: Instant) {
if let Err(_e) = write_serf_trace(info.as_mut().unwrap(), name, start) {
// XX: need NockStack allocated string interpolation
// eprintln!("\rserf: error writing event trace to file: {:?}", e);
pub fn write_serf_trace_safe(context: &mut Context, name: &str, start: Instant) {
if let Err(e) = write_serf_trace(context.trace_info.as_mut().unwrap(), name, start) {
flog!(
context,
"\rserf: error writing event trace to file: {:?}",
e
);
let info = &mut context.trace_info;
*info = None;
}
}
Expand Down

0 comments on commit f96297d

Please sign in to comment.