Another object bug
Simon Glover
If I'm understanding the docs correctly, this should print '0'.
Instead, it prints 'Array index out of bounds!'
newclass P1, "Foo"
addattribute P1, "i"
find_type I0, "Foo"
new P2, I0
classoffset I1, P2, "Foo"
print I1
print "\n"
end
Simon
Dan Sugalski
> If I'm understanding the docs correctly, this should print '0'.
> Instead, it prints 'Array index out of bounds!'
Another bug, though the offset ought to be 2 right now. (Attributes 0
and 1 are taken by other things so they're valid)
--
Dan
--------------------------------------"it's like this"-------------------
Dan Sugalski even samurai
d...@sidhe.org have teddy bears and even
teddy bears get drunk
Simon Glover
On Wed, 25 Feb 2004, Dan Sugalski wrote:
> At 4:54 PM -0500 2/25/04, Simon Glover wrote:
> > If I'm understanding the docs correctly, this should print '0'.
> > Instead, it prints 'Array index out of bounds!'
>
> Another bug, though the offset ought to be 2 right now. (Attributes 0
> and 1 are taken by other things so they're valid)
OK, I think I see what's going on now: attribute 0 is the class PMC,
attribute 1 is the classname PMC, and then any other attributes go
into slots 2+
However, there's currently nothing to stop you stomping on the PMCs
in slots 0 and 1 by setting attributes with those offsets: in other
words, doing:
newclass P1, "Foo"
addattribute P1, "i"
find_type I0, "Foo"
new P2, I0
new P3, .PerlInt
set P3, 1024
setattribute P2, 1, P3
works, but changes the classname PMC to be a PerlInt with a value of
1024. Are we supposed to be able to do this?
A second issue (and what confused me in the first place) is that in your
attribute example in PDD 15 you have:
Adding the attributes C<a> and C<b> to the new class C<Foo>:
newclass $P0, "Foo"
addattribute $P0, "a", "Foo::a" # This is offset 0
addattribute $P0, "b", "Foo::b" # This is offset 1
I assume that this should actually refer to offsets 2 and 3?
Finally, in looking at the ops code, I noticed an unrelated bug:
in the classname op, you do:
classname_pmc = VTABLE_get_pmc_keyed_int(interpreter,
(PMC *)PMC_data($2), POD_CLASS_NAME);
To get the classname for the PMC. The problem is that the op is
documented as working with a PMC respresenting a class, but you're
using the symbolic constant from the object enum. This currently works,
since the classname PMC is stored in slot 1 in both a ParrotObject and
a ParrotClass PMC, but it still looks like a bug.
Simon
Dan Sugalski
>On Wed, 25 Feb 2004, Dan Sugalski wrote:
>
>> At 4:54 PM -0500 2/25/04, Simon Glover wrote:
>> > If I'm understanding the docs correctly, this should print '0'.
>> > Instead, it prints 'Array index out of bounds!'
>>
>> Another bug, though the offset ought to be 2 right now. (Attributes 0
>> and 1 are taken by other things so they're valid)
>
> OK, I think I see what's going on now: attribute 0 is the class PMC,
> attribute 1 is the classname PMC, and then any other attributes go
> into slots 2+
>
> However, there's currently nothing to stop you stomping on the PMCs
> in slots 0 and 1 by setting attributes with those offsets: in other
> words, doing:
>
> newclass P1, "Foo"
> addattribute P1, "i"
> find_type I0, "Foo"
> new P2, I0
>
> new P3, .PerlInt
> set P3, 1024
> setattribute P2, 1, P3
>
> works, but changes the classname PMC to be a PerlInt with a value of
> 1024. Are we supposed to be able to do this?
Yes. At least... sorta kinda yes. Everything inside an object's
attributes that's outside the range (class_offset ..
class_offset+class_attrib_count] is supposed to not be touched.
This all assumes that compilers will generate correct code, and at
the moment there's no reason, other than for testing, to use absolute
offsets--the ops to do it all relatively are there and work. We may
hide or otherwise protect these slots in the future--right now I'm
happy with it all working as long as you don't poke it too hard.
(Many of the functions don't properly check for class or objectness,
for example)
> A second issue (and what confused me in the first place) is that in your
> attribute example in PDD 15 you have:
>
> Adding the attributes C<a> and C<b> to the new class C<Foo>:
>
> newclass $P0, "Foo"
> addattribute $P0, "a", "Foo::a" # This is offset 0
> addattribute $P0, "b", "Foo::b" # This is offset 1
>
> I assume that this should actually refer to offsets 2 and 3?
Yes. That example's wrong and I'll go fix it.
> Finally, in looking at the ops code, I noticed an unrelated bug:
> in the classname op, you do:
>
> classname_pmc = VTABLE_get_pmc_keyed_int(interpreter,
> (PMC *)PMC_data($2), POD_CLASS_NAME);
>
> To get the classname for the PMC. The problem is that the op is
> documented as working with a PMC respresenting a class, but you're
> using the symbolic constant from the object enum. This currently works,
> since the classname PMC is stored in slot 1 in both a ParrotObject and
> a ParrotClass PMC, but it still looks like a bug.
D'oh! I'll go fix. Though since classes are supposed to be objects
you could argue that it's actually correct, other than the fact that
it's wrong. :)
Leopold Toetsch
> At 4:54 PM -0500 2/25/04, Simon Glover wrote:
>> If I'm understanding the docs correctly, this should print '0'.
>> Instead, it prints 'Array index out of bounds!'
> Another bug, though the offset ought to be 2 right now. (Attributes 0
> and 1 are taken by other things so they're valid)
*Please* don't. C<classoffset> (and attribute access) should by all
means start with 0.
- Let's hide the internals
- If that offset is ever changed, existing code will break
- Its ugly
We can afford one more instruction ( offset += POD_FIRST_ATTRIB; )
that's just one CPU cycle.
leo
Luke Palmer
Agreed. Though I do like the ability to replace the class name PMC
currently in slot 0. But then there are a lot of PMCs in a lot of
places that I'd like to replace, and the eventual Parrot::B should do
that for me. So I vote Yes on offset 0.
Luke
Dan Sugalski
>Dan Sugalski <d...@sidhe.org> wrote:
>> At 4:54 PM -0500 2/25/04, Simon Glover wrote:
>>> If I'm understanding the docs correctly, this should print '0'.
>>> Instead, it prints 'Array index out of bounds!'
>
>> Another bug, though the offset ought to be 2 right now. (Attributes 0
>> and 1 are taken by other things so they're valid)
>
>*Please* don't. C<classoffset> (and attribute access) should by all
>means start with 0.
Why? Heck, I could reasonbly argue that they pick a random offset
every time parrot's invoked. Which actually sounds like a good thing,
to keep people on their toes and stop everyone from assuming they
know the structure of the objects.
If you want, consider them the attributes of parrot's base object class.
>- If that offset is ever changed, existing code will break
No, it won't. No code should ever assume an absolute offset. That in
itself's broken.
Leopold Toetsch
> At 8:10 AM +0100 2/26/04, Leopold Toetsch wrote:
>>
>>*Please* don't. C<classoffset> (and attribute access) should by all
>>means start with 0.
> Why?
Simplifies compilers:
newclass P1, "Foo"
addattribute P1, "i"
findclass I1, "Foo"
new P2, I1
classoffset I2, P2
In static cases, where P2 is known to be a C<Foo>, attrib #0 ("i") would
be always 0. That is, the C<classoffset> opcode can be omitted in that
case.
> No, it won't. No code should ever assume an absolute offset. That in
> itself's broken.
like t/pmc/objects.t?
leo
Dan Sugalski
>Dan Sugalski <d...@sidhe.org> wrote:
>> At 8:10 AM +0100 2/26/04, Leopold Toetsch wrote:
>>>
>>>*Please* don't. C<classoffset> (and attribute access) should by all
>>>means start with 0.
>
>> Why?
>
>Simplifies compilers:
>
> newclass P1, "Foo"
> addattribute P1, "i"
> findclass I1, "Foo"
> new P2, I1
>
> classoffset I2, P2
>
>In static cases, where P2 is known to be a C<Foo>, attrib #0 ("i") would
>be always 0. That is, the C<classoffset> opcode can be omitted in that
>case.
That's a very rare case, honestly, and since arguably every object is
a subclass of the base object class and people will end up sticking
attributes and methods into the base object class it's going to
happen anyway.
I think I'd rather leave it as a non-zero, non-guaranteed base offset.
> > No, it won't. No code should ever assume an absolute offset. That in
>> itself's broken.
>
>like t/pmc/objects.t?
I was waiting for you to pull that out. :) Yes, objects.t assumes
some evil low-level knowledge of the internals.
Dan Sugalski
>On Thu, 26 Feb 2004, Dan Sugalski wrote:
>
>> At 2:38 PM +0100 2/26/04, Leopold Toetsch wrote:
>> >Dan Sugalski <d...@sidhe.org> wrote:
> > >> At 8:10 AM +0100 2/26/04, Leopold Toetsch wrote:
>> >> itself's broken.
>> >
>> >like t/pmc/objects.t?
>>
>> I was waiting for you to pull that out. :) Yes, objects.t assumes
>> some evil low-level knowledge of the internals.
>
> started writing the tests, so I had to use absolete offsets. Do you
> want me to rework it to be less evil?
Yeah, I think that'd be a good idea. I've this nasty feeling that
sometime between 0.1.0 and 0.1.2 someone's going to end up adding in
more attributes at runtime to the base object class...
Simon Glover
On Thu, 26 Feb 2004, Dan Sugalski wrote:
Well, in part that's because classoffset wasn't implemented when I
started writing the tests, so I had to use absolete offsets. Do you
want me to rework it to be less evil?
Simon
Simon Glover
On Thu, 26 Feb 2004, Dan Sugalski wrote:
> At 10:03 AM -0500 2/26/04, Simon Glover wrote:
> >On Thu, 26 Feb 2004, Dan Sugalski wrote:
> >
> >> >like t/pmc/objects.t?
> >>
> >> I was waiting for you to pull that out. :) Yes, objects.t assumes
> >> some evil low-level knowledge of the internals.
> >
> > Well, in part that's because classoffset wasn't implemented when I
> > started writing the tests, so I had to use absolete offsets. Do you
> > want me to rework it to be less evil?
>
> Yeah, I think that'd be a good idea. I've this nasty feeling that
> sometime between 0.1.0 and 0.1.2 someone's going to end up adding in
> more attributes at runtime to the base object class...
OK, the object tests now use relative offsets throughout.
Simon
Luke Palmer
> At 2:38 PM +0100 2/26/04, Leopold Toetsch wrote:
> >Simplifies compilers:
> >
> > newclass P1, "Foo"
> > addattribute P1, "i"
> > findclass I1, "Foo"
> > new P2, I1
> >
> > classoffset I2, P2
> >
> >In static cases, where P2 is known to be a C<Foo>, attrib #0 ("i") would
> >be always 0. That is, the C<classoffset> opcode can be omitted in that
> >case.
>
> That's a very rare case, honestly, and since arguably every object is
> a subclass of the base object class and people will end up sticking
> attributes and methods into the base object class it's going to
> happen anyway.
And how do we deal with an object already in existence when the base
object gets an attribute added?
Luke
Dan Sugalski
Right now? We corrupt all the objects and classes that descend from
the class that gets the attribute added.
In the nearish future, we throw an exception.
After that, we post a notification to all child classes and walk
through the PMC pools inserting the new attribute in the proper spot
for the classes.
Not rocket science. (Darned *annoying*, mind, but not rocket science :)
James Mastros
> At 5:16 PM -0700 2/26/04, Luke Palmer wrote:
>> And how do we deal with an object already in existence when the base
>> object gets an attribute added?
>
> the PMC pools inserting the new attribute in the proper spot for the
> classes.
>
> Not rocket science. (Darned *annoying*, mind, but not rocket science :)
Er, for the "after that" stage, wouldn't it be better to not have all
the attributes for a given class be contiguous? It makes the core more
complex, but it avoids having to implement the complexity in every class
ever written. I know, not rocket science. But darned annoying, and
it's better to force one person to write darned annoying code (even if
it is you), then to force everybody to implement it.
-=- James Mastros