> Somebody gimme a cookie. /me hands Steve a cookie.
> If the rx info object is going away, then obviously those parts of the > patch need not be applied. But in the meantime, it's nice to have a > Parrot that doesn't crash. I agree. My disclaimer about the regex code in my original email was to suggest that we didn't need to focus on the rx issues, but if you've already done it... :) > I'm not going to apply this patch yet because I'm sure someone will > disagree with how it fixes some or all of these bugs. So would that > someone please speak up? Thanks. I suppose that someone is me, although there might be other someones. > In summary, this patch > > - Adds an OUT parameter to new_hash() so the hash is anchored to the root set > while it is being constructed. > - Adds an OUT parameter to hash_clone() for early anchoring. > - Adds an OUT parameter to rx_allocate_info() for early anchoring. > - Briefly disables DOD while a stack is being created so allocating the contents > of the stack buffer doesn't destroy the unanchored buffer header. These are needed for now. However, when we get that buffer/pmc unification, we should be able to make mark() methods in the header pools. Then, with support for non-pmc-wrapped buffers, we can find references to them on the system stack, and call their mark() method directly, avoiding the above hoops. At least, that's my hope. Is it possible to mark the above code with some XXX tag so that we can re-address it when we get the unification in place? > - Makes a major change to the Pointer PMC: the previously unused ->cache area > is now used to hold a pointer to a custom mark routine that will get fired > during PMC traversal. Previously, Pointers had the PMC_private_GC_FLAG set, > but nothing ever looked at it. With this change, Pointers behave as they > always did unless something externally sets the ->cache.struct_val field > (in other words, there is no vtable entry for setting the mark routine, > and the PMC's custom mark routine does nothing if that field is NULL.) > > - Reorders the rx_allocinfo opcode to assign things in the correct order and > fill in the ->cache.struct_val field of the Pointer PMC it creates. These are a bit hackish, but I agree they are needed to solve our GC_DEBUG problems (and by extension, "real-world Parrot programs" ;). Both of these should also be able to "go away" with the unification, so see previous paragraph. :) I think I'm going to make GC_DEBUG a parameter of the interpreter, and allow it to be turned on/off via opcodes. Then we could force our test suite to use GC_DEBUG to root out GC problems a lot sooner than they otherwise would. Fixing all GC_DEBUG problems would help allow this kind of testing to be part of the standard test suite. > - In interpreter.c, asserts that a few of the early buffer creations do not > return the same buffer (provides early warning of GC mischief) Oooh, nice! :) The rest of the things you listed, which I didn't comment on are, imo, perfectly fine. In conclusion, I don't have any objections to this patch, although it would be nice if "XXX Unification" markers were included in places that needed to be addressed later. Mike Lambert
