Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:119344
Date: Thu, 19 Jan 2023 11:05:03 +0100
To: Kamil Tekiela <tekiela246@gmail.com>
Cc: internals@lists.php.net
Message-ID: <Y8kVzxXiO27dqhhA@swift.blarg.de>
References: <Y8U9B4AVP/R7vazI@swift.blarg.de>
 <CAGBsUrdQAXE2qk_UDQY9v6pYpWH+=bX6S3ou+sttQamtDC1F9A@mail.gmail.com>
 <Y8VI5yanzli8D4Cm@swift.blarg.de>
 <alpine.DEB.2.23.453.2301181536160.23089@singlemalt.home.derickrethans.nl>
 <Y8gZWLQd1SvL6a3n@swift.blarg.de>
 <8DA390C5-5E67-4847-A89F-1A8CCC6C5389@gmail.com>
 <CAGBsUrcHNeMYC1bSUkRgp2YNWjQJFojBxw43gcy3dkzEZfF4wg@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <CAGBsUrcHNeMYC1bSUkRgp2YNWjQJFojBxw43gcy3dkzEZfF4wg@mail.gmail.com>
Subject: Re: [PHP-DEV] RFC: rules for #include directives
From: max+php@blarg.de (Max Kellermann)

On 2023/01/18 18:51, Kamil Tekiela <tekiela246@gmail.com> wrote:
> > #include "zend_portability.h" // for BEGIN_EXTERN_C
> What if in future the need for BEGIN_EXTERN_C disappears? Who is going to
> remember to update the comment?

(I just addressed that concern in reply to Rowan's email.)

> As you said yourself, this refactoring has no practical effect on
> compilation times.

No, I did not say that.  I said:

 "in this early stage, there is no measurable speedup yet"

 "Once I have disentangled and splitted those fat headers [...], the
 speedup will be measurable and perceptible."

> Another advantage listed is "more correct code". Perhaps it is, but does it
> mean that the current situation is likely to cause more frequent PHP bugs?
> Would this refactoring help in finding PHP bugs more quicker?

This isn't the kind of "correct" that (directly) prevents runtime
bugs; it's the kind of "correct" that leads to having more readable
code and thus making maintenance easier.  If maintenance is easier,
developers will likely produce fewer runtime bugs, but that's only an
indirect effect.

Maintenance becomes easier because it should be easier to find out
which header to include to make a certain feature available.  Right
now, the headers are a big mess - when do I have to include zend.h,
when do I have to include php.h and what is php_compat.h, and I
included zend_types.h but why are certain types not available?  Is
there documentation about this?  I found none...  And PHP extensions I
saw look like all people just try'n'error and copy'n'paste to
"develop" code.

> I mean things like a bug fix using a symbol that hasn't been used in
> the file but is included through one of the more generic
> headers. Once such a commit is merged into master, it may turn out
> that the symbol lacks a new include. This adds unnecessary work for
> bug fixers who will now have to verify this and find the appropriate
> include during merges.

The same can be said about all code changes - all code changes make
merging more difficult; sometimes with merge conflicts, sometimes with
header changes, sometimes with API changes, and most dangerously:
semantic changes.

Anyway, you care about the people who merge a bugfix to another
branch.  Yes, that may sometimes lead to a build problem.  Only push a
merge commit after you have verified that it builds.  Each breakage is
extra work.

But in return, after the transition to "cleaner code", maintenance and
development gets easier.  There, you get the time back you invested
earlier.

Max