[m-dev.] Tabling round 2 [2/2]

[Fergus Henderson]

On 13-Mar-1998, Oliver Hutchison <ohutch at students.cs.mu.oz.au> wrote:
> +:- interface.
> +
> +:- type table.

What does the `table' type represent?
Please include a comment here.

Also a general "header" comment explaining what this group of stuff
is for would be a good idea.

> +	% This is a dummy predicate its pred_proc_id but not its code 
> +	% is used in compiler/table_gen.m see the comment there.
> +:- impure pred get_table(table).
> +:- mode get_table(out) is det.

Please punctuate the comment.

> +	% Report an error message about the currernt subgoal looping. 
> +:- pred table_loopcheck_error is erroneous.

s/currernt/current/

It might be good to include some additional arguments,
e.g. the module:name/arity of the predicate or function.

> +	% Lookup or insert an integer in the given tri.
> +:- impure pred table_lookup_insert_int(table, int, table).
> +:- mode table_lookup_insert_int(in, in, out) is det.
> +
> +	% Lookup or insert a character in the given tri
> +:- impure pred table_lookup_insert_char(table, character, table).
> +:- mode table_lookup_insert_char(in, in, out) is det.
> +
> +	% Lookup or insert a string in the given tri.
> +:- impure pred table_lookup_insert_string(table, string, table).
> +:- mode table_lookup_insert_string(in, in, out) is det.
> +
> +	% Lookup or insert a float in the current tri.
> +:- impure pred table_lookup_insert_float(table, float, table).
> +:- mode table_lookup_insert_float(in, in, out) is det.
> +
> +	% Lookup or inert an enumeration type in the given tri.
> +:- impure pred table_lookup_insert_enum(table, int, T, table).
> +:- mode table_lookup_insert_enum(in, in, in, out) is det.
> +
> +	% Lookup or insert a monomorphic user defined type in the given trie.
> +:- impure pred table_lookup_insert_user(table, T, table).
> +:- mode table_lookup_insert_user(in, in, out) is det.
> +
> +	% Lookup or insert a polymorphic user defined type in the given trie.
> +:- impure pred table_lookup_insert_poly(table, T, table).
> +:- mode table_lookup_insert_poly(in, in, out) is det.

What are the semantics of the arguments to the above predicates?

Also, for these and the ones preceding them, could you please
explain why they are impure/semipure?

> +	% Return true if the subgoal represented by the given table has an
> +	% answer. NOTE : this is only used for det and semidet procedures.
> +:- semipure pred table_have_ans(table).
> +:- mode table_have_ans(in) is semidet. 
> +
> +	% Return true if the subgoal represented by the given table has a
> +	% true answer. NOTE : this is only used for det and semidet 
> +	% procedures.
> +:- semipure pred table_has_succeeded(table).
> +:- mode table_has_succeeded(in) is semidet. 
> +
> +	% Save the fact the the subgoal has succeeded in the given table.
> +:- impure pred table_mark_as_succeeded(table).
> +:- mode table_mark_as_succeeded(in) is det.
> +
> +	% Save the fact the the subgoal has failed in the given table.
> +:- impure pred table_mark_as_failed(table).
> +:- mode table_mark_as_failed(in) is failure.
> +
> +	% Return true if the subgoal represented by the given table has
> +	% failed. NOTE : this is only used for semidet procedures.
> +:- semipure pred table_has_failed(table).
> +:- mode table_has_failed(in) is semidet.

The order of these predicate declarations is inconsistent.
I suggest you move the declaration of table_has_failed to just
after the declaration of table_has_succeeded.

> +	% Create an answer block with the given number of slots and add it
> +	% to the given table.
> +:- impure pred table_create_ans_block(table, int, table).
> +:- mode table_create_ans_block(in, in, out) is det.

What's an answer block?

> +:- pragma c_header_code("
> +	
> +	/* Used to mark the status of the table */
> +#define ML_UNINTIALISED 	0
> +#define ML_WORKING_ON_ANS       1
> +#define ML_FAILED               2
> +#define ML_SUCCEEDED		1024

Inconsistent use of tabs/spaces.

Mispelt "UNINITIALIZED".

Why the magic number 1024?

> +	% This is a dummy procedure that never actualy gets called.
> +	% See the comments in table_gen.m for its perpose.
> +:- pragma c_code(get_table(T::out), will_not_call_mercury, "
> +	/* Mention T so we avoid a warning */
> +").

s/actualy/actually/
s/perpose/purpose/

A simpler way of avoiding the warning is to use `_T' instead of `T'.

	:- pragma c_code(get_table(_T::out), will_not_call_mercury, "").

> +table_loopcheck_error :-
> +	error("infinite recusion in loop_check procedure").

s/recusion/recursion/

"in loop_check procedure" is misleading.
You could say "in procedure with `pragma loopcheck' or `pragma memo'
or `pragma minimal_model'", but that's getting a bit verbose
and it doesn't really add much information.

How about just "detected infinite recursion"?

> +:- pragma c_code(table_lookup_insert_int(T0::in, I::in, T::out), 
> +		will_not_call_mercury, "
> +	T = (Word)MR_TABLE_INT((Word**)T0, I);
> +").

Why is it that in previous predicates, the argument of type `table'
was cast to `Word *', but here you cast it to `Word **'?

> +:- pragma c_code(table_mark_as_failed(T::in), will_not_call_mercury, "
> +	*((Word*) T) = ML_FAILED;
> +	SUCCESS_INDICATOR = FALSE;
> +").

If this predicate always fails, it would probably be slightly more efficient
to make it `det' and always follow it with an explicit call to `fail'
(i.e. an empty disjunction).

> +	% Suspend the current subgoal call.
> +:- impure pred table_suspend(table, table).
> +:- mode table_suspend(in, out) is nondet.

I think you mean just "suspend the current subgoal.".

What happens to the execution, then? 
Does it backtrack to the previous choice point?

	% suspend the current subgoal,
	% and then backtrack to the previous choice point.

> +	% Resume all suspended subgoal calls.
> +:- impure pred table_resume(table).
> +:- mode table_resume(in) is failure.

I don't understand what it means to resume *all* suspended subgoals.

> +:- pragma c_header_code("
> +
> +typedef struct {
> +	Word ans_num;
> +	Word ans;
> +} AnswerListNode;
> +
> +typedef struct {
> +	Word *last_ret_ans;
> +	Code *succ_ip;
> +	Word *s_p;
> +	Word *cur_fr;
> +	Word *max_fr;
> +	Word non_stack_block_size;
> +	Word *non_stack_block;
> +	Word det_stack_block_size;
> +	Word *det_stack_block;
> +} SuspendListNode;
> +
> +typedef enum {
> +   	have_no_ans,
> +	have_some_ans,
> +	have_all_ans
> +} TableStatus;
> +
> +typedef struct {
> +	TableStatus status;
> +	Word answer_table;
> +	Word num_ans;
> +	Word answer_list;
> +	Word *ans_list_tail;
> +	Word suspend_list;
> +	Word *suspend_list_tail;
> +	Word *non_stack_bottom;
> +	Word *det_stack_bottom;
> +} NondetTable;

Comments needed here.

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