Skip to content

Commit

Permalink
fix: less egregiously abuse random unwraps
Browse files Browse the repository at this point in the history
  • Loading branch information
Gankra committed Jul 13, 2024
1 parent dffbd21 commit d492efb
Show file tree
Hide file tree
Showing 10 changed files with 128 additions and 103 deletions.
2 changes: 1 addition & 1 deletion src/cli.rs
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ Hint: Try using `--pairs {name}_calls_rustc` or `--pairs rustc_calls_{name}`.

let filter_layer = EnvFilter::try_from_default_env()
.or_else(|_| EnvFilter::try_new("info"))
.unwrap();
.expect("failed to initialize logger");

let logger = crate::log::MapLogger::new();
tracing_subscriber::registry()
Expand Down
8 changes: 4 additions & 4 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ pub enum BuildError {
#[error("io error\n{0}")]
Io(#[from] std::io::Error),
#[error("rust compile error \n{} \n{}",
std::str::from_utf8(&.0.stdout).unwrap(),
std::str::from_utf8(&.0.stderr).unwrap())]
String::from_utf8_lossy(&.0.stdout),
String::from_utf8_lossy(&.0.stderr))]
RustCompile(std::process::Output),
#[error("c compile error\n{0}")]
CCompile(#[from] cc::Error),
Expand Down Expand Up @@ -125,8 +125,8 @@ pub enum LinkError {
#[error("io error\n{0}")]
Io(#[from] std::io::Error),
#[error("rust link error \n{} \n{}",
std::str::from_utf8(&.0.stdout).unwrap(),
std::str::from_utf8(&.0.stderr).unwrap())]
String::from_utf8_lossy(&.0.stdout),
String::from_utf8_lossy(&.0.stderr))]
RustLink(std::process::Output),
}

Expand Down
29 changes: 29 additions & 0 deletions src/fivemat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,27 @@ pub struct Fivemat<'a> {
out: &'a mut dyn Write,
}

pub struct FivematIndent<'a, 'b> {
inner: &'b mut Fivemat<'a>,
count: usize,
}
impl Drop for FivematIndent<'_, '_> {
fn drop(&mut self) {
self.inner.sub_indent(self.count);
}
}
impl<'a, 'b> std::ops::Deref for FivematIndent<'a, 'b> {
type Target = Fivemat<'a>;
fn deref(&self) -> &Fivemat<'a> {
self.inner
}
}
impl<'a, 'b> std::ops::DerefMut for FivematIndent<'a, 'b> {
fn deref_mut(&mut self) -> &mut Fivemat<'a> {
self.inner
}
}

impl<'a> Fivemat<'a> {
pub fn new(out: &'a mut dyn Write, indent_text: impl Into<String>) -> Self {
Fivemat {
Expand All @@ -26,6 +47,14 @@ impl<'a> Fivemat<'a> {
out,
}
}
pub fn indent<'b>(&'b mut self) -> FivematIndent<'a, 'b> {
self.indent_many(1)
}
pub fn indent_many<'b>(&'b mut self, count: usize) -> FivematIndent<'a, 'b> {
self.add_indent(count);
FivematIndent { inner: self, count }
}

pub fn add_indent(&mut self, count: usize) {
self.indent += count;
}
Expand Down
33 changes: 22 additions & 11 deletions src/harness/check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -134,8 +134,8 @@ impl TestHarness {
let caller_tag = load_tag(caller_val);
let callee_tag = load_tag(callee_val);

if caller_tag != expected_tag || callee_tag != expected_tag {
let expected = tagged_variant_name(tagged_ty, expected_tag);
if caller_tag != Some(expected_tag) || callee_tag != Some(expected_tag) {
let expected = tagged_variant_name(tagged_ty, Some(expected_tag));
let caller = tagged_variant_name(tagged_ty, caller_tag);
let callee = tagged_variant_name(tagged_ty, callee_tag);
return Err(tag_error(types, &expected_val, expected, caller, callee));
Expand All @@ -145,8 +145,8 @@ impl TestHarness {
let caller_tag = load_tag(caller_val);
let callee_tag = load_tag(callee_val);

if caller_tag != expected_tag || callee_tag != expected_tag {
let expected = enum_variant_name(enum_ty, expected_tag);
if caller_tag != Some(expected_tag) || callee_tag != Some(expected_tag) {
let expected = enum_variant_name(enum_ty, Some(expected_tag));
let caller = enum_variant_name(enum_ty, caller_tag);
let callee = enum_variant_name(enum_ty, callee_tag);
return Err(tag_error(types, &expected_val, expected, caller, callee));
Expand All @@ -156,8 +156,8 @@ impl TestHarness {
let caller_tag = load_tag(caller_val);
let callee_tag = load_tag(callee_val);

if caller_tag != expected_tag || callee_tag != expected_tag {
let expected = bool_variant_name(expected_tag, expected_tag);
if caller_tag != Some(expected_tag) || callee_tag != Some(expected_tag) {
let expected = bool_variant_name(expected_tag, Some(expected_tag));
let caller = bool_variant_name(expected_tag, caller_tag);
let callee = bool_variant_name(expected_tag, callee_tag);
return Err(tag_error(types, &expected_val, expected, caller, callee));
Expand Down Expand Up @@ -189,11 +189,16 @@ impl TestHarness {
}
}

fn load_tag(val: &ValBuffer) -> usize {
u32::from_ne_bytes(<[u8; 4]>::try_from(&val.bytes[..4]).unwrap()) as usize
fn load_tag(val: &ValBuffer) -> Option<usize> {
let buf = val.bytes.get(..4)?;
let bytes = <[u8; 4]>::try_from(buf).ok()?;
Some(u32::from_ne_bytes(bytes) as usize)
}

fn tagged_variant_name(tagged_ty: &kdl_script::types::TaggedTy, tag: usize) -> String {
fn tagged_variant_name(tagged_ty: &kdl_script::types::TaggedTy, tag: Option<usize>) -> String {
let Some(tag) = tag else {
return "<tag never recorded?>".to_owned();
};
let tagged_name = &tagged_ty.name;
let variant_name = tagged_ty
.variants
Expand All @@ -203,7 +208,10 @@ fn tagged_variant_name(tagged_ty: &kdl_script::types::TaggedTy, tag: usize) -> S
format!("{tagged_name}::{variant_name}")
}

fn enum_variant_name(enum_ty: &kdl_script::types::EnumTy, tag: usize) -> String {
fn enum_variant_name(enum_ty: &kdl_script::types::EnumTy, tag: Option<usize>) -> String {
let Some(tag) = tag else {
return "<tag never recorded?>".to_owned();
};
let enum_name = &enum_ty.name;
let variant_name = enum_ty
.variants
Expand All @@ -213,7 +221,10 @@ fn enum_variant_name(enum_ty: &kdl_script::types::EnumTy, tag: usize) -> String
format!("{enum_name}::{variant_name}")
}

fn bool_variant_name(expected_tag: usize, tag: usize) -> String {
fn bool_variant_name(expected_tag: usize, tag: Option<usize>) -> String {
let Some(tag) = tag else {
return "<tag never recorded?>".to_owned();
};
// Because we're using the tag variant machinery, this code is a bit weird,
// because we essentially get passed Option<bool> for `tag`, where we get
// None when the wrong path is taken.
Expand Down
2 changes: 1 addition & 1 deletion src/harness/read/procgen.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
pub fn procgen_test_for_ty_string(ty_name: &str, ty_def: Option<&str>) -> String {
let mut test_body = String::new();
procgen_test_for_ty_impl(&mut test_body, ty_name, ty_def).unwrap();
procgen_test_for_ty_impl(&mut test_body, ty_name, ty_def).expect("failed to format procgen!?");
test_body
}

Expand Down
5 changes: 4 additions & 1 deletion src/harness/run.rs
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,10 @@ fn run_dynamic_test(test_dylib: &LinkOutput, _full_test_name: &str) -> Result<Ru
let mut callee_vals = TestBuffer::new();

unsafe {
info!("running {}", test_dylib.test_bin.file_name().unwrap());
info!(
"running {}",
test_dylib.test_bin.file_name().unwrap_or_default()
);
// Load the dylib of the test, and get its test_start symbol
debug!("loading {}", &test_dylib.test_bin);
let lib = libloading::Library::new(&test_dylib.test_bin)?;
Expand Down
65 changes: 35 additions & 30 deletions src/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,11 +72,12 @@ impl MapLogger {
pub fn new() -> Self {
Self::default()
}
/*
pub fn clear(&self) {
let mut log = self.state.lock().unwrap();
let mut log = self.state.lock().ok()?;
let ids = log.sub_spans.keys().cloned().collect::<Vec<_>>();
for id in ids {
let span = log.sub_spans.get_mut(&id).unwrap();
let span = log.sub_spans.get_mut(&id)?;
if !span.destroyed {
span.events.clear();
continue;
Expand All @@ -85,23 +86,25 @@ impl MapLogger {
}
log.root_span.events.clear();
log.cur_string = None;
Some(())
}

fn print_span_if_test(&self, span_id: &Id) {
*/
fn print_span_if_test(&self, span_id: &Id) -> Result<(), std::fmt::Error> {
let span = {
let log = self.state.lock().unwrap();
let Some(span) = log.live_spans.get(span_id) else {
return;
return Ok(());
};
if !log.test_spans.contains(span) {
return;
return Ok(());
}
*span
};
eprintln!("{}", self.string_for_span(span));
eprintln!("{}", self.string_for_span(span)?);
Ok(())
}

pub fn string_for_span(&self, span: SpanId) -> Arc<String> {
pub fn string_for_span(&self, span: SpanId) -> Result<Arc<String>, std::fmt::Error> {
self.string_query(Query::Span(span))
}
/*
Expand Down Expand Up @@ -151,56 +154,56 @@ impl MapLogger {
}
}
*/
fn string_query(&self, query: Query) -> Arc<String> {
fn string_query(&self, query: Query) -> Result<Arc<String>, std::fmt::Error> {
use std::fmt::Write;

fn print_indent(output: &mut String, depth: usize) {
write!(output, "{:indent$}", "", indent = depth * 4).unwrap();
fn print_indent(output: &mut String, depth: usize) -> Result<(), std::fmt::Error> {
write!(output, "{:indent$}", "", indent = depth * 4)
}
fn print_span_recursive(
f: &mut Fivemat,
sub_spans: &LinkedHashMap<SpanId, SpanEntry>,
span: &SpanEntry,
range: Option<Range<usize>>,
) {
) -> Result<(), std::fmt::Error> {
if !span.name.is_empty() {
let style = Style::new().blue();
write!(f, "{}", style.apply_to(&span.name)).unwrap();
write!(f, "{}", style.apply_to(&span.name))?;
for (key, val) in &span.fields {
if key == "id" {
write!(f, " {}", style.apply_to(val)).unwrap();
write!(f, " {}", style.apply_to(val))?;
} else {
write!(f, "{key}: {val}").unwrap();
write!(f, "{key}: {val}")?;
}
}
writeln!(f).unwrap();
writeln!(f)?;
}

let event_range = if let Some(range) = range {
&span.events[range]
} else {
&span.events[..]
};
f.add_indent(1);
let mut f = f.indent();
for event in event_range {
match event {
EventEntry::Message(event) => {
if event.fields.contains_key("message") {
print_event(f, event);
print_event(&mut f, event)?;
}
}
EventEntry::Span(sub_span) => {
print_span_recursive(f, sub_spans, &sub_spans[sub_span], None);
print_span_recursive(&mut f, sub_spans, &sub_spans[sub_span], None)?;
}
}
}
f.sub_indent(1);
Ok(())
}

let mut log = self.state.lock().unwrap();
if Some(query) == log.last_query {
if let Some(string) = &log.cur_string {
return string.clone();
return Ok(string.clone());
}
}
log.last_query = Some(query);
Expand All @@ -213,22 +216,23 @@ impl MapLogger {
Query::Span(span_id) => (&log.sub_spans[&span_id], None),
};

print_span_recursive(&mut f, &log.sub_spans, span_to_print, range);
print_span_recursive(&mut f, &log.sub_spans, span_to_print, range)?;

let result = Arc::new(output_buf);
log.cur_string = Some(result.clone());
result
Ok(result)
}
}

fn immediate_event(event: &MessageEntry) {
fn immediate_event(event: &MessageEntry) -> std::fmt::Result {
let mut output = String::new();
let mut f = Fivemat::new(&mut output, INDENT);
print_event(&mut f, event);
print_event(&mut f, event)?;
eprintln!("{}", output);
Ok(())
}

fn print_event(f: &mut Fivemat, event: &MessageEntry) {
fn print_event(f: &mut Fivemat, event: &MessageEntry) -> Result<(), std::fmt::Error> {
use std::fmt::Write;
if let Some(message) = event.fields.get("message") {
let style = match event.level {
Expand All @@ -238,9 +242,10 @@ fn print_event(f: &mut Fivemat, event: &MessageEntry) {
Level::DEBUG => Style::new().blue(),
Level::TRACE => Style::new().green(),
};
// writeln!(output, "[{:5}] {}", event.level, message).unwrap();
writeln!(f, "{}", style.apply_to(message)).unwrap();
// writeln!(output, "[{:5}] {}", event.level, message)?;
writeln!(f, "{}", style.apply_to(message))?;
}
Ok(())
}

impl<S> Layer<S> for MapLogger
Expand Down Expand Up @@ -277,7 +282,7 @@ where
target: target.to_owned(),
};
if is_root {
immediate_event(&event);
immediate_event(&event).ok();
}
cur_span.events.push(EventEntry::Message(event));
}
Expand Down Expand Up @@ -332,7 +337,7 @@ where
fn on_close(&self, id: Id, _ctx: tracing_subscriber::layer::Context<'_, S>) {
// Mark the span as GC-able and remove it from the live mappings,
// as tracing may now recycle the id for future spans!
self.print_span_if_test(&id);
self.print_span_if_test(&id).ok();
let mut log = self.state.lock().unwrap();
let Some(&span_id) = log.live_spans.get(&id) else {
// Skipped span, ignore
Expand Down
6 changes: 3 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -215,9 +215,9 @@ fn main() -> Result<(), Box<dyn Error>> {

let mut output = std::io::stdout();
match cfg.output_format {
OutputFormat::Human => full_report.print_human(&harness, &mut output).unwrap(),
OutputFormat::Json => full_report.print_json(&harness, &mut output).unwrap(),
OutputFormat::RustcJson => full_report.print_rustc_json(&harness, &mut output).unwrap(),
OutputFormat::Human => full_report.print_human(&harness, &mut output)?,
OutputFormat::Json => full_report.print_json(&harness, &mut output)?,
OutputFormat::RustcJson => full_report.print_rustc_json(&harness, &mut output)?,
}

if full_report.failed() {
Expand Down
Loading

0 comments on commit d492efb

Please sign in to comment.