[m-dev.] For review: Make Mercury cope with impure code (part 2/2)

[Fergus Henderson]

Peter Schachte, you wrote:
> %			Check purity of a single predicate
> %
> %  Purity checking is quite simple.  Since impurity /must/ be declared, we can
> %  perform a single pass checking that the actual purity of each predicate
> %  matches the declared (or implied) purity.  A predicate is just as pure as
> %  its least pure goal.  While we're doing this, we attach a `feature' to each
> %  goal that is not pure, including non-atomic goals, indicating its purity.

Must subsequent passes over the HLDS maintain the correctness of these
annotations?  (Please document this.)
If so, do they?

> %  At the same time, we check that impure (semipure) atomic goals were marked
> %  in the source code as impure (semipure).  At this stage in the computation,
> %  this is indicated by already having the appropriate goal feature.  (During
> %  the translation from term to goal, the goal is wrapped in a purity/2 term
> %  indicating its marked purity, and in the translation from goal to
> %  hlds_goal, the purity/2 wrapper is turned into the appropriate feature on
> %  the hlds_goal_info.

Missing ')'.

> compute_expr_purity(call(PredId0,ProcId,Vars,BIState,UContext,Name0),
> 		call(PredId,ProcId,Vars,BIState,UContext,Name), GoalInfo,
> 		PredInfo, ModuleInfo, ActualPurity, NumErrors0, NumErrors) -->
> 	{ resolve_pred_overloading(PredId0, Vars, PredInfo, ModuleInfo,
> 				   Name0, Name, PredId) },
> 	{ module_info_preds(ModuleInfo, Preds) },
> 	{ map__lookup(Preds, PredId, CalleePredInfo) },
> 	{ pred_info_get_purity(CalleePredInfo, ActualPurity) },
> 	{ infer_goal_info_purity(GoalInfo, DeclaredPurity) },
> 	{ goal_info_get_context(GoalInfo, CallContext) },
> 	(   { code_util__compiler_generated(PredInfo) } ->
> 		% Don't report purity errors in compiler-generated code

The comment here is misleading.  It is only certain kinds of purity
errors that you don't report for compiler-generated code.
I suggest you replace that comment with "Don't require purity annotations
on calls in compiler-generated code".

> error_inconsistent_promise(ModuleInfo, PredInfo, PredId, Purity) -->
> 	{ pred_info_context(PredInfo, Context) },
> 	write_context_and_pred_id(ModuleInfo, PredInfo, PredId),
> 	prog_out__write_context(Context),
> 	report_warning("  warning: declared "),
> 	write_purity(Purity),
> 	io__write_string(" but promised pure.\n"),
> 	globals__io_lookup_bool_option(verbose_errors, VerboseErrors),
> 	( { VerboseErrors = yes } ->
> 		prog_out__write_context(Context),
> 		io__write_string(
> 		    "  pure predicates that call impure predicates should be\n"
> 		),
> 		prog_out__write_context(Context),
> 		io__write_string(
> 		    "  promised pure and have no impurity declaration.\n"

I suggest s/and have/and should have/
 
> resolve_pred_overloading(PredId0, Args0, CallerPredInfo, ModuleInfo,
> 		PredName0, PredName, PredId) :-
>         ( invalid_pred_id(PredId0) ->
>                 %
>                 % Find the set of candidate pred_ids for predicates which
>                 % have the specified name and arity
>                 %
>                 pred_info_typevarset(CallerPredInfo, TVarSet),
> 		pred_info_clauses_info(CallerPredInfo, ClausesInfo),
> 		ClausesInfo = clauses_info(_, _, VarTypes, _, _),
>                 typecheck__resolve_pred_overloading(ModuleInfo, Args0,
>                         VarTypes, TVarSet, PredName0, PredName, PredId)
>         ;
>                 PredId = PredId0,
>                 PredName = PredName0
>         ).

Inconsistent use of spaces instead of tabs.

> New File: tests/invalid/purity_nonsense.err_exp
> ===================================================================
> purity_nonsense.m:003: Error: Invalid impurity declaration: func undefined_func = foo.

s/Invalid/invalid/


OK, that concludes my review.  Basically the code looks very good,
modulo the few small things that I have pointed out.  The documentation
needs significant work, though; it's good as far as it goes, but it
doesn't go nearly far enough.

Cheers,
	Fergus.

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