On 09-Feb-2000, David Overton <dmo at ender.cs.mu.oz.au> wrote:
> Index: compiler/mercury_to_mercury.m
> @@ -750,6 +759,11 @@
> io__write_string(")")
> )
> ;
> + { GroundInstInfo = constrained_inst_var(Var) },
> + mercury_output_var(Var, VarSet, no)
> + % AAA
Hmm, another "AAA" comment... what is that for?
You should either add another comment explaining what
the AAA comment is there for, or delete it.
> +propagate_ctor_info(ground(Uniq, constrained_inst_var(Var)), _, _, _,
> + ground(Uniq, constrained_inst_var(Var))). % AAA
Likewise.
> @@ -730,6 +748,8 @@
> % be reported if anything tries to match with the inst.
> Modes = Modes0
> ).
> +propagate_ctor_info_lazily(ground(Uniq, constrained_inst_var(Var)), _, _, _,
> + ground(Uniq, constrained_inst_var(Var))). % AAA
Likewise.
> -:- pred inst_name_apply_substitution(inst_name, inst_subst, inst_name).
> -:- mode inst_name_apply_substitution(in, in, out) is det.
> +:- pred inst_name_apply_substitution(inst_name, inst_var_sub, inst_name).
> +:- mode inst_name_apply_substitution(in, in, out) is semidet.
It would be helpful to document when/why this predicate fails.
> inst_name_apply_substitution(user_inst(Name, Args0), Subst,
> user_inst(Name, Args)) :-
> inst_list_apply_substitution(Args0, Subst, Args).
> -inst_name_apply_substitution(unify_inst(Live, InstA0, InstB0, Real), Subst,
> - unify_inst(Live, InstA, InstB, Real)) :-
> - inst_apply_substitution(InstA0, Subst, InstA),
> - inst_apply_substitution(InstB0, Subst, InstB).
Hmm... why don't you apply the substitution to the insts in the inst_name?
I think it would be worth adding a comment explaining that.
> +ground_inst_info_apply_substitution(constrained_inst_var(Var), Subst, Uniq,
> + Result) :-
> + (
> + map__search(Subst, Var, Replacement)
> + ->
> + ( Replacement = ground(Uniq, GroundInstInfo) ->
> + Result = GroundInstInfo
> + ;
> + error("ground_inst_info_apply_substitution")
> + % AAA
> + )
> + ;
> + Result = constrained_inst_var(Var)
> + ).
Another "AAA" comment...
You should either add another comment explaining what
the AAA comment is there for, or delete it.
> -recompute_instmap_delta(RecomputeAtomic, Goal0 - GoalInfo0, Goal - GoalInfo,
> - VarTypes, InstMap0, InstMapDelta) -->
> +recompute_instmap_delta_1(RecomputeAtomic, Goal0 - GoalInfo0, Goal - GoalInfo,
> + VarTypes, InstMap0, InstMapDelta, RI0, RI) :-
> (
> - { RecomputeAtomic = no },
> - (
> - { goal_is_atomic(Goal0) }
> - ;
> + RecomputeAtomic = no,
> + goal_is_atomic(Goal0),
> + Goal0 \= unify(_,lambda_goal(_,_,_,_,_,_,_,_),_,_,_)
> % Lambda expressions always need to be processed.
> - { Goal0 = unify(_, Rhs, _, _, _) },
> - { Rhs \= lambda_goal(_, _, _, _, _, _, _, _) }
> - )
Hmm... you seem to have changed the semantics there:
you changed a disjunction into a conjunction.
I don't think that was mentioned in the log message.
What's the purpose of that change?
> +:- pred lift(pred(T, module_info, module_info), T, recompute_info,
> + recompute_info).
> +:- mode lift(pred(out, in, out) is det, out, in, out) is det.
> +
> +lift(P, R) -->
> + ModuleInfo0 =^ module_info,
> + { P(R, ModuleInfo0, ModuleInfo) },
> + ^module_info := ModuleInfo.
Hmm, "lift" is a rather generic name... a brief comment
explaining what this predicate does would be helpful, I think.
> +recompute_instmap_delta_call(PredId, ProcId, Args, VarTypes, InstMap,
> + InstMapDelta) -->
> + ModuleInfo =^ module_info,
> + { module_info_pred_proc_info(ModuleInfo, PredId, ProcId, _, ProcInfo) },
> + { proc_info_interface_determinism(ProcInfo, Detism) },
> + ( { determinism_components(Detism, _, at_most_zero) } ->
> + { instmap_delta_init_unreachable(InstMapDelta) }
> + ;
> + { proc_info_argmodes(ProcInfo, ArgModes0) },
> + { proc_info_inst_varset(ProcInfo, ProcInstVarSet) },
> + InstVarSet =^ inst_varset,
> + { rename_apart_inst_vars(InstVarSet, ProcInstVarSet,
> + ArgModes0, ArgModes1) },
> + { mode_list_get_initial_insts(ArgModes1, ModuleInfo,
> + InitialInsts) },
> + { map__init(InstVarSub0) },
> + lift(recompute_instmap_delta_call_1(Args, VarTypes, InstMap,
> + InitialInsts, InstVarSub0), InstVarSub),
> +
> + { mode_list_apply_substitution(ArgModes1, InstVarSub,
> + ArgModes2) },
> + lift(recompute_instmap_delta_call_2(Args, InstMap,
> + ArgModes2), ArgModes),
> + { instmap_delta_from_mode_list(Args, ArgModes,
> + ModuleInfo, InstMapDelta) }
> + ).
Hmm, that's a moderately complicated bit of code... some
comments here might help.
> +:- pred recompute_instmap_delta_call_1(list(prog_var), vartypes, instmap,
> + list(inst), inst_var_sub, inst_var_sub, module_info, module_info).
> +:- mode recompute_instmap_delta_call_1(in, in, in, in, in, out, in, out) is det.
This predicate seems poorly named to me...
I don't think it is actually recomputing any instmap deltas,
instead I think it is just computing a substitution?
> +recompute_instmap_delta_call_1([Arg | Args], VarTypes, InstMap, [Inst | Insts],
> + Sub0, Sub, ModuleInfo0, ModuleInfo) :-
> + % This is similar to modecheck_var_has_inst.
> + ( instmap__is_reachable(InstMap) ->
> + instmap__lookup_var(InstMap, Arg, ArgInst),
> + map__lookup(VarTypes, Arg, Type),
> + (
> + inst_matches_initial(ArgInst, Inst, Type, ModuleInfo0,
> + ModuleInfo1, Sub0, Sub1)
> + ->
> + ModuleInfo2 = ModuleInfo1,
> + Sub2 = Sub1
> + ;
> + % AAA error("recompute_instmap_delta_call_1: inst_matches_initial failed")
> + ModuleInfo2 = ModuleInfo0,
> + Sub2 = Sub0
I think you need some comments here.
Why do you ignore the failure if inst_matches_initial fails?
> -compare_inst_list_2([], [], _, same, _).
> +compare_inst_list_2([], [], _, [], same, _).
> compare_inst_list_2([InstA | InstsA], [InstB | InstsB],
> - no, Result, ModuleInfo) :-
> - compare_inst(InstA, InstB, no, Result0, ModuleInfo),
> - compare_inst_list_2(InstsA, InstsB, no, Result1, ModuleInfo),
> + std_util:no, [Type | Types], Result, ModuleInfo) :-
Please use `__' rather than `:' for the module qualifier.
But I would advise not using the module qualifier here,
just write `no' rather than `std_util__no'.
--
Fergus Henderson <fjh at cs.mu.oz.au> | "I have always known that the pursuit
WWW: <http://www.cs.mu.oz.au/~fjh> | of excellence is a lethal habit"
PGP: finger fjh at 128.250.37.3 | -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to: mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions: mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------