diff --git a/RELEASENOTES-1.4.md b/RELEASENOTES-1.4.md index 64dfd534..ab1201a2 100644 --- a/RELEASENOTES-1.4.md +++ b/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` \ No newline at end of file +- `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`. diff --git a/lib/1.4/dml-builtins.dml b/lib/1.4/dml-builtins.dml index 47741633..189dcd3a 100644 --- a/lib/1.4/dml-builtins.dml +++ b/lib/1.4/dml-builtins.dml @@ -2839,7 +2839,8 @@ template register is (_conf_attribute, get, set, shown_desc, param _le_byte_order : bool; param _is_read_only : bool; param mapped : bool; - param mapped = offset != unmapped_offset; + // the explicit cast avoids WTYPELIMITS + param mapped = cast(offset, uint64) != unmapped_offset; shared independent method _size() -> (int) { return this.bitsize / 8; } shared method _num_fields() -> (uint32) { diff --git a/py/dml/ctree.py b/py/dml/ctree.py index 31bddcfe..fcfd877e 100644 --- a/py/dml/ctree.py +++ b/py/dml/ctree.py @@ -1392,10 +1392,20 @@ 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, cls.eval_const(lhtype.min(), rh.value), + 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, cls.eval_const(lh.value, rhtype.min()), + lh, rhtype)) rh_maybe_negative = rhtype.signed rh = as_int(rh) rhtype = realtype(rh.ctype()) @@ -1404,9 +1414,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. @@ -1562,7 +1570,7 @@ class Equals(BinOp): op = '==' @classmethod - def make(cls, site, lh, rh): + def make(cls, site, lh, rh, not_equal=False): lhtype = realtype(lh.ctype()) rhtype = realtype(rh.ctype()) @@ -1602,10 +1610,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, not_equal, 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, not_equal, lh, rhtype)) rh_maybe_negative = rhtype.signed rh = as_int(rh) rhtype = realtype(rh.ctype()) @@ -1618,8 +1634,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( @@ -1696,7 +1710,7 @@ def mkNotEquals(site, lh, rh): if dml.globals.compat_dml12_int(site): return NotEquals_dml12.make(site, lh, rh) else: - return mkNot(site, Equals.make(site, lh, rh)) + return mkNot(site, Equals.make(site, lh, rh, True)) def usual_int_conv(lh, lhtype, rh, rhtype): assert lhtype.is_int and rhtype.is_int diff --git a/py/dml/ctree_test.py b/py/dml/ctree_test.py index ec8e2750..7ffa14ad 100644 --- a/py/dml/ctree_test.py +++ b/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()), diff --git a/py/dml/messages.py b/py/dml/messages.py index 8d7b9cc2..1fca22e0 100644 --- a/py/dml/messages.py +++ b/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.""" - def __init__(self, site, expr, ty): - DMLWarning.__init__(self, site) +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, result, expr, ty): + super().__init__(site, "true" if result else "false") self.expr = expr self.ty = ty - fmt = ("Comparing negative constant to unsigned integer has a constant " - + "result") + fmt = 'comparison is always %s due to limited range of data type' def log(self): DMLError.log(self) self.print_site_message( diff --git a/py/dml/types.py b/py/dml/types.py index a440417d..5aa4bd63 100644 --- a/py/dml/types.py +++ b/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 diff --git a/test/1.4/errors/T_WNEGCONSTCOMP.dml b/test/1.4/errors/T_WNEGCONSTCOMP.dml deleted file mode 100644 index fcdd4f24..00000000 --- a/test/1.4/errors/T_WNEGCONSTCOMP.dml +++ /dev/null @@ -1,36 +0,0 @@ -/* - © 2021 Intel Corporation - SPDX-License-Identifier: MPL-2.0 -*/ -dml 1.4; - -device test; - -method init() { - local uint64 a = -1; - /// WARNING WNEGCONSTCOMP - assert a != -1; - /// WARNING WNEGCONSTCOMP - assert a > -1; - /// WARNING WNEGCONSTCOMP - assert !(a < -1); - /// WARNING WNEGCONSTCOMP - assert a >= -1; - /// WARNING WNEGCONSTCOMP - assert !(a <= -1); - local uint8 a8 = -1; - /// WARNING WNEGCONSTCOMP - assert a8 != -1; - /// WARNING WNEGCONSTCOMP - assert a8 > -1; - - // Sanity - local int64 b = -1; - // No warning - assert b == -1; - // No warning - assert !(b < -1); - - // Test workaround - assert a == cast(-1, uint64); -} diff --git a/test/1.4/errors/T_WTYPELIMITS.dml b/test/1.4/errors/T_WTYPELIMITS.dml new file mode 100644 index 00000000..d5eba164 --- /dev/null +++ b/test/1.4/errors/T_WTYPELIMITS.dml @@ -0,0 +1,98 @@ +/* + © 2021 Intel Corporation + SPDX-License-Identifier: MPL-2.0 +*/ +dml 1.4; + +device test; + +param inf = 1.0/+0.0; +param nan = inf/inf; + +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); + // No warnings for inf and nan, as they aren't considered constant by DML + assert a < inf; + assert a > -inf; + assert a != -inf; + assert !(a < nan); + assert !(a > nan); + assert a != nan; + + // Test workaround + assert a == cast(-1, uint64); +}