rfc:propertygetsetsyntax-as-implemented:change-requests

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revisionPrevious revision
Next revision		Previous revision
rfc:propertygetsetsyntax-as-implemented:change-requests [2012/10/21 17:52] – cpriestrfc:propertygetsetsyntax-as-implemented:change-requests [2017/09/22 13:28] (current) – external edit 127.0.0.1
Line 1: Line 1:
 ====== v1.1 -> v1.2 ====== ====== v1.1 -> v1.2 ======
 +    * [[rfc/propertygetsetsyntax-as-implemented|Versions 1.1 RFC]]
 +
  
 ===== Current Consensus Changes ===== ===== Current Consensus Changes =====
  
 ==== Syntax Change ==== ==== Syntax Change ====
-Change syntax to use public set($value) {}, public get(), etc. Along with that means no “magic” $value in the setter+<del>[[https://github.com/cpriest/php-src/issues/6|https://github.com/cpriest/php-src/issues/6]]</del> 
 + 
 +Allow syntax to use parenthesized syntax as well so that both a [TypeHint] and $value may be specified. 
 <code php> <code php>
 class TimePeriod { class TimePeriod {
Line 17: Line 22:
 } }
 </code> </code>
 +
 +The abbreviated syntax is also allowed as per 1.1 RFC.
 +
 +==== read-only / write-only keywords ====
 +<del>[[https://github.com/cpriest/php-src/issues/7|https://github.com/cpriest/php-src/issues/7]]</del>
 +
 +read-only / write-only keywords will be eliminated, the issue will be pushed to userland developers who can enforce the same by coding something like this:
 +
 +<code php>
 +class TimePeriod {
 +    private $Seconds;
 +
 +    public $Hours {
 +        get() { return $this->Hours; }
 +        private final set($value) { throw new Exception("Setting of TimePeriod::$Hours is not allowed."); }
 +    }
 +}
 +</code>
 +
 +This was previously being debated, the full details of this are in the last section under "Previously Debated"
  
 ==== Automatically Implemented Accessors ==== ==== Automatically Implemented Accessors ====
 +[[https://github.com/cpriest/php-src/issues/5|https://github.com/cpriest/php-src/issues/5]]
 +
 v1.1 properties shadow (or over-ride) accessors, so if an accessor exists and a property is defined (only possible from within the accessors setter), then the property would shadow the accessor.  This was done to keep it consistent with what %%__get()%% does and also to support lazy loading. v1.1 properties shadow (or over-ride) accessors, so if an accessor exists and a property is defined (only possible from within the accessors setter), then the property would shadow the accessor.  This was done to keep it consistent with what %%__get()%% does and also to support lazy loading.
  
Line 49: Line 76:
 </code> </code>
  
-=== Potential Issues === +==== Shadowing ==== 
-  - Decreased performance due to accessor check against any property access, could possibly be next to no impact depending on implementation (property structure with pointer to accessor structure). +[[https://github.com/cpriest/php-src/issues/11|https://github.com/cpriest/php-src/issues/11]]
- +
  
-===== Being Debated =====+Accessors will shadow properties such that if a property named $foo is declared and an accessor for $foo is declared, the accessor will be used instead, only the accessor will have direct access to the underlying property.
  
-==== Shadowing ==== +<code php> 
-v1.1 has properties shadow accessors, the suggestion is that this be inverted.+class TimePeriod { 
 +    private $Seconds;
  
-Specifically this means that in v1.1, if an accessor is defined and no property is defined, then the accessor will be used.  But when a property is assigned/declared then the property will "shadow" the accessor and the accessor will no longer be used until the property is unset().+    public $Seconds { 
 +        get() { return $this->Seconds; /* Accesses the private $Seconds directly */ } 
 +        set($x) { $this->Seconds = $x; /* Accesses the private $Seconds directly */ } 
 +    } 
 +}
  
-v1.2 Proposes that this be inverted such that if there is an accessor defined for a given property name, the accessor will always be used.  The accessor would be able to get/set/isset/unset the property with the same name as the accessor.  No direct access to the property would be allowed except from within the accessor.  +$o = new TimePeriod(); 
 +$o->Seconds = 45;  /* Calls set accessor */ 
 +echo $o->Seconds;  /* Calls get accessor */ 
 +</code>
  
-v1.2 proposal seems to make the most sense however it would incur a slight (possibly *very* slight) performance penalty.+==== isset / unset / attempted writes when no setter / attempted reads when no getter ==== 
 +[[https://github.com/cpriest/php-src/issues/12|https://github.com/cpriest/php-src/issues/12]]
  
 +Invalid calls to isset/unset will fail silently and return false in the case of isset(), no warning/notice should be thrown.
 +
 +<code php>
 +class TimePeriod {
 +    private $Seconds;
 +
 +    protected $Seconds {
 +        get() { return $this->Seconds; /* Accesses the private $Seconds directly */ }
 +        set($x) { $this->Seconds = $x; /* Accesses the private $Seconds directly */ }
 +    }
 +}
 +
 +$o = new TimePeriod();
 +isset($o->Seconds); /* Returns false, no error thrown */
 +unset($o->Seconds);  /* Does nothing, no error thrown */
 +</code>
 +
 +==== Extra shorthand declaration ====
 +
 +Not much further support and good arguments against it, dropped.
 +
 +==== Interfaces ====
 +[[https://github.com/cpriest/php-src/issues/13|https://github.com/cpriest/php-src/issues/13]]
 +
 +An implementing class may specify a property with an appropriate access level to satisfy an accessor declaration requirement of an interface.
 +<code php>
 +interface i {
 +    public $Seconds { get; }
 +}
 +
 +class TimePeriod implements i {
 +
 +    /* Satisfies interface implementation requirements of interface i */
 +    public $Seconds;
 +}
 +
 +</code>
 +
 +===== Being Debated =====
  
 ==== internal accessor method visibility / callability ==== ==== internal accessor method visibility / callability ====
Line 86: Line 160:
  
 Revealing these internal matters to them would only leave them confused, possibly frustrated //and likely asking about it to the internals mailing list to answer (repeatedly)//. Revealing these internal matters to them would only leave them confused, possibly frustrated //and likely asking about it to the internals mailing list to answer (repeatedly)//.
 +
 +===== Issues =====
 +
 +==== Static Accessors ====
 +Stas pointed out a number of critical mistakes with the current implementation of static accessors.  They would not currently work under these circumstances:
 +  - Class defined after a static accessor reference was made
 +  - Dynamic variable use of an accessor --> $class::$variable = 5;
 +
 +The current implementation "back-patches" the op_array at the time of a static variable declaration by patching the op array into a method call.  zend_do_assign() further detects these calls to %%ex: __getStaticHours()%% into %%__setStaticHours()%%, and a change to pass_two() detects invalid static accessor gets (those which were not properly backpatched with zend_do_assign())
 +
 +Stas suggested that he could either help me to implement this more appropriately (such as changing the engine to make calls into zend_object_handler.c) so that these calls can be resolved dynamically, or that static accessors be shelved for a future version of accessors.
 +
 +**Update 12/30/2012**: Too much engine work is necessary to implement static accessors, this is being pushed to a future RFC change.
 +===== TODO =====
 +  - <del>Exceptions thrown from an accessor reveal underlying implementation details</del> [[https://github.com/cpriest/php-src/issues/14|github#14]]
 +  - <del>get_class_methods() - hide accessors or re-write to call into Reflection* code?</del> [[https://github.com/cpriest/php-src/issues/15|github#15]]
 +  - <del>Check debug_backtrace() output as well</del> [[https://github.com/cpriest/php-src/issues/16|github#16]]
 +  - <del>Add notes to RFC on shadowing behavior of properties and acessors [[https://github.com/cpriest/php-src/issues/17|github#17]]</del>
 +  - <del>Fix automatically implemented isset to use !== rather than != (Thanks Stas!)</del>
 +  - <del>Check that the following should work and write tests for them:</del> [[https://github.com/cpriest/php-src/issues/18|github#18]]
 +    - <del>$foo->bar[] = 1</del>
 +    - <del>$foo->bar[123] = 4</del>
 +    - <del>$foobar = &$foo->bar</del>
 +    - <del>$foo->bar->baz = $x with $bar being object or empty or non-object value (proper error and no leaks -- Stas)</del>
 +    - <del>$foo->bar{$x} = "foo" (string offsets), also checks that non-string vars work fine</del>
 +    - <del>$foo->bar = &bar->baz, etc. with modifying both sides and checking everything works (including modifying via accessors -- Stas)</del>
 +  - <del>Expand on the "Error Messaging" with examples of what has been translated. [[https://github.com/cpriest/php-src/issues/17|github#17]]</del>
 +  - <del>Move all accessor tests into Zend/tests/accessors</del>
 +
 +===== Previously Debated =====
  
 ==== read-only / write-only keywords ==== ==== read-only / write-only keywords ====
Line 131: Line 235:
 } }
 </code> </code>
 +
 +==== Shadowing ====
 +v1.1 has properties shadow accessors, the suggestion is that this be inverted.
 +
 +Specifically this means that in v1.1, if an accessor is defined and no property is defined, then the accessor will be used.  But when a property is assigned/declared then the property will "shadow" the accessor and the accessor will no longer be used until the property is unset().
 +
 +v1.2 Proposes that this be inverted such that if there is an accessor defined for a given property name, the accessor will always be used.  The accessor would be able to get/set/isset/unset the property with the same name as the accessor.  No direct access to the property would be allowed except from within the accessor.  
 +
 +v1.2 proposal seems to make the most sense however it would incur a slight (possibly *very* slight) performance penalty.
  
 ==== isset / unset / attempted writes when no setter / attempted reads when no getter ==== ==== isset / unset / attempted writes when no setter / attempted reads when no getter ====
Line 139: Line 252:
   - Let the compilation occur and at runtime when a disallowed action is attempted, emit a warning and move on.   - Let the compilation occur and at runtime when a disallowed action is attempted, emit a warning and move on.
   - As is currently, either at compilation or at runtime we issue a fatal error and stop execution (probably least preferable if at runtime)   - As is currently, either at compilation or at runtime we issue a fatal error and stop execution (probably least preferable if at runtime)
- 
-==== Interfaces ==== 
-v1.1 fully allows accessors to be defined in interfaces and requires that those accessors be defined in implementing classes.  Some people have suggested that a property defined by the class should also satisfy the requirement. 
- 
-=== Arguments For === 
-  - From the outside observer of an interface, a class which defines a property has satisfied the requirement, because an interface only defines what must be allowed, not what cannot be done. 
- 
-=== Arguments Against === 
-  - Additional overhead on interface checking, would only be allowed for a non-asymmetrical accessor 
-  - Would give userland developers the ability to write poor code as properties are non-observable (object would not know its property was changed) 
  
 ==== Extra shorthand declaration ==== ==== Extra shorthand declaration ====
Line 167: Line 270:
 </code> </code>
  
-===== Issues =====+==== Interfaces ==== 
 +v1.1 fully allows accessors to be defined in interfaces and requires that those accessors be defined in implementing classes.  Some people have suggested that a property defined by the class should also satisfy the requirement.
  
-==== Static Accessors ==== +=== Arguments For === 
-Stas pointed out a number of critical mistakes with the current implementation of static accessors.  They would not currently work under these circumstances: +  - From the outside observer of an interface, a class which defines a property has satisfied the requirement, because an interface only defines what must be allowed, not what cannot be done.
-  - Class defined after a static accessor reference was made +
-  - Dynamic variable use of an accessor --> $class::$variable = 5;+
  
-The current implementation "back-patches" the op_array at the time of a static variable declaration by patching the op array into a method call.  zend_do_assign() further detects these calls to %%ex: __getStaticHours()%% into %%__setStaticHours()%%, and a change to pass_two() detects invalid static accessor gets (those which were not properly backpatched with zend_do_assign()) +=== Arguments Against === 
- +  - Additional overhead on interface checking, would only be allowed for a non-asymmetrical accessor 
-Stas suggested that he could either help me to implement this more appropriately (such as changing the engine to make calls into zend_object_handler.c) so that these calls can be resolved dynamically, or that static accessors be shelved for a future version of accessors. +  - Would give userland developers the ability to write poor code as properties are non-observable (object would not know its property was changed)
- +
- +
-===== TODO ===== +
-  - Exceptions thrown from an accessor reveal underlying implementation details +
-  - get_class_methods() - hide accessors or re-write to call into Reflection* code? +
-  - Check debug_backtrace() output as well +
-  - Add notes to RFC on shadowing behavior of properties and acessors +
-  - Fix automatically implemented isset to use !== rather than != (Thanks Stas!) +
-  - Check that the following should work and write tests for them: +
-    - $foo->bar[] = 1 +
-    - $foo->bar[123] = 4 +
-    - $foobar = &$foo->bar +
-    - $foo->bar->baz = $x with $bar being object or empty or non-object value (proper error and no leaks -- Stas) +
-    - $foo->bar{$x} = "foo" (string offsets), also checks that non-string vars work fine +
-    - $foo->bar = &bar->baz, etc. with modifying both sides and checking everything works (including modifying via accessors -- Stas) +
-  - Expand on the "Error Messaging" with examples of what has been translated.+
  
rfc/propertygetsetsyntax-as-implemented/change-requests.1350841948.txt.gz · Last modified: 2017/09/22 13:28 (external edit)

Page Tools