[m-rev.] for review: warn about unsigned comparisons against zero that are tautologies

Julien Fischer jfischer at opturion.com
Sat Oct 20 16:53:00 AEDT 2018


For review by anyone.

-----------------

Warn about unsigned comparisons against zero that are tautologies etc.

If --warn-simple-code is enabled, then emit a warning for comparisons of
unsigned integer values against zero if that comparison is a tautology or
contradiction.

compiler/simplify_goal_call.m:
    Implement the new warning.

tests/warnings/Mmakefile:
tests/warnings/unsigned_zero_cmp.{m,exp}:
    Add a test of the new warning.

Julien.

diff --git a/compiler/simplify_goal_call.m b/compiler/simplify_goal_call.m
index 814002d..adf1e9d 100644
--- a/compiler/simplify_goal_call.m
+++ b/compiler/simplify_goal_call.m
@@ -130,6 +130,8 @@ simplify_goal_plain_call(GoalExpr0, GoalExpr, GoalInfo0, GoalInfo,
      maybe_generate_warning_for_infinite_loop_call(PredId, ProcId,
          Args, IsBuiltin, PredInfo, ProcInfo, GoalInfo0, NestedContext,
          Common0, !Info),
+    maybe_generate_warning_for_useless_comparison(PredInfo,
+        InstMap0, Args, GoalInfo0, !Info),

      % Try to evaluate the call at compile-time.
      ModuleSymName = pred_info_module(PredInfo),
@@ -595,6 +597,111 @@ input_args_are_equiv([Arg | Args], [HeadVar | HeadVars], [Mode | Modes],
      ),
      input_args_are_equiv(Args, HeadVars, Modes, CommonInfo, ModuleInfo).

+%---------------------%
+
+    % Generate warnings for comparisons at are tautologies or contradictions.
+    % For example:
+    %
+    %      X : uint32 < 0u32
+    %      X : uint >= 0u
+    %
+:- pred maybe_generate_warning_for_useless_comparison(
+    pred_info::in, instmap::in, list(prog_var)::in,
+    hlds_goal_info::in, simplify_info::in, simplify_info::out) is det.
+
+maybe_generate_warning_for_useless_comparison(
+        PredInfo, InstMap, Args, GoalInfo, !Info) :-
+    pred_info_is_pred_or_func(PredInfo) = PredOrFunc,
+    ModuleSymName = pred_info_module(PredInfo),
+    ( if
+        simplify_do_warn_simple_code(!.Info),
+        is_std_lib_module_name(ModuleSymName, ModuleName),
+        PredOrFunc = pf_predicate,
+        Args = [ArgA, ArgB]
+    then
+        pred_info_get_name(PredInfo, PredName),
+        instmap_lookup_var(InstMap, ArgA, InstA),
+        instmap_lookup_var(InstMap, ArgB, InstB),
+        ( if
+            is_useless_unsigned_comparison(ModuleName, PredName, InstA,
+                InstB, WarnPieces)
+        then
+            GoalContext = goal_info_get_context(GoalInfo),
+            PredPieces = describe_one_pred_info_name(should_module_qualify,
+                PredInfo),
+            Pieces = [words("Warning: call to")] ++ PredPieces ++
+                WarnPieces,
+            Msg = simple_msg(GoalContext,
+                [option_is_set(warn_simple_code, yes,
+                    [always(Pieces)])]),
+            Severity = severity_conditional(warn_simple_code, yes,
+                severity_warning, no),
+            Spec = error_spec(Severity,
+                phase_simplify(report_in_any_mode), [Msg]),
+            simplify_info_add_message(Spec, !Info)
+        else
+            true
+        )
+    else
+        true
+    ).
+
+:- pred is_useless_unsigned_comparison(string::in, string::in, mer_inst::in,
+    mer_inst::in, format_components::out) is semidet.
+
+is_useless_unsigned_comparison(ModuleName, PredName, ArgA, ArgB,
+        Pieces) :-
+    (
+        PredName = ">=",
+        arg_is_unsigned_zero(ModuleName, ArgB, ZeroStr),
+        Pieces = [words("cannot fail."), nl,
+            words("Comparison of"), words(ModuleName),
+            words(">="), words(ZeroStr), words("is always true.")]
+    ;
+        PredName = "=<",
+        arg_is_unsigned_zero(ModuleName, ArgA, ZeroStr),
+        Pieces = [words("cannot fail."), nl,
+            words("Comparison of"), words(ZeroStr), words("=<"),
+            words(ModuleName), words("is always true.")]
+    ;
+        PredName = "<",
+        arg_is_unsigned_zero(ModuleName, ArgB, ZeroStr),
+        Pieces = [words("cannot succeed."), nl,
+            words("Comparison of"), words(ModuleName),words("<"),
+            words(ZeroStr), words("is always false.")]
+    ;
+        PredName = ">",
+        arg_is_unsigned_zero(ModuleName, ArgA, ZeroStr),
+        Pieces = [words("cannot succeed."), nl,
+            words("Comparison of"), words(ZeroStr), words(">"),
+            words(ModuleName), words("is always false.")]
+    ).
+
+:- pred arg_is_unsigned_zero(string::in, mer_inst::in, string::out) is semidet.
+
+arg_is_unsigned_zero(ModuleName, Arg, ZeroStr) :-
+    (
+        ModuleName = "uint",
+        Arg = bound(_, _, [bound_functor(uint_const(0u), [])]),
+        ZeroStr = "0u"
+    ;
+        ModuleName = "uint8",
+        Arg = bound(_, _, [bound_functor(uint8_const(0u8), [])]),
+        ZeroStr = "0u8"
+    ;
+        ModuleName = "uint16",
+        Arg = bound(_, _, [bound_functor(uint16_const(0u16), [])]),
+        ZeroStr = "0u16"
+    ;
+        ModuleName = "uint32",
+        Arg = bound(_, _, [bound_functor(uint32_const(0u32), [])]),
+        ZeroStr = "0u32"
+    ;
+        ModuleName = "uint64",
+        Arg = bound(_, _, [bound_functor(uint64_const(0u64), [])]),
+        ZeroStr = "0u64"
+    ).
+
  %---------------------------------------------------------------------------%
  %
  % Predicates that improve the code, if they can.
diff --git a/tests/warnings/Mmakefile b/tests/warnings/Mmakefile
index 6216631..b065a1a 100644
--- a/tests/warnings/Mmakefile
+++ b/tests/warnings/Mmakefile
@@ -46,6 +46,7 @@ ERRORCHECK_PROGS = \
  	table_with_inline \
  	unify_f_g \
  	unify_x_f_x \
+	unsigned_zero_cmp \
  	unused_args_test \
  	unused_import \
  	unused_interface_import \
diff --git a/tests/warnings/unsigned_zero_cmp.exp b/tests/warnings/unsigned_zero_cmp.exp
index e69de29..3d964e5 100644
--- a/tests/warnings/unsigned_zero_cmp.exp
+++ b/tests/warnings/unsigned_zero_cmp.exp
@@ -0,0 +1,46 @@
+unsigned_zero_cmp.m:028: Warning: call to predicate `uint.<'/2 cannot succeed.
+unsigned_zero_cmp.m:028:   Comparison of uint < 0u is always false.
+unsigned_zero_cmp.m:029: Warning: call to predicate `uint.>'/2 cannot succeed.
+unsigned_zero_cmp.m:029:   Comparison of 0u > uint is always false.
+unsigned_zero_cmp.m:030: Warning: call to predicate `uint.=<'/2 cannot fail.
+unsigned_zero_cmp.m:030:   Comparison of 0u =< uint is always true.
+unsigned_zero_cmp.m:031: Warning: call to predicate `uint.>='/2 cannot fail.
+unsigned_zero_cmp.m:031:   Comparison of uint >= 0u is always true.
+unsigned_zero_cmp.m:034: Warning: call to predicate `uint8.<'/2 cannot succeed.
+unsigned_zero_cmp.m:034:   Comparison of uint8 < 0u8 is always false.
+unsigned_zero_cmp.m:035: Warning: call to predicate `uint8.>'/2 cannot succeed.
+unsigned_zero_cmp.m:035:   Comparison of 0u8 > uint8 is always false.
+unsigned_zero_cmp.m:036: Warning: call to predicate `uint8.=<'/2 cannot fail.
+unsigned_zero_cmp.m:036:   Comparison of 0u8 =< uint8 is always true.
+unsigned_zero_cmp.m:037: Warning: call to predicate `uint8.>='/2 cannot fail.
+unsigned_zero_cmp.m:037:   Comparison of uint8 >= 0u8 is always true.
+unsigned_zero_cmp.m:040: Warning: call to predicate `uint16.<'/2 cannot
+unsigned_zero_cmp.m:040:   succeed.
+unsigned_zero_cmp.m:040:   Comparison of uint16 < 0u16 is always false.
+unsigned_zero_cmp.m:041: Warning: call to predicate `uint16.>'/2 cannot
+unsigned_zero_cmp.m:041:   succeed.
+unsigned_zero_cmp.m:041:   Comparison of 0u16 > uint16 is always false.
+unsigned_zero_cmp.m:042: Warning: call to predicate `uint16.=<'/2 cannot fail.
+unsigned_zero_cmp.m:042:   Comparison of 0u16 =< uint16 is always true.
+unsigned_zero_cmp.m:043: Warning: call to predicate `uint16.>='/2 cannot fail.
+unsigned_zero_cmp.m:043:   Comparison of uint16 >= 0u16 is always true.
+unsigned_zero_cmp.m:046: Warning: call to predicate `uint32.<'/2 cannot
+unsigned_zero_cmp.m:046:   succeed.
+unsigned_zero_cmp.m:046:   Comparison of uint32 < 0u32 is always false.
+unsigned_zero_cmp.m:047: Warning: call to predicate `uint32.>'/2 cannot
+unsigned_zero_cmp.m:047:   succeed.
+unsigned_zero_cmp.m:047:   Comparison of 0u32 > uint32 is always false.
+unsigned_zero_cmp.m:048: Warning: call to predicate `uint32.=<'/2 cannot fail.
+unsigned_zero_cmp.m:048:   Comparison of 0u32 =< uint32 is always true.
+unsigned_zero_cmp.m:049: Warning: call to predicate `uint32.>='/2 cannot fail.
+unsigned_zero_cmp.m:049:   Comparison of uint32 >= 0u32 is always true.
+unsigned_zero_cmp.m:052: Warning: call to predicate `uint64.<'/2 cannot
+unsigned_zero_cmp.m:052:   succeed.
+unsigned_zero_cmp.m:052:   Comparison of uint64 < 0u64 is always false.
+unsigned_zero_cmp.m:053: Warning: call to predicate `uint64.>'/2 cannot
+unsigned_zero_cmp.m:053:   succeed.
+unsigned_zero_cmp.m:053:   Comparison of 0u64 > uint64 is always false.
+unsigned_zero_cmp.m:054: Warning: call to predicate `uint64.=<'/2 cannot fail.
+unsigned_zero_cmp.m:054:   Comparison of 0u64 =< uint64 is always true.
+unsigned_zero_cmp.m:055: Warning: call to predicate `uint64.>='/2 cannot fail.
+unsigned_zero_cmp.m:055:   Comparison of uint64 >= 0u64 is always true.
diff --git a/tests/warnings/unsigned_zero_cmp.m b/tests/warnings/unsigned_zero_cmp.m
index e69de29..ffe20d7 100644
--- a/tests/warnings/unsigned_zero_cmp.m
+++ b/tests/warnings/unsigned_zero_cmp.m
@@ -0,0 +1,55 @@
+%---------------------------------------------------------------------------%
+% vim: ts=4 sw=4 et ft=mercury
+%---------------------------------------------------------------------------%
+%
+% Test warnings for unsigned comparisons against zero that are always true
+% or always false.
+%
+%---------------------------------------------------------------------------%
+
+:- module unsigned_zero_cmp.
+:- interface.
+
+:- pred test_uint(uint::in) is semidet.
+:- pred test_uint8(uint8::in) is semidet.
+:- pred test_uint16(uint16::in) is semidet.
+:- pred test_uint32(uint32::in) is semidet.
+:- pred test_uint64(uint64::in) is semidet.
+
+:- implementation.
+
+:- import_module uint.
+:- import_module uint8.
+:- import_module uint16.
+:- import_module uint32.
+:- import_module uint64.
+
+test_uint(X) :-
+   X < 0u,
+   0u > X,
+   0u =< X,
+   X >= 0u.
+
+test_uint8(X) :-
+   X < 0u8,
+   0u8 > X,
+   0u8 =< X,
+   X >= 0u8.
+
+test_uint16(X) :-
+   X < 0u16,
+   0u16 > X,
+   0u16 =< X,
+   X >= 0u16.
+
+test_uint32(X) :-
+   X < 0u32,
+   0u32 > X,
+   0u32 =< X,
+   X >= 0u32.
+
+test_uint64(X) :-
+   X < 0u64,
+   0u64 > X,
+   0u64 =< X,
+   X >= 0u64.


More information about the reviews mailing list