[m-rev.] for review: warn about unneeded initial and final state vars

[Julien Fischer]

On Fri, 16 May 2025 at 03:46, Zoltan Somogyi <zoltan.somogyi at runbox.com> wrote:

> Add options to warn about unused state vars.
>
> The new --warn-unneeded-initial-statevar option asks the compiler to warn about
> code such as
>
>     pred_a(!.X, ...) :-
>         ... code that uses !.X, but does not update it ...
>
> In this case, the preferred fix is to just replace all occurrences
> of !.X with X.
>
> The new --warn-unneeded-final-statevar option asks the compiler to warn about
> code such as
>
>     pred_a(!X, ...) :-
>         ... code that maybe uses !.X, but does not update it ...
>
> In this case, the preferred fix also involves replacing all occurrences
> of !.X with X, but it also involves either deleting the argument
> containing !:X (the best option), or, if there is some reason why
> the predicate's signature must stay unchanged, to replace !:X with X as well.
> And if the clause body does not actually refer to either !.X or !:X, then
> *both* arguments represented by !X should be deleted.
> The first option is a style warning; the second option, due to the
> signature change it may call for, is a non-style warning.

We have a growing set of style warnings, and while we do have the option
--inhibit-style-warnings to turn them all off, we do not have its converse. I
think that it is probably worth adding --enable-all-style-warnings. It may
result in a lot of warnings, but it seems preferable to invoking the style
options one-by-one (and presumably having to and look up what some of them are
in between such invocations).

> Both options have a version whose name adds a "-lambda" suffix, and which
> does the same warnings for the heads of lambda expressions, not clauses.
>
> Note that several of the modules below, include some that help to implement
> the warnings also contain code changes that result from *acting* on
> the new warnings, e.g. by deleting unneeded statevar arguments.

...

> diff --git a/compiler/headvar_names.m b/compiler/headvar_names.m
> index 5a2cdfd04..3b71eb64d 100644
> --- a/compiler/headvar_names.m
> +++ b/compiler/headvar_names.m
> @@ -22,13 +22,28 @@
>  :- import_module libs.
>  :- import_module libs.globals.
>
> +:- import_module list.
> +
> +:- type maybe_look_for_unused_statevars
> +    --->    do_not_look_for_unused_statevars
> +            % No clause had any unused state variables.
> +    ;       look_for_unused_statevars(list(string)).
> +            % At least one clause had unused state variables.
> +            % The argument gives, for each head variable,
> +            % either its the consensus name (if it has one),
> +            % or an empty string (if it does not).
> +            % (pre_typecheck.m uses that info to decide whether an unused
> +            % state variable in one clause is justified by consistency
> +            % with anoher clause that *does* use that same state variable.)

s/anoher/another/

That looks fine otherwise.

Julien.