Hey Stas, a bunch of this has already been covered but I will attempt to answer
each of these as is my current understanding of "the hives" decision... :P
> 1. Accessors IMO should be regular PHP methods that PHP generates with two
> additional things:
> a. Their name is generated by PHP
> b. Their argument set is defined by the accessor pattern (i.e. same thing as
> __get/__set).
> We should keep the amount of magic and special cases to the minimum, the
> engine is complex enough as it is.
> This of course includes the full range of options available for the methods -
> final, reflection, call scenarios, etc.
This is the way it is, though Nikita strongly disagrees that they should be
"callable, visible" methods on the object and I agree with Nikita on this
issue, I never did like the idea that __getHours() and __setHours() were
methods of a class just because of an accessor definition, which is why I
worked to have Reflection hide that fact. If I were to go about this again,
they probably will not be methods of the class. Internally there is little
requiring an op_array to be attached to a class in order to be executed.
> 2. isset is defined as:
> isset() { return $this->Hours != NULL; } This does not seem to be
> correct - != NULL does not work like isset now.
> Try this:
> class A { public $x = 0; }
> $a = new A;
> var_dump(isset($a->x));
> var_dump($a->x != NULL);
> This needs to be fixed - generated isset() should be defined to work exactly
> like regular isset and actually use the same code path.
> Argument to isset() call can be retrieved using accessor but it should not
> differ from isset() result in any possible way or situation
> (including error situations - e.g. notices, etc.)
The reason that = NULL and != NULL was chosen is because isset() and unset()
are special states that are available only to a variable or property, since a
get/set do not return a real property they cannot be used with isset/unset.
Now that there has been discussion of an accessor being a real property
shadowed by get/set, standard isset/unset could potentially be used against the
underlying property.
> 3. How references and complex cases are handled? Didn't find anything about
> it, e.g. how it handles $foo->bar++, $foo->bar[] = 1,
> $foo->bar[123] = 4, etc. ($foo->bar being property with get/set defined of
> course)? How $foobar =& $foo->bar is handled? The sort()
> case mentions by-ref return but does not explicitly mention all other cases.
> These need to be covered explicitly in the RFC.
This is covered in the RFC, perhaps not clearly enough (let me know how I could
expand on it further for clarity). To answer each of your questions, you can
think of what would happen as if it were a function call. For example, if you
did $foo->bar()[123] = 4 you would be modifying a return-by-val. If bar were
instead defined as returning a reference, then it would indeed work as you
expect above. $foobar = &$foo->bar would work as expected as long as the
getter is returning a reference as well.
I, perhaps mistakenly, assumed that if return-by-ref and sort() worked
properly, then all other "usages of references" should equally work the same,
is that not right? If it's not right, why not?
> 4. We have some LSP controls now in place to ensure non-LSP overrides
> generate E_STRICT. Will this be the case for properties too?
> Meaning, what happens if you add overriding protected getter to a property
> that was previously fully public - thus violating the LSP?
Certainly, this basic object oriented functionality, of course it is upheld.
This is one of the reasons I leveraged standard functions in the first place,
because these issues were automatically handled by the existing core.
>
> 5. What happens if you override accessor property with plain old variable
> property? I.e.:
>
> class A {
> protected $secret {
> get() { return "secret"; }
> }
> }
>
> class B extends A {
> public $secret = "not a secret anymore"; }
>
> Also, what happens here if A extends B in the same scenario (this refers to
> the #4 too).
This is currently up in the air on the RFC side and is being referred to as
which "shadows" which. The code currently has it that properties shadow
accessors, Nikita suggested that should be inverted and I agree, so unless
someone else thinks otherwise, accessors will probably shadow properties.
>
> 6. Thinking more about isset/unset - why not make isset/unset always be the
> default unless overridden? I can't really see the case
> where you want echo $foo->bar to work but if(isset($foo->bar)) echo $foo->bar
> to not work. I think isset() and unset() should always
> use automatic implementations by default (fixed in accord with #2 of course).
See response to #2 above, without a getter/setter returning a real property,
isset/unset accessors were necessary because isset()/unset() cannot perform on
a dynamic value. Consider this: unset($foo->bar()) or isset($foo->bar()).
Since we are moving in the direction of accessors shadowing properties, I could
see the default implementation of isset/unset actually be more like unset() {
return unset($this->bar); } and isset() { return isset($this->bar); }
> 7. "Error messaging" section is not clear. Some examples would help - what is
> being translated to what?
I will expand on this section in the RFC, but this section is referring to the
fact that if bar were an accessor without a set() and this were attempted:
$foo->bar = 4;
It would have errored with:
Fatal error: no function __setbar() exists on object $foo <--
That is not exactly the message returned by core, but you get the point
So the section on error messaging is referring to the fact that all errors
involving accessors have been changed to make more sense, ie:
Fatal error: cannot set $foo->bar, no setter is defined.
> 8. "Static accessors" section is not clear, namely this one:
> This yielded the possibility that a getter call was being made while it
> should not be allowed (if there was no getter defined) and so
> pass_two() was changed to look for these non-backpatched illegal static
> getter calls and a compile time error is produced.
>
> When the code is compiled, the class definition (and, in fact, the name of
> the class we're talking about) may not be available, so how
> can you known if certain property of this class has getters defined?
Oh crap, you're referring to the idea that during compilation a class
definition may exist below the point of "current compilation," right? I had
not considered this case and you're right, this would break badly.
> Also, I'm not sure what is the thing about backpatching and converting to
> function calls - I think it should work via engine handlers
> just as the rest of the things work in the engine, is it not the case? If
> not, it should be made the case.
The static accessors was above my pay grade when I first tackled them, in
comparison the object get/set was a piece of cake because it was already being
handled by __get() and __set() but static properties are handled in an entirely
different way. Because of the nearly completely undocumented code, I could not
make heads nor tails of how it worked. I still do not know if I could make
this work at the vm/opcode level during runtime but I'd be much more willing to
give it a go now.
Most of my deep questions over the last year have gone unanswered and so I was
left to my own devices to find a way to make it work.
> 9. This:
> Eliminate the ability for an accessor to be called via $o→__getHours(), the
> accessor functions will be completely unavailable for use
> except as property references ($o→Hours)
>
> I think it a mistake. More magic and complication in the engine is not a good
> thing and would require tons of special case checks in
> all places where we deal with functions. I think it is wrong - if we create a
> callable entity, it should be callable. __ in the name is the
> indication enough that you're dealing with special method and you should
> tread carefully, we should not go out of our way to mess
> up the engine to prevent it.
You and Nikita get your gloves on, makes it easier on me to go your direction
but Nikita did not like this at all.
> 10. I'm not sure what the point about debug_backtrace() is but again, we
> should not create more complicated magic there, it just
> should show what really is happening.
This would refer to the fact that a debug backtrace would show that it's in a
function called __getHours() (for example) which would only serve to confuse a
php developer who doesn't know that a getter for $Hours is really a function
named __getHours(). Therefore I will be patching the error so that it
indicates something the php developer will understand, namely that the line is
referring to a getter.
> 11. About read-only/write-only thing - I like not having the keywords, but it
> is still not clear how final produces read-only property -
> do I say something like "final set();" or how can I say "there's no set and
> never will be"?
I could not get this point across to anyone but gave up, private final set();
!= read-only. With a non "read-only" keyword scenario, a "set" call would
still be allowed and would show up as an access violation to outside observers
and would be allowed from within the class. This is not what I wanted but I
had enough other points of contention and I yielded on this.
> I'm sorry if some points were already discussed - I scanned through the
> multiple threads but may very well missed something in
> 100+ messages that there are, in any case I feel most of the questions above
> must be reflected in RFC explicitly.
>
> Thanks,
> --
> Stanislav Malyshev, Software Architect
> SugarCRM: http://www.sugarcrm.com/
> (408)454-6900 ext. 227
