Generalize WNEGCONSTCOMP to warn for all constants outside a type's range

Author: mandolaerik

RELEASENOTES-1.4.md

@@ -312,4 +312,7 @@
 - `release 6 6313`
 - `release 7 7026`
 - `note 6` The warning message for comparing a value of unsigned type to a negative constant has been improved to also warn for unsigned types shorter than 64 bits. 
-- `release 6 6315`
+- `release 6 6315`
+- `note 6` Added warnings for comparing an integer to constant values
+  outside the integer's range. E.g., given a variable `x` of type `uint8`,
+  warnings will now be reported on expressions like `x >= 256` or `x == 3.14`.

py/dml/ctree.py

@@ -1392,10 +1392,18 @@ def make(cls, site, lh, rh):
             and lh.constant and rh.constant):
             return mkBoolConstant(site, cls.eval_const(lh.value, rh.value))
         if lhtype.is_int:
+            if rh.constant and rhtype.is_arith and (
+                    cls.eval_const(lhtype.min(), rh.value)
+                    == cls.eval_const(lhtype.max(), rh.value)):
+                report(WTYPELIMITS(site, rh, lhtype))
             lh_maybe_negative = lhtype.signed
             lh = as_int(lh)
             lhtype = realtype(lh.ctype())
         if rhtype.is_int:
+            if lh.constant and lhtype.is_arith and (
+                    cls.eval_const(lh.value, rhtype.min())
+                    == cls.eval_const(lh.value, rhtype.max())):
+                report(WTYPELIMITS(site, lh, rhtype))
             rh_maybe_negative = rhtype.signed
             rh = as_int(rh)
             rhtype = realtype(rh.ctype())
@@ -1404,9 +1412,7 @@ def make(cls, site, lh, rh):
             and lh_maybe_negative != rh_maybe_negative):
             (signed_expr, unsigned_expr) = ((lh, rh) if lh_maybe_negative
                                             else (rh, lh))
-            if signed_expr.constant and signed_expr.value < 0:
-                report(WNEGCONSTCOMP(site, signed_expr,
-                                     unsigned_expr.ctype()))
+
             # we must convert (uint64)x < (int64)y to DML_lt(x, y), because
             # C:'s < would do an unsigned comparison. No need to do this if y
             # is a small constant, though.
@@ -1602,10 +1608,18 @@ def make(cls, site, lh, rh):
                                                  AddressOfMethod))
                     return mkBoolConstant(site, False)
         if lhtype.is_int:
+            if rh.constant and rhtype.is_arith and (
+                    not (lhtype.min() <= rh.value <= lhtype.max())
+                    or (rhtype.is_float and rh.value % 1)):
+                report(WTYPELIMITS(site, rh, lhtype))
             lh_maybe_negative = lhtype.signed
             lh = as_int(lh)
             lhtype = realtype(lh.ctype())
         if rhtype.is_int:
+            if lh.constant and lhtype.is_arith and (
+                    not (rhtype.min() <= lh.value <= rhtype.max())
+                    or (lhtype.is_float and lh.value % 1)):
+                report(WTYPELIMITS(site, lh, rhtype))
             rh_maybe_negative = rhtype.signed
             rh = as_int(rh)
             rhtype = realtype(rh.ctype())
@@ -1618,8 +1632,6 @@ def make(cls, site, lh, rh):
             # unsigned to a constant literal.
             (signed_expr, unsigned_expr) = ((lh, rh) if lh_maybe_negative
                                             else (rh, lh))
-            if signed_expr.constant and signed_expr.value < 0:
-                report(WNEGCONSTCOMP(site, signed_expr, unsigned_expr.ctype()))
             if not (signed_expr.constant and 0 <= signed_expr.value < 1 << 63):
                 return mkApply(
                     site, mkLit(

py/dml/ctree_test.py

@@ -1186,9 +1186,9 @@ def compare_numeric(self, lh, rh, op, result):
         rh_var = variable('rh', rh.ctype())
         var_cond = op(site, lh_var, rh_var)
         # compare constant and variable
-        logging.ignore_warning('WNEGCONSTCOMP')
+        logging.ignore_warning('WTYPELIMITS')
         mix_cond = op(site, lh, rh_var)
-        logging.enable_warning('WNEGCONSTCOMP')
+        logging.enable_warning('WTYPELIMITS')
         self.assertIsInstance(var_cond.ctype(), types.TBool)
         self.assertIsInstance(mix_cond.ctype(), types.TBool)
         return ['%s = %s;' % (lh.ctype().declaration('lh'), lh.read()),

py/dml/messages.py

@@ -2110,16 +2110,18 @@ def log(self):
             self.other_site,
             "original non-throwing declaration")
 
-class WNEGCONSTCOMP(DMLWarning):
-    """DML uses a special method when comparing an unsigned and signed integer,
-    meaning that comparing a negative constant to an unsigned integer always
-    has the same result, which is usually not the intended behaviour."""
+class WTYPELIMITS(DMLWarning):
+    """When comparing an integer with a constant outside the range of the
+    integer's type, the comparison always has the same result. One
+    common example is that a value of type `uint64` always is
+    considered to be different from -1; in this case, the constant
+    can be rewritten as `cast(-1, uint64)`.
+    """
     def __init__(self, site, expr, ty):
-        DMLWarning.__init__(self, site)
+        super().__init__(site)
         self.expr = expr
         self.ty = ty
-    fmt = ("Comparing negative constant to unsigned integer has a constant "
-           + "result")
+    fmt = 'Comparison with constant outside the range of data type'
     def log(self):
         DMLError.log(self)
         self.print_site_message(

py/dml/types.py

@@ -511,6 +511,17 @@ def __init__(self, bits, signed, members=None, const=False):
     is_arith = True
     is_endian = False
 
+    def min(self):
+        if self.signed:
+            return -(1 << (self.bits - 1))
+        else:
+            return 0
+    def max(self):
+        if self.signed:
+            return (1 << (self.bits - 1)) - 1
+        else:
+            return (1 << self.bits) - 1
+
     @property
     def is_bitfields(self):
         return self.members is not None

test/1.4/errors/T_WNEGCONSTCOMP.dml

@@ -0,0 +1,88 @@
+/*
+  © 2021 Intel Corporation
+  SPDX-License-Identifier: MPL-2.0
+*/
+dml 1.4;
+
+device test;
+
+method init() {
+    local uint64 a = -1;
+    // Test all operands
+    /// WARNING WTYPELIMITS
+    assert a != -1;
+    /// WARNING WTYPELIMITS
+    assert -1 != a;
+    /// WARNING WTYPELIMITS
+    assert !(-1 == a);
+    /// WARNING WTYPELIMITS
+    assert a > -1;
+    /// WARNING WTYPELIMITS
+    assert !(a < -1);
+    /// WARNING WTYPELIMITS
+    assert a >= -1;
+    /// WARNING WTYPELIMITS
+    assert !(a <= -1);
+
+    // Upper and lower bounds are tight, regardless if the constant is on RH or
+    // LH side
+    local uint8 a8 = -1;
+    /// WARNING WTYPELIMITS
+    assert a8 != -1;
+    /// WARNING WTYPELIMITS
+    assert -1 != a8;
+    // no warning
+    assert a8 != 0;
+    assert 0 != a8;
+    /// WARNING WTYPELIMITS
+    assert a8 != 256;
+    /// WARNING WTYPELIMITS
+    assert 256 != a8;
+    // no warning
+    assert a8 == 255;
+    assert 255 == a8;
+    /// WARNING WTYPELIMITS
+    assert a8 >= 0;
+    /// WARNING WTYPELIMITS
+    assert 0 <= a8;
+    /// no warning
+    assert a8 > 0;
+    assert 0 < a8;
+    /// WARNING WTYPELIMITS
+    assert !(a8 > 255);
+    /// WARNING WTYPELIMITS
+    assert !(255 < a8);
+    /// no warning
+    assert a8 >= 255;
+    assert 255 <= a8;
+
+    // Limits for signed integers are tight
+    local int8 i8 = 128;
+    /// WARNING WTYPELIMITS
+    assert i8 != 128;
+    // no warning
+    assert i8 != 127;
+    /// WARNING WTYPELIMITS
+    assert i8 != -129;
+    // no warning
+    assert i8 == -128;
+
+    // Non-integer float constants also give warnings
+    /// WARNING WTYPELIMITS
+    assert i8 != 0.1;
+    /// WARNING WTYPELIMITS
+    assert 0.1 != i8;
+    // no warning
+    assert i8 != 0.0;
+    assert 0.0 != i8;
+
+    // Sanity
+    local int64 b = -1;
+    // No warning
+    assert b == -1;
+    // No warning
+    assert !(b < -1);
+
+    // Test workaround
+    assert a == cast(-1, uint64);
+}