[m-rev.] for post-commit review: fix Mantis bug #512

[Peter Wang]

On Fri, 23 Oct 2020 01:37:48 +1100 "Zoltan Somogyi" <zoltan.somogyi at runbox.com> wrote:
> For review by Peter, since he filed the bug.
> 
> Zoltan.

> Rebuild nonlocals and instmaps when deleting code.
> 
> This fixes Mantis bug #512, whose symptom is a compiler abort in some
> very specific circumstances. The regression test for the bug below
> documents the long causal chain needed to tickle this abort.
> 
> compiler/simplify_goal_conj.m:
>     When deleting unreachable code, set the requanify flag, which also
>     causes instmap_deltas to be recomputed. The immediate cause of the bug
>     was an instmap_delta that should have been thus recomputed, but wasn't.
> 
>     Do this in both places where simplification can delete dead code,
>     though the Mantis 512 test case involves only one.
> 
> tests/valid/bug512.m:
>     A regression test for this bug.
> 
> tests/valid/Mercury.options:
> tests/valid/Mmakefile:
>     Enable the new test case, and run it with the flags that tickle
>     the bug it is guarding against.

> diff --git a/compiler/simplify_goal_conj.m b/compiler/simplify_goal_conj.m
> index 0f397625a..8867568e6 100644
> --- a/compiler/simplify_goal_conj.m
> +++ b/compiler/simplify_goal_conj.m
> @@ -102,7 +102,15 @@ simplify_goal_plain_conj(Goals0, GoalExpr, GoalInfo0, GoalInfo,
>              determinism_components(InnerDetism, CanFail, at_most_many),
>              goal_info_set_determinism(InnerDetism, GoalInfo0, InnerInfo),
>              InnerGoal = hlds_goal(conj(plain_conj, Goals), InnerInfo),
> -            GoalExpr = scope(commit(dont_force_pruning), InnerGoal)
> +            GoalExpr = scope(commit(dont_force_pruning), InnerGoal),
> +            % We have deleted goals that contain what could be
> +            % the last references to variables. This may require
> +            % adjustments to the nonlocals sets of not only this goal,
> +            % but of the other goals containing it. Likewise, it may
> +            % require adjustments of the instmap_deltas of such
> +            % containing goals, and we recompute instmap_deltas
> +            % only with requantification.
> +            simplify_info_set_should_requantify(!Info)

I didn't make the connection between wrapping a commit scope around a
conjunction that cannot produce solutions and "We have deleted goals".
Does the comment need to be changed to "We MAY have deleted goals"?

Nonetheless, it looks fine. Thanks for looking at the bug.

Peter