Skip to content

Commit

Permalink
Merge pull request #636 from Bzero/assert_eq_floating_point_note
Browse files Browse the repository at this point in the history
Add note regarding floating point errors to assert_eq
  • Loading branch information
sharkdp authored Oct 29, 2024
2 parents e889cc2 + 1b618c6 commit f064611
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 19 deletions.
12 changes: 7 additions & 5 deletions numbat/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -500,16 +500,18 @@ impl ErrorDiagnostic for RuntimeError {
.with_labels(vec![span
.diagnostic_label(LabelStyle::Primary)
.with_message("assertion failed")])],
RuntimeError::AssertEq2Failed(span_lhs, lhs, span_rhs, rhs) => {
RuntimeError::AssertEq2Failed(assert_eq2_error) => {
vec![Diagnostic::error()
.with_message("Assertion failed")
.with_labels(vec![
span_lhs
assert_eq2_error
.span_lhs
.diagnostic_label(LabelStyle::Secondary)
.with_message(format!("{lhs}")),
span_rhs
.with_message(format!("{}", assert_eq2_error.lhs)),
assert_eq2_error
.span_rhs
.diagnostic_label(LabelStyle::Primary)
.with_message(format!("{rhs}")),
.with_message(format!("{}", assert_eq2_error.rhs)),
])
.with_notes(vec![inner])]
}
Expand Down
22 changes: 14 additions & 8 deletions numbat/src/ffi/procedures.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,14 @@ use std::sync::OnceLock;

use super::macros::*;
use crate::{
ast::ProcedureKind, ffi::ControlFlow, interpreter::assert_eq_3::AssertEq3Error,
pretty_print::PrettyPrint, span::Span, value::Value, vm::ExecutionContext, RuntimeError,
ast::ProcedureKind,
ffi::ControlFlow,
interpreter::assert_eq::{AssertEq2Error, AssertEq3Error},
pretty_print::PrettyPrint,
span::Span,
value::Value,
vm::ExecutionContext,
RuntimeError,
};

use super::{Args, Callable, ForeignFunction};
Expand Down Expand Up @@ -81,12 +87,12 @@ fn assert_eq(_: &mut ExecutionContext, mut args: Args, arg_spans: Vec<Span>) ->
let lhs = arg!(args);
let rhs = arg!(args);

let error = ControlFlow::Break(RuntimeError::AssertEq2Failed(
span_lhs,
lhs.clone(),
span_rhs,
rhs.clone(),
));
let error = ControlFlow::Break(RuntimeError::AssertEq2Failed(AssertEq2Error {
span_lhs: span_lhs,
lhs: lhs.clone(),
span_rhs: span_rhs,
rhs: rhs.clone(),
}));

if lhs.is_quantity() {
let lhs = lhs.unsafe_as_quantity();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,33 @@
use crate::{quantity::Quantity, span::Span};
use crate::{quantity::Quantity, span::Span, value::Value};
use compact_str::{format_compact, CompactString};
use std::fmt::Display;
use thiserror::Error;

#[derive(Debug, Clone, Error, PartialEq, Eq)]
pub struct AssertEq2Error {
pub span_lhs: Span,
pub lhs: Value,
pub span_rhs: Span,
pub rhs: Value,
}

impl Display for AssertEq2Error {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
let optional_message = if format!("{}", self.lhs) == format!("{}", self.rhs) {
"\nNote: The two printed values appear to be the same, this may be due to floating point precision errors.\n \
For dimension types you may want to test approximate equality instead: assert_eq(q1, q2, ε)."
} else {
""
};

write!(
f,
"Assertion failed because the following two values are not the same:\n {}\n {}{}",
self.lhs, self.rhs, optional_message
)
}
}

#[derive(Debug, Clone, Error, PartialEq, Eq)]
pub struct AssertEq3Error {
pub span_lhs: Span,
Expand Down
8 changes: 4 additions & 4 deletions numbat/src/interpreter/mod.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
pub(crate) mod assert_eq_3;
pub(crate) mod assert_eq;

use crate::{
dimension::DimensionRegistry,
Expand All @@ -12,7 +12,7 @@ use crate::{

pub use crate::markup as m;

use assert_eq_3::AssertEq3Error;
use assert_eq::{AssertEq2Error, AssertEq3Error};
use compact_str::{CompactString, ToCompactString};
use thiserror::Error;

Expand All @@ -32,8 +32,8 @@ pub enum RuntimeError {
QuantityError(QuantityError),
#[error("Assertion failed")]
AssertFailed(Span),
#[error("Assertion failed because the following two values are not the same:\n {1}\n {3}")]
AssertEq2Failed(Span, Value, Span, Value),
#[error("{0}")]
AssertEq2Failed(AssertEq2Error),
#[error("{0}")]
AssertEq3Failed(AssertEq3Error),
#[error("Could not load exchange rates from European Central Bank.")]
Expand Down
18 changes: 17 additions & 1 deletion numbat/tests/interpreter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -856,7 +856,7 @@ fn test_statement_pretty_printing() {
mod tests {
use super::*;
#[cfg(test)]
mod assert_eq_3 {
mod assert_eq {
use super::*;

#[test]
Expand Down Expand Up @@ -960,5 +960,21 @@ mod tests {
-77.0089°
"###);
}

#[test]
fn test_floating_point_warning() {
insta::assert_snapshot!(fail("assert_eq(2+ 2, 2 + 1)"), @r###"
Assertion failed because the following two values are not the same:
4
3
"###);
insta::assert_snapshot!(fail("assert_eq(2 + 2e-12, 2 + 1e-12)"), @r###"
Assertion failed because the following two values are not the same:
2.0
2.0
Note: The two printed values appear to be the same, this may be due to floating point precision errors.
For dimension types you may want to test approximate equality instead: assert_eq(q1, q2, ε).
"###);
}
}
}

0 comments on commit f064611

Please sign in to comment.