[m-dev.] for review: cleanup of tabling

[Zoltan Somogyi]

> > +	is_new_answer = (table->MR_integer == 0);
> > +	table->MR_integer = 1;	/* any nonzero value will do */
> 
> Hmm, why use 0 and 1 rather than MR_ANS_NOT_GENERATED
> and MR_ANS_GENERATED? 
> 
> The comment here perhaps hints at the answer, but it's a bit cryptic;
> what is it that any nonzero value will do?
> 
> I think some more documentation here would help.

The documentation is located at the declaration of MR_TableNode.

> > -	answer_node = table_allocate_bytes(sizeof(MR_AnswerListNode));
> > -	answer_node->answer_num = table->num_ans;
> > -	answer_node->answer_data = 0;
> > +	answer_node = MR_table_allocate(MR_AnswerListNode);
> > +	answer_node->answer_num = subgoal->num_ans;
> > +	answer_node->answer_data.MR_integer = 0;
> >  	answer_node->next_answer = NULL;
> 
> The setting of the MR_Integer field to zero is a little
> mysterious; a comment would probably help.

The documentation is located at the declaration of MR_TableNode, and there
is a comment about two lines before your quote starts.

> > +	table = (MR_TrieNode) T;
> > +	/* XXX F vs float_to_word(F) */
> > +	MR_TABLE_SAVE_ANSWER(table, Offset, F,
> >  		mercury_data___type_ctor_info_float_0);
> >  ").
> 
> The XXX here is right -- this code is buggy.
> It should be `float_to_word(F)' rather than `F'.

Yes, you are right. The new code would have been correct if the pragma C code
had the new attribute, but of course it didn't.

Zoltan.
--------------------------------------------------------------------------
mercury-developers mailing list
Post messages to:       mercury-developers at cs.mu.oz.au
Administrative Queries: owner-mercury-developers at cs.mu.oz.au
Subscriptions:          mercury-developers-request at cs.mu.oz.au
--------------------------------------------------------------------------