rfc:final_by_default_anonymous_classes

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:final_by_default_anonymous_classes [2024/01/15 09:55] – danogrfc:final_by_default_anonymous_classes [2024/02/06 19:51] (current) – Move to declined ilutov
Line 2: Line 2:
   * Date: 2023-12-23   * Date: 2023-12-23
   * Author: Daniil Gentili <daniil@daniil.it>   * Author: Daniil Gentili <daniil@daniil.it>
-  * Status: Under discussion+  * Status: Declined
  
 ===== Introduction ===== ===== Introduction =====
Line 12: Line 12:
 This RFC proposes to make anonymous classes final by default.  This RFC proposes to make anonymous classes final by default. 
  
-This should also allow some additional opcache optimizations, such as any JIT logic gated behind a check on ZEND_ACC_FINAL, i.e. https://github.com/php/php-src/blob/master/ext/opcache/jit/zend_jit_trace.c#L4507.+The main reason for this change, in my view, is to allow some additional opcache optimizations, such as any JIT logic gated behind a check on ZEND_ACC_FINAL, i.e. https://github.com/php/php-src/blob/master/ext/opcache/jit/zend_jit_trace.c#L4507. 
 + 
 +Another reason is mentioned by Nikita in https://externals.io/message/121906#121909: "It makes very little sense to me that everyone needs to explicitly mark their anonymous classes as final just because there is a class_alias loophole that could, in theory, have been used to extend anonymous classes in the past".
  
 Example, extending an anonymous class throws an error: Example, extending an anonymous class throws an error:
Line 27: Line 29:
  
  
-Optionally, an ''open'' keyword can be introduced to optionally make anonymous classes non-final.+Optionally, an ''open'' keyword inspired by Kotlin can be introduced to optionally make anonymous classes non-final.
  
 Example, extending an open anonymous class does not throw an error: Example, extending an open anonymous class does not throw an error:
Line 37: Line 39:
 </code> </code>
  
-On one hand, looking at code like ''new class {}'', you would assume that since the class apparently has no name, it should not be impossible to extend it, but on the other hand, there are valid usecases for extending even anonymous classes (comments in the PR (https://github.com/php/php-src/pull/11126 referenced proxying), I can think of phpunit mocking to a much, much lesser extent given that you should realistically (hopefully) never have to mock an anonymous class that does not already implement an interface), and completely precluding the possibility of extending a class that+Rationale regarding ''open'': 
 + 
 +On one hand, looking at code like ''new class {}'', you would assume that since the class apparently has no name, it should not be impossible to extend it, but on the other hand, there are valid usecases for extending even anonymous classes (comments in the PR (https://github.com/php/php-src/pull/11126 referenced proxying), and I can also think of phpunit mocking: completely precluding the possibility of mocking a class that
  
   - Has a name (even if it's not immediately obvious)   - Has a name (even if it's not immediately obvious)
Line 48: Line 52:
 Still, there are some useful patterns, mainly regarding testing and mocking, for example I use https://github.com/dg/bypass-finals as a dev dependency to make all final classes non-final at runtime to allow mocking in phpunit, but it works by installing a custom default stream contexts that intercepts requires, tokenizes files and removes final keywords from classes; this approach would break for anonymous classes if they were rendered final by default without an option to make them non-final.  Still, there are some useful patterns, mainly regarding testing and mocking, for example I use https://github.com/dg/bypass-finals as a dev dependency to make all final classes non-final at runtime to allow mocking in phpunit, but it works by installing a custom default stream contexts that intercepts requires, tokenizes files and removes final keywords from classes; this approach would break for anonymous classes if they were rendered final by default without an option to make them non-final. 
  
-This is why I remain ambivalent about the options, as seen both in my emails and in the original text of the RFC:+Thus, I also propose the additional of an optional ''open'' keyword that can be used to make anonymous classes non-final.
  
-Personally, I would have instead preferred the much cleaner approach of making all anonymous classes final by default, (preferrably) without offering the option to make them non-final. +The implementation PR will be provided by me in case of acceptance.
- +
-However, I understand that this might be a little bit too restrictive for something that may have some valid usecases, even if extending anonymous classes currently requires some hack-ish workarounds with class_alias. +
- +
-As an extra proposal possibly requiring a separate RFC, it might be a good idea to also disallow the use of ''class_alias'' altogether for final anonymous classes (suggested by nikolas-grekas in https://github.com/php/php-src/pull/11126#issuecomment-1522042841).+
  
 ===== Backward Incompatible Changes ===== ===== Backward Incompatible Changes =====
Line 69: Line 69:
  
 ===== Proposed Voting Choices ===== ===== Proposed Voting Choices =====
 +
 +Voting started on January 15th, 2024 and runs until January 30th, 2024, 00:00 UTC. 
  
 2/3 required to accept. 2/3 required to accept.
  
-<doodle title="Make final anonymous classes final by default?" auth="danog" voteType="single" closed="true">+<doodle title="Make final anonymous classes final by default?" auth="danog" voteType="single" closed="true" closeon="2024-01-30T00:00:00Z">
    * Yes    * Yes
    * No    * No
 </doodle> </doodle>
  
-<doodle title="Add an optional open keyword for anonymous classes?" auth="danog" voteType="single" closed="true">+ 
 +2/3 required to accept. 
 + 
 +<doodle title="Add an optional open keyword for anonymous classes?" auth="danog" voteType="single" closed="true" closeon="2024-01-30T00:00:00Z">
    * Yes    * Yes
    * No    * No
Line 85: Line 90:
 ===== References ===== ===== References =====
  
-  * Internals discussion: https://externals.io/message/121356, https://externals.io/message/121685+  * Internals discussion: https://externals.io/message/122026, https://externals.io/message/121356, https://externals.io/message/121685, https://externals.io/message/122026
   * Previous optionally final anonymous classes RFC: https://wiki.php.net/rfc/final_anonymous_classes   * Previous optionally final anonymous classes RFC: https://wiki.php.net/rfc/final_anonymous_classes
  
  
rfc/final_by_default_anonymous_classes.1705312553.txt.gz · Last modified: 2024/01/15 09:55 by danog

Page Tools