studlyCaps patch

php.internals

(Marcus Börger)

22 years ago
Hello internals, after last discussion on this list regarding PHP's naming convention for OOCode i took a deeper look at fixing my main concerns. That is error messages, backtraces and reflection show lowercased method names. However this is easily changeable; with a minor performance decrease at compiletime when handling methods (not functions). The attached patch now changes two files for two different purposes: 1) Zend/zend_compile.c, These changes allow to show the original function/method names in errors, backtraces and reflection. This works for internal as well as for user functions/methods. 2) Zend/zend_default_classes.c, changes the method names of the internal exception class to studlyCaps naming convention. The patches were already tested by some other developers and we found out thefollowing things: 1) Some few error messsages show the new method name but should show the old method. For example when trying to override a final method the new name is shown instead of the old one which the message states. 2) Errors for non existing functions are still shown lowercased. This could be changed too. But the performance decrease is higher so i wouldn't recommend that. I think decision whether to incorporate this patch is up to Zeev and Andi.
-- Best regards, Marcus mailto:helly@php.net

Sebastian Bergmann

22 years ago
Marcus B?rger wrote:
> I think decision whether to incorporate this patch is up to Zeev and > Andi.
FWIW, +MAX_INT :)
-- Sebastian Bergmann http://sebastian-bergmann.de/ http://phpOpenTracker.de/ Das Buch zu PHP 5: http://professionelle-softwareentwicklung-mit-php5.de/

Lukas Smith

22 years ago
> From: Marcus Börger [mailto:marcus.boerger@t-online.de] > Sent: Friday, September 12, 2003 1:06 PM
> after last discussion on this list regarding PHP's naming convention
for
> OOCode i took a deeper look at fixing my main concerns. That is error > messages, backtraces and reflection show lowercased method names.
However
> this is easily changeable; with a minor performance decrease at > compiletime > when handling methods (not functions). > > The attached patch now changes two files for two different purposes: > 1) Zend/zend_compile.c, These changes allow to show the original > function/method names in errors, backtraces and reflection. This works
for
> internal as well as for user functions/methods. > > 2) Zend/zend_default_classes.c, changes the method names of the
internal
> exception class to studlyCaps naming convention.
> I think decision whether to incorporate this patch is up to Zeev and
Andi. I am +1 on this obviously. However it is important to note that this is also useful for userland code, so I am +1 on this regardless of the php OO CS. Regards, Lukas

Andi Gutmans

22 years ago
I think this patch is fine. I think (1) can be fixed quite easily. What do you mean when you say that (2) would result in a performance decrease? Would this only be in an error situation or also during regular runtime? Andi At 01:05 PM 9/12/2003 +0200, Marcus Börger wrote:

(Marcus Börger)

22 years ago
Hello Andi, Tuesday, September 16, 2003, 9:37:40 PM, you wrote:
> I think this patch is fine. > I think (1) can be fixed quite easily. What do you mean when you say that > (2) would result in a performance decrease? Would this only be in an error > situation or also during regular runtime?
>>2) Errors for non existing functions are still shown lowercased. This could >>be changed too. But the performance decrease is higher so i wouldn't >>recommend that. >>
My soulution to (2) required an additional estrndup() for all functions which are called by name. Maybe i should apply the parts done so far and send you the patch for (2), how about doing so?
-- Best regards, Marcus mailto:helly@php.net

Andi Gutmans

22 years ago
At 10:10 PM 9/16/2003 +0200, Marcus Börger wrote:
>Hello Andi, > >Tuesday, September 16, 2003, 9:37:40 PM, you wrote: > > > I think this patch is fine. > > I think (1) can be fixed quite easily. What do you mean when you say that > > (2) would result in a performance decrease? Would this only be in an error > > situation or also during regular runtime? > > >>2) Errors for non existing functions are still shown lowercased. This could > >>be changed too. But the performance decrease is higher so i wouldn't > >>recommend that. > >> > >My soulution to (2) required an additional estrndup() for all functions which >are called by name. Maybe i should apply the parts done so far and send you >the patch for (2), how about doing so?
Good idea. We can then see if it's worth it or not. Andi

(Marcus Börger)

22 years ago
Hello Andi, Wednesday, September 17, 2003, 6:07:40 AM, you wrote:
> At 10:10 PM 9/16/2003 +0200, Marcus Börger wrote: >>Hello Andi, >> >>Tuesday, September 16, 2003, 9:37:40 PM, you wrote: >> >> > I think this patch is fine. >> > I think (1) can be fixed quite easily. What do you mean when you say that >> > (2) would result in a performance decrease? Would this only be in an error >> > situation or also during regular runtime? >> >> >>2) Errors for non existing functions are still shown lowercased. This could >> >>be changed too. But the performance decrease is higher so i wouldn't >> >>recommend that. >> >> >> >>My soulution to (2) required an additional estrndup() for all functions which >>are called by name. Maybe i should apply the parts done so far and send you >>the patch for (2), how about doing so?
> Good idea. We can then see if it's worth it or not.
> Andi
Here you go :-) As you can see i need one additional emalloc in compile which should be ok but also one additional emalloc/lowercase operation for dynamic function calls with static function names. That is you are going to use a function prior to declaring it. Best regards, Marcus mailto:helly@php.net

Zeev Suraski

22 years ago
At 13:54 17/09/2003, Marcus Börger wrote:
>Here you go :-) > >As you can see i need one additional emalloc in compile which should be ok >but also one additional emalloc/lowercase operation for dynamic function >calls with static function names. That is you are going to use a function >prior to declaring it.
Looks harmless. Go ahead and commit it. Zeev