[m-rev.] for review: fix predicate_table_insert bug

[Simon Taylor]

On 24-Oct-2003, Peter Ross <pro at missioncriticalit.com> wrote:
> On Fri, Oct 24, 2003 at 08:58:57AM +1000, Fergus Henderson wrote:
> > On 23-Oct-2003, Peter Ross <pro at missioncriticalit.com> wrote:
> > > 
> > > predicate_table_insert modifies the pred_info before inserting it into
> > > the pred_table, this "invalidates" the pred_info predicate_table_insert
> > > is called with however there are places in the compiler which use this
> > > invalid pred_info for further processing.
> > > 
> > > Fix this issue by no longer modifying the pred_info inside
> > > predicate_table_insert instead record the extra information in a new
> > > table.
> > 
> > That looks fine, thanks!
> > 
> > [It does lead to a slightly odd/inelegant data structure design,
> > where we have two different map(pred_id, ...) maps in the pred_table,
> > one mapping to pred_info and one mapping to the accessibility.
> > I probably would have been inclined to fix the bug differently,
> > by just finding all the places which call predicate_table_insert
> > and making sure that they do not rely on the pred_info remaining
> > unchanged, and adding some documentation to predicate_table_insert
> > to make it clear that it may modify the pred_info before inserting
> > it in the table, so the caller should not use the pred_info afterwards.
> > But your solution seems more robust.]
> > 
> I did that first, there is only one place as far as I can tell, however
> this is a more robust solution.
> 
> One improvement could be to modify the pred_table structure itself, so
> that it was a tuple (name_accessibility - pred_info).

Why not just return the updated pred_info?  That would be clearer.
I'm opposed to this change being committed.

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