[m-rev.] for review: debugging tabling procedures

[Fergus Henderson]

On 08-Nov-2002, Zoltan Somogyi <zs at cs.mu.OZ.AU> wrote:
> Index: runtime/mercury_tabling.h
...
> +/*
> +** These functions return a dynamically resizable array (using the primitives
> +** in mercury_array_macros.h) containing all the elements in the given
> +** dynamically resizable hash table.
> +*/
> +
> +extern	MR_bool		MR_int_hash_contents(MR_TrieNode t,
> +				MR_Integer **values_ptr, int *value_next_ptr);
> +extern	MR_bool		MR_float_hash_contents(MR_TrieNode t,
> +				MR_Float **values_ptr, int *value_next_ptr);
> +extern	MR_bool		MR_string_hash_contents(MR_TrieNode t,
> +				MR_ConstString **values_ptr,
> +				int *value_next_ptr);

The names MR_<type>_hash_contents() make it sound like these routines
hash the contents of something.  I think it would be clearer to name these
MR_<type>_hash_get_contents(), MR_<type>_get_hash_table_contents(),
or MR_<type>_hash_table_get_contents().

> Index: runtime/mercury_trace_internal.c
...
> +static MR_bool
> +MR_trace_fill_in_int_table_arg_slot(MR_TrieNode *table_cur_ptr,
> +	int arg_num, MR_ConstString given_arg,
> +	MR_Call_Table_Arg *call_table_arg_ptr)

Some comments explaining what this function does (and the meaning of
the return value) would be helpful.

> +static MR_bool
> +MR_update_int_table_arg_slot(MR_TrieNode *table_cur_ptr,
> +	MR_Call_Table_Arg *call_table_arg_ptr)

Likewise here.

One more comment: even though this only affects debug grades, I think
it would probably be a good idea to increment the binary compatibility
version number in runtime/mercury_grade.h.  It's often a good idea to
bump that up when doing a new major release, just in case there have
been any other changes that broke backwards binary compatibility for
which we failed to increment the binary compatibility version number.

That completes my review.

Once these comments (and the other ones I've raised in this thread)
have been addressed, I'll be happy for this to be committed on the
main branch.  If/when the automatic testing on the main branch passes
(or at least doesn't introduce any new failures) for all the systems that
we run tests on, then I'll be happy for it to also be committed on the
release branch (i.e. the "version-0_11-branch" branch).

-- 
Fergus Henderson <fjh at cs.mu.oz.au>  |  "I have always known that the pursuit
The University of Melbourne         |  of excellence is a lethal habit"
WWW: <http://www.cs.mu.oz.au/~fjh>  |     -- the last words of T. S. Garp.
--------------------------------------------------------------------------
mercury-reviews mailing list
post:  mercury-reviews at cs.mu.oz.au
administrative address: owner-mercury-reviews at cs.mu.oz.au
unsubscribe: Address: mercury-reviews-request at cs.mu.oz.au Message: unsubscribe
subscribe:   Address: mercury-reviews-request at cs.mu.oz.au Message: subscribe
--------------------------------------------------------------------------