Re: list strawman

[Stephen Leake <stephen.a.leake.1@gsfc.nasa.gov>]

Ted Dennison<dennison@telepath.com> writes:

> In article <u7kqu3yux.fsf@gsfc.nasa.gov>, Stephen Leake says...
> >One issue I did not point out is the changes I had to make to the
> >spec. In particular, I had to declare "List" as tagged, and use 'class
> >in a couple places. Do you have any objections to that?
> 
> If you could move the tagged-ness to the private section, I'd prefer
> that done. Since its done entirely for implmentation reasons
> (right?), that's where it goes. There's no need to dictate
> taggedness onto someone who might not need it.

I agree, but I needed the 'Class in functions returning Iterator to
avoid dispatching on two types. If I use 'Class in the public part,
the type has to be _publicly_ tagged.

An alternate solution is to wrap the Controlled iterator type in a
record, so that the public Iterator type is not itself tagged.

My current solution allows users to derive new types from List; I
figured that was a Good Thing.

> >> The List Finalize routine needs to invalidate any iterators on the
> >> list's iterator list. You don't do this. (The comment says you do,
> >> but you just seem to be calling Unchecked_Conversion on the list
> >> header, unless I am missing something)
> >
> >Actually Unchecked_Deallocation. That eventually calls Finalize on the
> >iterator list, which should invalidate all of them. A test case
> >proving this is needed.
> 
> I don't see how this makes the jump from your List object to the
> iterator object. In order for an iterator to know that its list is
> gone, you'd have to tell the iterator somehow. Something would have
> to set the Iterator's IA field to null, or its IA.List or IA.I
> fields to null or something. Just dynamicly deallocating the List's
> iterators from its iterator list isn't going to do that. Instead
> you'd leave the iterator's IA field pointing off into space
> somewhere. Are you setting that to null somewhere that I'm not
> seeing?

You're right; I'm not doing this right. I think it is easy to fix; in
Finalize (List), loop thru the Iterator_list, and call
Invalidate_Iterator. Apparently I meant to do that, because I declared
the subprogram Invalidate_Iterator, but I never call it :).

> >There is a tradeoff between putting stuff in Initialize, and doing
> >stuff with aggregates when an object is created. If you assign an
> >aggregate to a Controlled object, Initialize is _not_ called (LRM
> >7.6 (10)). So any place you assign an aggregate to a Controlled
> >object, you have to be sure you do everything Initialize would do.
> >I'm pretty sure I haven't got that right yet.
> 
> I wouldn't call it a trade-off exactly. I use initialize for
> allocating nessecary substructures (with default clean values), and
> nothing else. Finalize needs to undo that, along with undoing any
> bookeeping work that other routines might do. Adjust needs to take
> the copied fields and make newly-allocated unique copies of any
> pointed-to objects (or not in the case of iterators). Every
> Initialize will eventually get a finalize, as will every Adjust.

Yes, that is a good design policy. But currently, in Initialize
(Iterator), I add the iterator to the List's Iterator_List. So that
doesn't get done in some cases. I need to change this.

> I think it would have helped me to see a matrix something like this:
> 
> Operation
> Example           Controlled Primitives called
> ------------------------------------------------------------
> Declaration  
> A : Mytype;       Initialize (A);
> Initialization 
> A : Mytype := B;  Adjust (A);
> Assignment
> A := B;           Finalize (A); {fields copied} Adjust (A);
> 
> {Note: Often a temporary B will be created during assinment too,
> in which case we have:
>                  Adjust (Temp_B); Finalize (A); Adjust (A); Finalize (Temp_B);
> )

Yes, this is helpful. I suspect there is something like this in
Cohen's "Ada as a second language"; that's where it really belongs :).
We're supposed to assume readers "know the language"!

> >You might consider using my Test_Storage_Pools package in your test
> >harness; it makes it possible to find memory leaks as part of testing.
> 
> I actually have been checking for leaks too, with some code internal
> to the package. I essentially increment an alloction counter at
> every "new" call, decrement it at every Unchecked_Deallocation, and
> make sure its 0 when the Empty list (declared in the spec) gets
> finalized. Its not task-safe, but it works for a test.

Yes, but my Test_Storage_Pools does the same thing, without modifying
the source; it's a generic parameter (the storage_pool). That makes it
easier to take out in a release. And you don't have to worry about
forgetting a "new".

-- 
-- Stephe