Add lint for double (in)equality (#2104)

This PR adds a lint for double equality and inequality.

It also updates the DF Chemistry sample to perform a correct `NAN`
check.

---------

Co-authored-by: Alex Hansen <[email protected]>

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<Lint>, _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

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(

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)
 }

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;