diff --git a/compiler/qsc_linter/src/lints/hir.rs b/compiler/qsc_linter/src/lints/hir.rs index cddb2d2019..9f71da8dee 100644 --- a/compiler/qsc_linter/src/lints/hir.rs +++ b/compiler/qsc_linter/src/lints/hir.rs @@ -6,10 +6,10 @@ use std::rc::Rc; use qsc_data_structures::span::Span; use qsc_hir::{ hir::{ - CallableDecl, CallableKind, Expr, ExprKind, Field, ItemKind, Res, SpecBody, SpecDecl, Stmt, - StmtKind, + BinOp, CallableDecl, CallableKind, Expr, ExprKind, Field, ItemKind, Res, SpecBody, + SpecDecl, Stmt, StmtKind, }, - ty::Ty, + ty::{Prim, Ty}, visit::{self, Visitor}, }; @@ -35,12 +35,42 @@ use super::lint; // For more details on how to add a lint, please refer to the crate-level documentation // in qsc_linter/lib.rs. declare_hir_lints! { + (DoubleEquality, LintLevel::Warn, "strict comparison of doubles", "consider comparing them with some margin of error"), (NeedlessOperation, LintLevel::Allow, "operation does not contain any quantum operations", "this callable can be declared as a function instead"), (DeprecatedFunctionConstructor, LintLevel::Allow, "deprecated function constructors", "function constructors for struct types are deprecated, use `new` instead"), (DeprecatedWithOperator, LintLevel::Allow, "deprecated `w/` and `w/=` operators for structs", "`w/` and `w/=` operators for structs are deprecated, use `new` instead"), (DeprecatedDoubleColonOperator, LintLevel::Allow, "deprecated `::` for field access", "`::` operator is deprecated, use `.` instead"), } +#[derive(Default)] +struct DoubleEquality { + level: LintLevel, +} + +impl HirLintPass for DoubleEquality { + fn check_expr(&mut self, expr: &Expr, buffer: &mut Vec, _compilation: Compilation) { + // Ignore the case where the identifier is being compared for equality with itself, + // Since this is used to check if the double is `NaN`. This will be compiled to + // fcmp oeq value value + // in LLVM because we only implemented the ord side of fcmp. + // See https://llvm.org/docs/LangRef.html#id313 for more details. + if let ExprKind::BinOp(BinOp::Eq, ref lhs, ref rhs) = expr.kind { + if let (ExprKind::Var(lhs_id, _), ExprKind::Var(rhs_id, _)) = (&lhs.kind, &rhs.kind) { + if lhs_id == rhs_id { + return; + } + } + } + + if let ExprKind::BinOp(BinOp::Eq | BinOp::Neq, ref lhs, ref rhs) = expr.kind { + if matches!(lhs.ty, Ty::Prim(Prim::Double)) && matches!(rhs.ty, Ty::Prim(Prim::Double)) + { + buffer.push(lint!(self, expr.span)); + } + } + } +} + /// Helper to check if an operation has desired operation characteristics /// /// empty operations: no lint, special case of `I` operation used for Delay diff --git a/compiler/qsc_linter/src/tests.rs b/compiler/qsc_linter/src/tests.rs index b9a014bf34..d6f4487f00 100644 --- a/compiler/qsc_linter/src/tests.rs +++ b/compiler/qsc_linter/src/tests.rs @@ -214,6 +214,58 @@ fn division_by_zero() { ); } +#[test] +fn double_equality() { + check( + &wrap_in_callable("1.0 == 1.01;", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "1.0 == 1.01", + level: Warn, + message: "strict comparison of doubles", + help: "consider comparing them with some margin of error", + code_action_edits: [], + }, + ] + "#]], + ); +} + +#[test] +fn check_double_equality_with_itself_is_allowed_for_nan_check() { + check( + &wrap_in_callable( + r#" + let a = 1.0; + let is_nan = not (a == a); + "#, + CallableKind::Function, + ), + &expect![[r#" + [] + "#]], + ); +} + +#[test] +fn double_inequality() { + check( + &wrap_in_callable("1.0 != 1.01;", CallableKind::Function), + &expect![[r#" + [ + SrcLint { + source: "1.0 != 1.01", + level: Warn, + message: "strict comparison of doubles", + help: "consider comparing them with some margin of error", + code_action_edits: [], + }, + ] + "#]], + ); +} + #[test] fn needless_parens_in_assignment() { check( diff --git a/samples/estimation/df-chemistry/src/DoubleFactorizedChemistry.qs b/samples/estimation/df-chemistry/src/DoubleFactorizedChemistry.qs index 58407d788d..5d1617aaba 100644 --- a/samples/estimation/df-chemistry/src/DoubleFactorizedChemistry.qs +++ b/samples/estimation/df-chemistry/src/DoubleFactorizedChemistry.qs @@ -341,7 +341,7 @@ internal function AllEigenVectorsAsBitString(eigenVectors : Double[][], precisio for index in 0..Length(eigenVector) - 2 { // We apply MinD, such that rounding errors do not lead to // an argument for ArcCos which is larger than 1.0. (p. 52, eq. 56) - let theta = sins == 0.0 ? 0.0 | 0.5 * ArcCos(MinD(eigenVector[index] / sins, 1.0)); + let theta = not (AbsD(sins) > 0.0) ? 0.0 | 0.5 * ArcCos(MinD(eigenVector[index] / sins, 1.0)); // all angles as bit string let factor = theta / tau; @@ -357,5 +357,5 @@ internal function AllEigenVectorsAsBitString(eigenVectors : Double[][], precisio } internal function IsNaN(value : Double) : Bool { - value != value + not (value == value) } diff --git a/samples/estimation/df-chemistry/src/Prepare.qs b/samples/estimation/df-chemistry/src/Prepare.qs index 688f3892a7..504b5652b3 100644 --- a/samples/estimation/df-chemistry/src/Prepare.qs +++ b/samples/estimation/df-chemistry/src/Prepare.qs @@ -116,7 +116,7 @@ internal function DiscretizedProbabilityDistribution(bitsPrecision : Int, coeffi let nCoefficients = Length(coefficients); Fact(bitsPrecision <= 31, $"Bits of precision {bitsPrecision} unsupported. Max is 31."); Fact(nCoefficients > 1, "Cannot prepare state with less than 2 coefficients."); - Fact(oneNorm != 0.0, "State must have at least one coefficient > 0"); + Fact(AbsD(oneNorm) > 0.0, "State must have at least one coefficient > 0"); let barHeight = 2^bitsPrecision - 1;