Re: [lmi] Unused but set variables

[Greg Chicares]

On 12/13/21 4:29 PM, Vadim Zeitlin wrote:
> 
>  After the CI was triggered by a recent commit to master removing
> gui_test.sh, there were two breakages: one was expected, as the script was
> used for running the GUI tests in the MinGW build (what wasn't expected is
> that it's still broken, after replacing the script with the manual
> invocation of wx_test, but this is a different topic),

Would you like me to restore 'gui_test.sh'?

> while another one
> was not, as it was due to the clang version change in Debian Sid which
> happened in the meanwhile. The new clang version, 13, has brought (at
> least) two new warnings: one is about not declaring a copy ctor when the
> assignment operator is declared (even if it's only declared as deleted,
> which seems a bit stupid) and happens only in wx headers and so only
> affects the autotools build which doesn't use -isystem for them.

We plan to upgrade to a debian 'bookworm' chroot for production
in January. Would that be the best time for a wx upgrade? Or
should we ignore this problem for now?

>  But the other new warning, "-Wunused-but-set-variable", does happen in lmi
> code itself and both of its occurrences seem to indicate actual problems:
> 
>  The first one is
> 
> group_values.cpp:366:20: error: variable 'eoy_inforce_lives' set but not used 
> [-Werror,-Wunused-but-set-variable] 
> 
> and, indeed, eoy_inforce_lives is assigned to, but never read from. I'm not
> sure what was meant here, but it seems suspicious that we go to the trouble
> of calling InforceLivesEoy() for each cell, yet never use the result. But
> if this is indeed the right thing to do, could we remove eoy_inforce_lives
> entirely?

This command:
  git grep eoy_inforce_lives
demonstrates that the variable is never used, so it should be
okay to remove it (after testing of course). IIRC, in the past
it was used to allocate a special amount to lives inforce at
the end of each year, but that special amount no longer exists.

>  The second one is
> 
> interest_rates.cpp:719:23: error: parameter 'AnnualSepAcctIMFRate' set but 
> not used [-Werror,-Wunused-but-set-parameter]
> 
> and is even weirder, as we change a function parameter, which is curious on
> its own, and not only we don't use it later, but we don't use it at all,
> which is even curiosier. I have no idea about what should really happen
> here, but the simplest change avoiding the warning would be to simply
> remove AnnualSepAcctIMFRate parameter from DynamicMlySepAcctRate. I know
> that this function is supposed to be redesigned entirely (there is a
> comment about it), but it's probably not going to happen in the immediate
> future,

I'm not sure what this function ought to do. However, it plainly
does nothing with this argument, so it would seem safe to comment
out the name of the argument and the statement that modifies it.
As above, of course, I'll test that change before pushing it.

> while I'd like to make clang CI build pass again reasonably soon
> and I think it would be better to avoid disabling this warning, as it seems
> quite useful -- at least judging from two true positives above.

Agreed.