rfc:include_cleanup

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
Last revisionBoth sides next revision
rfc:include_cleanup [2023/01/18 14:53] – Status: Under Discussion maxkrfc:include_cleanup [2023/02/01 09:21] – Status:Voting maxk
Line 3: Line 3:
   * Date: 2023-01-18   * Date: 2023-01-18
   * Author: Max Kellermann, max.kellermann@ionos.com   * Author: Max Kellermann, max.kellermann@ionos.com
-  * Status: Under Discussion+  * Status: Voting
   * First Published at: https://github.com/php/php-src/pull/10216   * First Published at: https://github.com/php/php-src/pull/10216
  
Line 55: Line 55:
   * later Dmitry Stogov changed his mind, saying "This is just a useless permutation. [...] How is this clearly?" https://github.com/php/php-src/pull/10220#issuecomment-1383739816 which Derick Rethans agreed with ("It adds nothing but clutter." https://github.com/php/php-src/pull/10220#issuecomment-1383784480)   * later Dmitry Stogov changed his mind, saying "This is just a useless permutation. [...] How is this clearly?" https://github.com/php/php-src/pull/10220#issuecomment-1383739816 which Derick Rethans agreed with ("It adds nothing but clutter." https://github.com/php/php-src/pull/10220#issuecomment-1383784480)
   * Dmitry Stogov rejected the idea of forward declarations because "Functions may change their prototypes, unions may be turned into structures, etc" (https://github.com/php/php-src/pull/10338#discussion_r1071169392)   * Dmitry Stogov rejected the idea of forward declarations because "Functions may change their prototypes, unions may be turned into structures, etc" (https://github.com/php/php-src/pull/10338#discussion_r1071169392)
 +  * Derick Rethans believes that "comments that go out of date as to why a header is included is also clutter" (though this can be said about all documentation/code comments, and the argument can thus be used to reject all code comments) 
 +  * Dmitry Stogov and Derick Rethans worried that these changes would make merging branches harder, due to possible merge conflicts or build failures because the source branch does not have correct #includes and now uses a new symbol from a header which was not included (though the same can be said about all code changes, and changing #includes is not very intrusive, because these rarely ever change in stable branches, and there's never a semantic change) 
 +  * There was some opposition against struct forward declarations; they allegedly "decrease code readability".
  
 ===== Backward Incompatible Changes ===== ===== Backward Incompatible Changes =====
Line 99: Line 101:
  
 ===== Proposed Voting Choices ===== ===== Proposed Voting Choices =====
-  * Yes (clean up headers and #include directives+  * Clean up #include directives? (Yes/No)
-  * No (do not clean up headers and #include directives) +
- +
-Other questions that could possibly be decided on:+
   * Is it allowed to document an #include line with a code comment? (Yes/No)   * Is it allowed to document an #include line with a code comment? (Yes/No)
 +  * Is it allowed to forward-declare structs/unions/typedefs? (Yes/No)
   * Is it allowed to split a large header, e.g. move zend_result to a separate header, to reduce dependencies on catch-all headers such as zend_types.h? (Yes/No)   * Is it allowed to split a large header, e.g. move zend_result to a separate header, to reduce dependencies on catch-all headers such as zend_types.h? (Yes/No)
 +
 +===== Vote =====
 +
 +Voting started on 2023-02-01 (two weeks after the RFC was posted to php-internals) and will end on 2023-02-15 at 15:00 UTC (another two weeks).
 +
 +Primary vote:
 +
 +<doodle title="Should #include directives be cleaned up?" auth="maxk" voteType="single" closed="false" closeon="2023-02-15T15:00:00Z">
 +   * Yes
 +   * No
 +</doodle>
 +
 +Secondary (clarifications on how to clean up):
 +
 +<doodle title="Is it allowed to document an #include line with a code comment?" auth="maxk" voteType="single" closed="false" closeon="2023-02-15T15:00:00Z">
 +   * Yes
 +   * No
 +</doodle>
 +
 +<doodle title="Is it allowed to forward-declare structs/unions/typedefs?" auth="maxk" voteType="single" closed="false" closeon="2023-02-15T15:00:00Z">
 +   * Yes
 +   * No
 +</doodle>
 +
 +<doodle title="Is it allowed to split a large header to reduce dependencies?" auth="maxk" voteType="single" closed="false" closeon="2023-02-15T15:00:00Z">
 +   * Yes
 +   * No
 +</doodle>
  
 ===== Patches and Tests ===== ===== Patches and Tests =====
Line 111: Line 139:
 https://github.com/php/php-src/pull/10338 tracks the specification. https://github.com/php/php-src/pull/10338 tracks the specification.
 https://github.com/php/php-src/pull/10345 tracks my proposed implementation. https://github.com/php/php-src/pull/10345 tracks my proposed implementation.
 +
 +https://github.com/php/php-src/pull/10410 is a PR splitted from https://github.com/php/php-src/pull/10345 with some minimal cleanup, as suggested on php-internals by Jakub Zelenka; it contains compatibility tweaks so (bad) third-party extensions do not break, such as always including errno.h from php.h
 +
 +https://github.com/php/php-src/pull/10404 is a PR that adds a few third-party extensions to the CI; more extensions could easily be added. This is slighly out of this RFC's scope, but may help those who worry about extension breakages.
 +
 +If voters decide that #include directives should not have code comments, then existing comments should be removed to reduce "clutter"; see https://github.com/php/php-src/pull/10472
  
 ===== References ===== ===== References =====
rfc/include_cleanup.txt · Last modified: 2023/02/15 15:11 by maxk