[m-rev.] for review: warn about unsigned comparisons against zero that are tautologies
Zoltan Somogyi
zoltan.somogyi at runbox.com
Sat Oct 20 19:31:43 AEDT 2018
On Sat, 20 Oct 2018 05:53:00 +0000 (UTC), Julien Fischer <jfischer at opturion.com> wrote:
> +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
I have two minor issues with this diff. The first is how you test whether
warn_simple_code is enabled. *Either* it should be tested in the
condition of this if-then-else, *or* it should be tested in the conditional
error message components and severity, but not both, since one is enough.
And if it is tested in this condition, it should be the *last* test: if you are
gonna fail, you wanna fail fast.
> +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.")]
The other issue is the wording of these messages.
I think something along the lines of "All uint8 values are >= 0u8"
would be clearer.
Other than that, the diff is fine.
Note that I think Peter is also working on this module, so one of you
will probably get a conflict :-(
Zoltan.
More information about the reviews
mailing list