[m-rev.] for review: convert mode analysis to use var_tables

Julien Fischer jfischer at opturion.com
Mon May 2 11:24:55 AEST 2022


On Mon, 2 May 2022, Zoltan Somogyi wrote:

> Convert mode analysis to use var_tables.
> 
> compiler/instmap.m:
> compiler/mode_comparison.m:
> compiler/mode_debug.m:
> compiler/mode_info.m:
> compiler/modecheck_call.m:
> compiler/modecheck_coerce.m:
> compiler/modecheck_goal.m:
> compiler/modecheck_unify.m:
> compiler/modecheck_util.m:
> compiler/modes.m:
> compiler/unique_modes.m:
>     Convert these modules to use var_tables.
> 
> compiler/mode_errors.m:
>     Convert this module to use var_tables.
>
>     Fix an ancient error that I think has escaped detection until now
>     because it arises only in the presence of a mode error in a procedure
>     whose mode is being inferred. The bug is that when we modecheck a call,
>     say from p to q, and find no matching modes in the callee because
>     its mode inference has generated errors, then we report those errors
>     in the callee as part of the explanation of the error in the caller.
>     That is fine. What was not fine is that we printed any variables
>     in the callee's mode_error using the *caller's* varset. We now
>     print them using the callee's var table.
> 
> compiler/type_util.m:
>     Add a var_table-using variant of an existing predicate,
>     for use in new code above.
> 
> compiler/pd_util.m:
>     Conform to the changes above.
> 
> tests/invalid/mode_inf.m:
>     Modify this test case to make the caller and callee use disjoint
>     sets of variable names, which is probably why the incorrect variables
>     in the error message about the callee has not been noticed.
> 
> tests/invalid/mode_inf.err_exp:
>     Expect the updated, and now correct, version of that error message.

You haven't included the diff for these last two files.

...

> diff --git a/compiler/modecheck_call.m b/compiler/modecheck_call.m
> index b021ccd75..0be1452d6 100644
> --- a/compiler/modecheck_call.m
> +++ b/compiler/modecheck_call.m
> @@ -74,7 +74,7 @@
>  :- import_module parse_tree.prog_type.
>  :- import_module parse_tree.prog_util.
>  :- import_module parse_tree.set_of_var.
> -:- import_module parse_tree.vartypes.
> +:- import_module parse_tree.var_table.
>
>  :- import_module bool.
>  :- import_module map.
> @@ -86,9 +86,9 @@
>  modecheck_call_pred(PredId, MaybeDetism, ProcId0, TheProcId,
>          ArgVars0, ArgVars, _GoalInfo, ExtraGoals, !ModeInfo) :-
>      mode_info_get_may_change_called_proc(!.ModeInfo, MayChangeCalledProc),
> -    mode_info_get_pred_id_table(!.ModeInfo, PredIdTable),
> +    mode_info_get_pred_id_table(!.ModeInfo, PredIdTable), % ZZZ
>      map.lookup(PredIdTable, PredId, PredInfo),
> -    pred_info_get_proc_table(PredInfo, Procs),
> +    pred_info_get_proc_table(PredInfo, Procs),  % ZZZ ProcTable
>      (
>          MayChangeCalledProc = may_not_change_called_proc,
>          ( if ProcId0 = invalid_proc_id then
> @@ -222,7 +222,7 @@ modecheck_find_matching_modes([ProcId | ProcIds], PredId, Procs, ArgVars0,
>      mode_info_get_instvarset(!.ModeInfo, InstVarSet0),
>      rename_apart_inst_vars(InstVarSet0, ProcInstVarSet, InstVarSet,
>          ProcArgModes0, ProcArgModes),
> -    mode_info_set_instvarset(InstVarSet, !ModeInfo),
> +    mode_info_set_instvarset(InstVarSet, !ModeInfo),    % ZZZ undone where?
>      mode_info_get_module_info(!.ModeInfo, ModuleInfo),
>      proc_info_arglives(ModuleInfo, ProcInfo, ProcArgLives0),

Are these ZZZ comments something you intend to commit?

> diff --git a/compiler/modecheck_goal.m b/compiler/modecheck_goal.m
> index 0675e1ab4..93fd7e900 100644
> --- a/compiler/modecheck_goal.m
> +++ b/compiler/modecheck_goal.m
> @@ -139,7 +139,7 @@
>  :- import_module parse_tree.prog_mode.
>  :- import_module parse_tree.prog_rename.
>  :- import_module parse_tree.set_of_var.
> -:- import_module parse_tree.vartypes.
> +:- import_module parse_tree.var_table.
>
>  :- import_module bag.
>  :- import_module list.
> @@ -151,7 +151,6 @@
>  :- import_module string.
>  :- import_module term.
>  :- import_module uint.
> -:- import_module varset.
>
>  %---------------------------------------------------------------------------%
>  %---------------------------------------------------------------------------%
> @@ -264,10 +263,10 @@ modecheck_goal_plain_call(GoalExpr0, GoalInfo0, GoalExpr, !ModeInfo) :-
>      mode_checkpoint(enter, CallString, !ModeInfo),
>
>      mode_info_set_call_context(call_context_call(mode_call_plain(PredId)),
> -        !ModeInfo),
> +        !ModeInfo), % ZZZ
>
>      mode_info_get_instmap(!.ModeInfo, InstMap0),
> -    DeterminismKnown = no,
> +    DeterminismKnown = no,  % ZZZ
>      modecheck_call_pred(PredId, DeterminismKnown, ProcId0, ProcId,
>          ArgVars0, ArgVars, GoalInfo0, ExtraGoals, !ModeInfo),

Ditto.

The rest looks fine.

Julien.


More information about the reviews mailing list