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

fix: less egregiously abuse random unwraps #66

Merged
merged 1 commit into from
Jul 13, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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
Loading