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
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
fix: less egregiously abuse random unwraps
Gankra committed Jul 13, 2024

Unverified

The email in this signature doesn’t match the committer email.
commit 2ce38b8c6d0600f0285573ed88fa59e08c9b7c2e
2 changes: 1 addition & 1 deletion src/cli.rs
Original file line number Diff line number Diff line change
@@ -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()
8 changes: 4 additions & 4 deletions src/error.rs
Original file line number Diff line number Diff line change
@@ -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),
@@ -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),
}

29 changes: 29 additions & 0 deletions src/fivemat.rs
Original file line number Diff line number Diff line change
@@ -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 {
@@ -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;
}
33 changes: 22 additions & 11 deletions src/harness/check.rs
Original file line number Diff line number Diff line change
@@ -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));
@@ -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));
@@ -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));
@@ -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
@@ -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
@@ -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.
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
}

5 changes: 4 additions & 1 deletion src/harness/run.rs
Original file line number Diff line number Diff line change
@@ -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)?;
65 changes: 35 additions & 30 deletions src/log.rs
Original file line number Diff line number Diff line change
@@ -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;
@@ -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))
}
/*
@@ -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);
@@ -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 {
@@ -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
@@ -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));
}
@@ -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
6 changes: 3 additions & 3 deletions src/main.rs
Original file line number Diff line number Diff line change
@@ -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() {
Loading