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

[Peter Wang]

On Fri, 16 May 2025 03:46:37 +1000 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by anyone. I will update the user guide once
> the new documentation in options.m is approved.
> 
> Much of the diff consists of me acting on the new warnings,
> so you can see how useful they are. I have not yet acted on
> *all* such warnings, because this diff is large enough already;
> that will come later.
> 
> Zoltan

> 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.
> 

I've had a chance to try this change on the Prince codebase now.
A common pattern is a predicate that takes the I/O state as unique input
in order to read the "global state", but the I/O state needs to be
threaded through its caller (for whatever reason).

For example:

    :- pred p(int::in, io::di, io::uo) is det.

    p(_, !IO) :-
	read_global_state(!.IO).

    :- pred read_global_state(io::ui) is det.

    read_global_state(IO) :- ...

The unneeded final statevars warning suggests that !:IO could be deleted
in p/3, but it cannot be written like so:

    p(_, IO, IO) :-
	read_global_state(IO).

as the modechecker will complain that IO has instantiatedness `ground'.

To avoid the warning, it is necessary to write out the unification
previously implied by state variable notation:

    p(_, IO0, IO) :-
	read_global_state(IO0),
	IO = IO0.

I guess, ideally, the compiler would recognise that a final statevar is
not "unneeded" if the statevar occurs as a unique-input argument in a
call (or some generalisation of that).

Peter
-------------- next part --------------
:- module unique_input.
:- interface.

:- import_module io.

:- pred main(io::di, io::uo) is det.

%-----------------------------------------------------------------------------%

:- implementation.

:- import_module list.

main(!IO) :-
    foldl(p, [], !IO),
    foldl(q, [], !IO).

:- pred p(int::in, io::di, io::uo) is det.

p(_, !IO) :-
    read_global_state(!.IO),
    read_global_state(!.IO).

:- pred q(int::in, io::di, io::uo) is det.

q(_, IO0, IO) :-
    read_global_state(IO0),
    read_global_state(IO0),
    IO = IO0.

:- pred read_global_state(io::ui) is det.

read_global_state(_).