[m-dev.] tightening up require_complete_switch

[Peter Wang]

On Mon, 18 Apr 2016 22:26:33 +1000 (AEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> On Mon, 11 Apr 2016 12:29:27 +1000 (AEST), "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> > On Mon, 11 Apr 2016 11:31:06 +1000, Peter Wang <novalazy at gmail.com> wrote:
> > > Or simply remove the subtlety in require_complete_switch even if
> > > that means require_complete_switch could no longer be used in some
> > > multi-moded code.
> > 
> > I will do this in a test workspace and see what problems a bootcheck
> > throws up. I don't think this point can be usefully discussed without
> > knowing the result.
> 
> I have now done this. The bootcheck did not reveal any problems at all.
> The only two test cases that failed were (a) a test that checked the exact
> wording of the warning we used to generate in this case, and (b) a test
> of the fact that the warning did NOT prevent the compiler from generating
> target code.
> 
> Given this fact, I have no objection to making the compiler generate an error
> whenever a require_complete_switch [X] scope is not a switch on X.
> The attached diff does this. Given that this is a (slight) change to the language,
> I would appreciate everyone's input on it.
> 
> I plan to repeat the experiment with the require_switch_arms_detism scopes.
> I will report back, but I strongly suspect that the result will be the same,
> so I would like feedback on making the same change to that scope in that case.
> 
> As for making the [X] part optional, I think that benefit is minimal, while the
> cost, in terms of making any error messages about the scope more complex
> and therefore harder to understand, a bit (though not a lot) more substantial.

Assuming that require_switch_arms_* scopes could be changed as well,
removing the [X] part means that all require_* checks would have the
same form `require_something GOAL', a more consistent language design.

require_complete_switch is quite prevalent in code outside the compiler
so I don't want to advocate a breaking change.  If it were possible to
remove the [X] part with a significant transition period, then I would
like to see that happen.  Otherwise, any benefit may not be enough to
justify breaking people's code.

I do not think error messages would be any harder to understand.
I assumed reqscope_check_scope would look something like this:

        Reason = require_complete_switch(MaybeRequiredVar),
        SubGoal = hlds_goal(SubGoalExpr, _),
        ( if SubGoalExpr = switch(SwitchVar, CanFail, Cases) then
	    ... report if SwitchVar != RequiredVar (if supplied)
	    ... report if switch on SwitchVar is incomplete
        else
	    ... report that SubGoal is not a switch
        )

> diff --git a/doc/reference_manual.texi b/doc/reference_manual.texi
> index ef57532..611093e 100644
> --- a/doc/reference_manual.texi
> +++ b/doc/reference_manual.texi
> @@ -950,15 +950,13 @@ for all the function symbols in the type of @var{Var},
>  then @code{require_complete_switch [@var{Var}] @var{Goal}}
>  is equivalent to @var{Goal}.
>  If @var{Goal} is a switch on @var{Var} but it is not a complete switch,
> -i.e. it has no arm
> -for at least one of the function symbols in the type of @var{Var},
> -then the compiler is required to generate an error message.
> +i.e. if some of the function symbols in the type of @var{Var}
> +that @var{Var} could be bound to at this point in the code
> +are not covered by any arm of the switch,
> +then the compiler is required to generate an error message,
> +preferably one that lists the uncovered function symbols.

The "preferably one that lists the uncovered function symbols" part is
jarring when read from a user's point of view (the main audience), so I
suggest deleting it.

Thanks, Zoltan.

Peter