[m-dev.] For review: parallel conjunction

[Fergus Henderson]

Thomas Charles CONWAY, you wrote:
> 
> +		GoalExpr = par_conj(_GoalList, _SM),
> +		error("bytecode_gen of parallel conjunction not implemented")
> +	;

Please put "sorry, " at the start of all "not implemented" error messages,
unless you are sure that the problem should be caught and reported at some
earlier stage of compilation, in which case you should indicate this in a
comment.

> +detect_cse_in_goal_2(par_conj(Goals, SM), _, _, CseInfo, CseInfo, no,
> +		par_conj(Goals, SM)).

Shouldn't you recursively process the subgoals?

> +det_infer_goal_2(par_conj(Goals0, SM), GoalInfo, InstMap0, SolnContext,
> +		DetInfo, _, _, par_conj(Goals, SM), Detism, Msgs) :-
> +	det_infer_par_conj(Goals0, InstMap0, SolnContext, DetInfo,
> +		Goals, Detism, Msgs0),
> +	(
> +		Detism \= det
> +	->
> +		Msgs = [par_conj_not_det(GoalInfo)|Msgs0]
> +	;
> +		Msgs = Msgs0
> +	).

Hmm... aren't you really interested in model_det rather than det?
What about erroneous or cc_multi parallel conjunctions?
Any particular reason to disallow them?

> +det_diagnose_goal_2(par_conj(Goals, _SM), _GoalInfo, Desired, _Actual,
> +		Context, DetInfo, Diagnosed) -->
> +	det_diagnose_conj(Goals, Desired, Context, DetInfo, Diagnosed).
> +

Hmmm, if determinism analysis is different for par_conjs, then
determinism diagnosis should be different too, I think.

> +det_msg_get_type(par_conj_not_det(_), error).
>  
>  :- pred det_report_msg(det_msg, module_info, io__state, io__state).
>  :- mode det_report_msg(in, in, di, uo) is det.
> @@ -968,6 +975,10 @@
>  	{ det_info_init(ModuleInfo, PredId, ProcId, Globals, DetInfo) },
>  	det_diagnose_goal(Goal, DeclaredDetism, [], DetInfo, _),
>  	io__set_exit_status(1).
> +det_report_msg(par_conj_not_det(GoalInfo), _) -->
> +	{ goal_info_get_context(GoalInfo, Context) },
> +	prog_out__write_context(Context),
> +	io__write_string("Warning: parallel conjunction is not deterministic.\n").

It should be an error message, not a warning.
You should call io__set_exit_status(1).
The wording "is not deterministic" is a trifle vague; I'd prefer

	"determinism of parallel conjunction is not `det'.\n"


> +move_follow_code_in_goal_2(par_conj(Goals, SM), par_conj(Goals, SM),
> +		_Flags, R, R).
> +

You should processes nested goals recursively.

> +find_follow_vars_in_goal_2(par_conj(Goals0, SM), ArgsMethod, ModuleInfo,
> +		FollowVars0, par_conj(Goals, SM), FollowVars) :-
> +	find_follow_vars_in_disj(Goals0, ArgsMethod, ModuleInfo, FollowVars0,
> +		Goals, FollowVars).

This code is non-obivous; it needs to be documented.

> +traverse_goal(par_conj(Goals0, SM) - Info, par_conj(Goals, SM) - Info,
> +			PredProcId, Changed, GoalSize) -->
> +	traverse_disj(Goals0, Goals, PredProcId, Changed, GoalSize).
> +

Ditto.

... to be continued.

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