Everything

php

Work on reflection API

php.internals

Timm Friebe

23 years ago

Hello,
I went ahead and spent some time on the new reflection API. A patch (778
lines:)) and a demo script are attached.

What has changed:

---------------------------------------------------------------------

- Added documentation for each and every function (using the style
used in ext/*/*.c), e.g.:

/* {{{ proto Reflection_Class[] Reflection_Class::getInterfaces()
Returns an array of interfaces this class implements */

- Replaced duplicated code with macros or functions

- Added protection against static calls to the Reflection_* classes'
functions (resulting in fatal errors if you try to do so).

- Made Reflection_Class::getInterfaces return an empty array instead
of FALSE in case the class does not implement any interfaces

- Made Reflection_Function::getStaticVariables return an empty array
instead of FALSE in case the function does not have any static
variables.

- Made use of the ZEND_DO_THROW macro in the constructors (see
previous mail to engine2@ including a patch to zend_API.h)

- Added Reflection_Class::isFinal

- Added Reflection_Class::isAbstract

- Added Reflection_Class::newInstance. This method takes a variable
amount of arguments and passes them to the object's constructor
if there is one.

- Added Reflection_Class::isInstance

---------------------------------------------------------------------

I couldn't figure out how to remove the following memleaks which
occur when the reflection_class_factory is called:

/usr/home/thekid/devel/php/php/Zend/zend_reflection_api.c(296) :
Freeing 0x08371D24 (7 bytes), script=reflection.php
Last leak repeated 1 time
/usr/home/thekid/devel/php/php/Zend/zend_reflection_api.c(295) :
Freeing 0x08371CE4 (16 bytes), script=reflection.php
Last leak repeated 1 time

--
Timm

George Schlossnagle

23 years ago

On Tuesday, July 1, 2003, at 02:27  PM, Timm Friebe wrote:

> - Made use of the ZEND_DO_THROW macro in the constructors (see
>   previous mail to engine2@ including a patch to zend_API.h)

I privatized this for the moment until your other patch is accepted or
what have you.  Otherwise looks great.  Committing.

>
> - Added Reflection_Class::newInstance. This method takes a variable
>   amount of arguments and passes them to the object's constructor
>   if there is one.

I don't think this works correctly for builtin classes that need to do
their own allocation.  I'll poke some more, or you can convince me I'm
wrong.

Otherwise everything looks cool.

Timm Friebe

23 years ago

On Tue, 2003-07-01 at 20:27, Timm Friebe wrote:

> Hello,
> I went ahead and spent some time on the new reflection API. A patch (778
> lines:)) and a demo script are attached.

More to come:
---------------------------------------------------------------------
- Added some necessary efree()'s before an exception is thrown (as
ZEND_DO_THROW returns from the function immediately.

- Added Reflection_Method class
TBI: getDeclaringClass()

- Added Reflection_Class::getModifiers which returns a long
consisting of the bitmask of modifiers which are registered
as constants with the following names:

STATIC
ABSTRACT
FINAL
INTERFACE
ABSTRACT_CLASS
FINAL_CLASS
PUBLIC
PROTECTED
PRIVATE
PPP_MASK
CHANGED
IMPLICIT_PUBLIC

TBDiscussed: Would something like
$modifiers= array('public', 'static')
be cooler? Currently, one would be checking with
$modifiers & STATIC
etc.

- Added Reflection_Class::getConstructor

- Added Reflection_Class::getMethod(string name)

- Added Reflection_Class::getMethods()

---------------------------------------------------------------------

> I couldn't figure out how to remove the following memleaks which
> occur when the reflection_class_factory is called:

I found and eliminated them:)

Btw, the Reflection_Method->invoke() will not work without the patch to
zend_execute_API.c in my previous mail to engine2@.

- Timm

George Schlossnagle

23 years ago

Csn you make your diff of current cvs?  I committed a good bit of your
previous patch.

I don't know if i dig the way you implemented reflection_method.  I
have some partially complete work that handles this in a manner more
consistent with the rest of the code.

George

On Tuesday, July 1, 2003, at 05:27  PM, Timm Friebe wrote:

> On Tue, 2003-07-01 at 20:27, Timm Friebe wrote:
>> Hello,
>> I went ahead and spent some time on the new reflection API. A patch
>> (778
>> lines:)) and a demo script are attached.
>
> More to come:
> ---------------------------------------------------------------------
> - Added some necessary efree()'s before an exception is thrown (as
>   ZEND_DO_THROW returns from the function immediately.
>
> - Added Reflection_Method class
>   TBI: getDeclaringClass()

Cool.

>
> - Added Reflection_Class::getModifiers which returns a long
>   consisting of the bitmask of modifiers which are registered
>   as constants with the following names:
>
>   STATIC
>   ABSTRACT
>   FINAL
>   INTERFACE
>   ABSTRACT_CLASS
>   FINAL_CLASS
>   PUBLIC
>   PROTECTED
>   PRIVATE
>   PPP_MASK
>   CHANGED
>   IMPLICIT_PUBLIC
>
>   TBDiscussed: Would something like
>     $modifiers= array('public', 'static')
>   be cooler? Currently, one would be checking with
>     $modifiers & STATIC
>   etc.

or

array('public' => 1, 'static' => 1)

The bitmask is pretty cryptic.  Besides STATIC is a keyword.


-- George Schlossnagle
-- Principal Consultant
-- OmniTI Computer Consulting, Inc.
-- +1.410.872.4910 x202
-- 1024D/1100A5A0 1370 F70A 9365 96C9 2F5E  56C2 B2B9 262F 1100 A5A0

Timm Friebe

23 years ago

On Tue, 2003-07-01 at 23:36, George Schlossnagle wrote:

> Csn you make your diff of current cvs?  I committed a good bit of your
> previous patch.

Yup, you'll find it attached.

> I don't know if i dig the way you implemented reflection_method.  I
> have some partially complete work that handles this in a manner more
> consistent with the rest of the code.

? I simply copied your idea with the _factory method an intern->ptr
pointing to the class entry|the function pointer...

[...]

> >   TBDiscussed: Would something like
> >     $modifiers= array('public', 'static')
> >   be cooler? Currently, one would be checking with
> >     $modifiers & STATIC
> >   etc.
>
> or
>
> array('public' => 1, 'static' => 1)

Seems like a better solution (more user-friendly) - I've implemented
this.

> The bitmask is pretty cryptic.  Besides STATIC is a keyword.

You're right:)

- Timm

Timm Friebe

23 years ago

On Wed, 2003-07-02 at 00:23, Timm Friebe wrote:

> On Tue, 2003-07-01 at 23:36, George Schlossnagle wrote:
> > Csn you make your diff of current cvs?  I committed a good bit of your
> > previous patch.
> Yup, you'll find it attached.

Changelog:
---------------------------------------------------------------------------
- Added Reflection_Property class

- Added Reflection_Class::getProperty()

- Added Reflection_Class::getProperties()
---------------------------------------------------------------------------

This should be it for right now. The updated test script demonstrates
the new features.

Ti "speaks to himself" mm

Timm Friebe

23 years ago

On Wed, 2003-07-02 at 02:17, Timm Friebe wrote:

> On Wed, 2003-07-02 at 00:23, Timm Friebe wrote:
> > On Tue, 2003-07-01 at 23:36, George Schlossnagle wrote:
> > > Csn you make your diff of current cvs?  I committed a good bit of your
> > > previous patch.
> > Yup, you'll find it attached.

This should now be the final patch. It implements the following:
---------------------------------------------------------------------------
- Added Reflection_Property::getDeclaringClass()

- Added Reflection_Method::getDeclaringClass()
---------------------------------------------------------------------------

Ti "time for bed, it's 5:30 AM" mm

Timm Friebe

23 years ago

On Wed, 2003-07-02 at 05:32, Timm Friebe wrote:
[...]

> This should now be the final patch.

Then again, it would help to attach it:)

- Timm

George Schlossnagle

23 years ago

I'll apply this tomorrow if no one beats me to it.

On Tuesday, July 1, 2003, at 11:34  PM, Timm Friebe wrote:

> On Wed, 2003-07-02 at 05:32, Timm Friebe wrote:
> [...]
>> This should now be the final patch.
>
> Then again, it would help to attach it:)
>
> - Timm
> <zend_reflection_api.c.diff>

-- George Schlossnagle
-- Principal Consultant
-- OmniTI Computer Consulting, Inc.
-- +1.410.872.4910 x202
-- 1024D/1100A5A0 1370 F70A 9365 96C9 2F5E  56C2 B2B9 262F 1100 A5A0

Andrei Zmievski

23 years ago

On Wed, 02 Jul 2003, Timm Friebe wrote:

> On Wed, 2003-07-02 at 05:32, Timm Friebe wrote:
> [...]
> > This should now be the final patch.
>
> Then again, it would help to attach it:)

Why aren't you using the standard zend_parse_parameters() API function
here?

-Andrei
* It's an emergent property of human minds to create. *

George Schlossnagle

23 years ago

On Tuesday, July 1, 2003, at 06:23  PM, Timm Friebe wrote:

> On Tue, 2003-07-01 at 23:36, George Schlossnagle wrote:
>> Csn you make your diff of current cvs?  I committed a good bit of your
>> previous patch.
> Yup, you'll find it attached.

cool.

>
>> I don't know if i dig the way you implemented reflection_method.  I
>> have some partially complete work that handles this in a manner more
>> consistent with the rest of the code.
> ? I simply copied your idea with the _factory method an intern->ptr
> pointing to the class entry|the function pointer..

It needs the class entry too to really have context.  At least thats
how my implementation was working.

george

Timm Friebe

23 years ago

On Wed, 2003-07-02 at 02:44, George Schlossnagle wrote:
[...]

> It needs the class entry too to really have context.  At least thats
> how my implementation was working.

Hrm, AFAIK function_ptr->common.scope == <the zend_class_entry in which
a function resides in>.

- Ti "works for me" mm:)

George Schlossnagle

23 years ago

I don't see how that could work with inherited methods....

On Tuesday, July 1, 2003, at 08:53  PM, Timm Friebe wrote:

> On Wed, 2003-07-02 at 02:44, George Schlossnagle wrote:
> [...]
>> It needs the class entry too to really have context.  At least thats
>> how my implementation was working.
>
> Hrm, AFAIK function_ptr->common.scope == <the zend_class_entry in which
> a function resides in>.
>
> - Ti "works for me" mm:)
>
>
>

-- George Schlossnagle
-- Principal Consultant
-- OmniTI Computer Consulting, Inc.
-- +1.410.872.4910 x202
-- 1024D/1100A5A0 1370 F70A 9365 96C9 2F5E  56C2 B2B9 262F 1100 A5A0

Timm Friebe

23 years ago

On Wed, 2003-07-02 at 03:06, George Schlossnagle wrote:

> I don't see how that could work with inherited methods....

You mean a constellation like this:

class Base {
public function inbase() {
}
}

class AParent extends Base {
}

class Child extends AParent {
}

$class= new Reflection_Class('child');
var_dump($class->getMethods());
exit;

? - works wonderfully and outputs:

array(1) {
[0]=>
&object(reflection_method)#2 (2) {
["name"]=>
string(6) "inbase"
["class"]=>
string(5) "child"
}
}

- Timm :)

Andrei Zmievski

23 years ago

On Tue, 01 Jul 2003, Timm Friebe wrote:

> - Added Reflection_Class::getModifiers which returns a long
>   consisting of the bitmask of modifiers which are registered
>   as constants with the following names:
>
>   STATIC
>   ABSTRACT
>   FINAL
>   INTERFACE
>   ABSTRACT_CLASS
>   FINAL_CLASS
>   PUBLIC
>   PROTECTED
>   PRIVATE
>   PPP_MASK
>   CHANGED
>   IMPLICIT_PUBLIC

Should these be grouped somehow? PPP has a separate domain than
ABSTRACT_CLASS and FINAL_CLASS, for example.

>   TBDiscussed: Would something like
>     $modifiers= array('public', 'static')
>   be cooler? Currently, one would be checking with
>     $modifiers & STATIC

No, I'd prefer a bitmask rather than an array like that.

-Andrei
* There is no knowledge that is not power. -- Ralph Waldo Emerson *