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

[Zoltan Somogyi]

On 12-Nov-2002, Fergus Henderson <fjh at cs.mu.OZ.AU> wrote:
> > +	;	table_trie_step_float
> > +	;	table_trie_step_enum(int)
> > +	;	table_trie_step_user(type)
> > +	;	table_trie_step_poly.
> 
> The meaning of the int in step_enum(int) should be documented.

Done. It is the number of alternatives in the enum type.

> What happens with arguments whose arg_mode is top_unused?

The compiler throws an exception. Tabled predicates are not allowed
to have unused arguments.

Making the compiler behave in a more civilized fashion when the program
contains such an error would be a separate change. It is low priority,
and I don't have time for it now.

> > +					% XXX For now, the compiler fully
> > +					% supports only procedures whose
> > +					% arguments are all builtin types,
> > +					% and the debugger further restricts
> > +					% this to procedures whose arguments
> > +					% are integers. However, this is still
> > +					% sufficient for debugging most
> > +					% problems in the tabling system.
> 
> Is the part "the debugger further restricts this to procedures whose
> arguments are integers" still correct?

No. I thought I caught and fixed all the references to this short-lived
further restriction, but it seems I didn't.

> > +type_save_category(polymorphic_type,  "any").
> > +type_save_category(user_type, 	 "any").
> 
> Some comments here explaining the meaning of these routines would be
> very helpful.

Done.

> > +	** The logic used here is duplicated in the function MR_trace_is_float
> > +	** trace/mercury_trace_util.c.
> 
> 	** in trace/mercury_trace_util.c.
> 	   ^^

Done.

> > Index: runtime/mercury_stack_layout.h
> > @@ -453,6 +449,64 @@
> >  } MR_Table_Io_Decl;
> >  
> >  /*
> > +** The MR_Table_Gen structure.
> > +** Each argument of a tabled predicate is an input or an output.
> 
> What about arguments with mode `unused'?

As explained above, there are no such arguments.

> Shouldn't that be spelt "lookup_only", rather than "insert_only"? 

Yes. Done.

> > +#define MR_HASH_CONTENTS_FUNC_BODY                                      \
> > +        MR_bool                                                         \
> > +        func_name(MR_TrieNode t, type_name **values_ptr,                \
> > +            int *value_next_ptr)                                        \
> > +        {                                                               \
> 
> A comment here explaining what this macro does (and/or what the
> functions that it generates do) would be helpful.

Done. The functions it implements are already documented in the header file.

> > +static void
> > +MR_print_answerblock(const MR_Proc_Layout *proc, MR_Word *answer_block)
> > +{
> ...
> > +#ifdef	MR_HIGHLEVEL_CODE
> > +				(double) MR_unbox_float(answer_block[i]));
> > +#else
> > +				(double) MR_word_to_float(answer_block[i]));
> > +#endif
> 
> The argument to MR_unbox_float() should have type `MR_Box', not
> `MR_Word', so you should use a cast to MR_Box there.

Done.

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