[m-dev.] diff: typeclasses

[Fergus Henderson]

det_analysis.m:
> +det_infer_goal_2(class_method_call(TCVar, Num, ArgVars, Types, Modes, Det0),
> +		GoalInfo, _InstMap0, SolnContext,
> +		_MiscInfo, _NonLocalVars, _DeltaInstMap,
> +		class_method_call(TCVar, Num, ArgVars, Types, Modes, Det0),
> +		Det, Msgs) :-
> +	determinism_components(Det0, CanFail, NumSolns),
> +	(
> +		NumSolns = at_most_many_cc,
> +		SolnContext \= first_soln
> +	->
> +			% XXX this will give a slightly misleading error
> +			% XXX message
> +		Msgs = [higher_order_cc_pred_in_wrong_context(GoalInfo, Det0)],

If that XXX will really occur, it should be fixed.
If (as I believe is the case) it won't occur, e.g. because calls aren't
converted into class_method_calls until _after_ determinism analysis,
then you should document that (and it warrants only a comment, not an XXX).

det_analysis.m:
>  get_all_pred_procs_2(_Preds, [], PredProcs, PredProcs).
>  get_all_pred_procs_2(Preds, [PredId|PredIds], PredProcs0, PredProcs) :-
>  	map__lookup(Preds, PredId, Pred),
> -	pred_info_non_imported_procids(Pred, ProcIds),
> -	fold_pred_modes(PredId, ProcIds, PredProcs0, PredProcs1),
> +	pred_info_get_marker_list(Pred, Markers),
> +	(
> +			% ignore class members
> +		list__member(request(class_method), Markers)
> +	->
> +		PredProcs1 = PredProcs0
> +	;
> +		pred_info_non_imported_procids(Pred, ProcIds),
> +		fold_pred_modes(PredId, ProcIds, PredProcs0, PredProcs1)
> +	),

Why ignore class members?

dnf.m:
>  	Goal0 = _GoalExpr - GoalInfo,
>  	goal_info_get_nonlocals(GoalInfo, NonLocals),
>  	set__to_sorted_list(NonLocals, ArgVars),
> +		% XXX
> +		% XXX Does this new pred necessarily have an empty context?
> +		% XXX I would think not. The pred context should probably be
> +		% XXX added to the dnf_info.
> +		% XXX

You should fix this XXX.

I think it would be clearer if you say "class context" rather than
just "context".  Also you should add a definition of class context
to notes/glossary.html.

> +++ higher_order.m	1997/10/13 03:47:44
> @@ -295,6 +295,10 @@
>  	{ Goal0 = higher_order_call(_,_,_,_,_,_) - _ }, 
>  	maybe_specialize_higher_order_call(Goal0, Goal, PredProcId, Changed).
>  
> +		% For now, we do not specialize class method calls
> +traverse_goal(Goal, Goal, _, unchanged, 1) -->
> +	{ Goal = class_method_call(_,_,_,_,_,_) - _ }.
> +

Now _that_ warrants an XXX ;-)

higher_order.m:
> +		% XXX
> +		% XXX This is not, in general, correct.
> +		% XXX
> +	TypeConstraints = [],

That shold be fixed.

hlds_out.m:
> +	io__write_string("\n% Class Table:\n"),
> +	{ module_info_classes(ModuleInfo, ClassTable) },
> +		% XXX fix this up.
> +	io__write(ClassTable).

FWIW, you should use `io__print' here, rather than `io__write'.

lambda.m:
> +	Info = lambda_info(VarSet, VarTypes, Constraints, TypeVarSet, 
> +		TVarMap, TCVarMap, _, _, ModuleInfo),
...
> +	pred_info_set_class_context(PredInfo1, Constraints, PredInfo).

I think you ought to restrict the constraints to just those
that apply to variables occurring inside the lambda goal.

There should at least be a comment somewhere in lambda.m
saying that the handling of type classes may be incomplete.

> +module_add_class_defn(Module0, Constraints, Name, Vars, Interface, VarSet,
> +		Context, Status, Module) -->
> +	{ module_info_classes(Module0, Classes0) },
> +	{ list__length(Vars, ClassArity) },
> +	{ Key = class_id(Name, ClassArity) },
> +	(
> +		{ map__search(Classes0, Key, _) }
> +	->
> +			% XXX format the output properly (?)
> +		prog_out__write_context(Context),
> +		io__write_string("Error: typeclass "),
> +		prog_out__write_sym_name(Name),
> +		io__write_char('/'),
> +		io__write_int(ClassArity),
> +		io__write_string(" multiply defined.\n"),

Regarding the XXX, you should insert ` and ' in the error
message.

Also, you should print out another error message line
"  Here is the previous definition.\n"
with the appropriate term__context (using the value
found by the call to map__search).

... to be continued.

(Sorry, I don't have the attention span to review an 8000-line diff
in one go. ;-)

-- 
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.