CODING_STANDARDS.md: add rules for #include directives

[MaxKellermann]

This is my proposal for keeping header dependencies small and correct.

See #10220 for the discussion.

[cmb69]

Thank you for the PR! Given that the changes have been controversial so far, it seems to be a good idea to pursue the RFC process. Do you have already written to the internals mailing list in this regard?

[dstogov]

The purpose of includes is too avoid code duplication. This statement says it's better to duplicate code.
May be you had a bit different thoughts, but in current state this looks improper.

[MaxKellermann]

It's better to write a single line of forward declaration instead of including a whole header (which in turn includes a dozen other headers, each of which includes another dozen of more headers).
This doesn't duplicate code any more than an #include duplicates code.
It's the same amount of code (one line), but the forward declaration is better because it's trivial to process by the compiler, expresses exactly what it does, comes without namespace pollution, without I/O, without parsing more code, and so on.

[dstogov]

It's better to write a single line of forward declaration instead of including a whole header

and then fix this forward declaration in all places instead of a single place...

[MaxKellermann]

What needs to be fixed, in your opinion?

[dstogov]

The C declaration code. It's not always once and forever. Functions may change their prototypes, unions may be turned into structures, etc

[cmb69]

If these updates can be automated, and apparently that is possible (never used that tool, though), it might not be that bad. :)

Actually, I was in favor of this include style, if at least system headers also adhered to it – but obviously, they don't (at least most of them).

[MaxKellermann]

I see what you mean, and I should clarify that only structs and unions (and their typedefs) should be forward-declared, not functions - function prototypes (which are another kind of forward declaration) should not, because prototypes are too complex and indeed too much in motion.
If something gets switched between union and struct (which I think is very rare), then you'll get compiler errors which are trivial to fix (a simple sed will fix it) - just like you'd get compiler errors for all callers if you change a function prototype. The latter (which is not affected by my proposal) is not trivial to fix, but still happens all the time and you manage it easily.
Means: there's no reason to believe that struct/union switches are too difficult to do just because there are forward declarations.

[MaxKellermann]

if at least system headers also adhered to it – but obviously, they don't

Many projects like glibc, GCC (libstdc++) and Boost are aware of the problem, and each new version improves - which, of course, causes breakages in lots of buggy software that relies on header bloat in system headers and forgets to include necessary headers. For example, several of my projects received bug reports from people trying to compile my code with GCC13....

But anyway, even if system headers are too bloaty, that's just one more reason why doing it correctly in one's own software. Fewer includes means fewer indirect includes, which means more verification for correctness.

My proposal is really common sense. I'm really surprised about the resistance that suddenly emerges here.

[MaxKellermann]

Do you have already written to the internals mailing list in this regard?

I've just posted: https://marc.info/?l=php-internals&m=167387060703714&w=2

[MaxKellermann]

Draft PR to "revert the revert" (and for your review while we discuss this PR): #10345

[MaxKellermann]

@dstogov, in your review here, you objected to forward declarations. Yet I found lots of forward declarations authored by you, for example:

php-src/Zend/Optimizer/zend_func_info.h

Lines 44 to 45 in bff7a56

	typedef struct _zend_func_info zend_func_info;
	typedef struct _zend_call_info zend_call_info;

These two look very much like the code I submitted, which you asked to revert. You added it in commit c88ffa9

More forward declarations authored by you (excerpt):

php-src/ext/opcache/jit/zend_jit.h

Lines 90 to 92 in bff7a56

	typedef struct _zend_jit_trace_rec zend_jit_trace_rec;
	typedef struct _zend_jit_trace_stack_frame zend_jit_trace_stack_frame;
	typedef struct _sym_node zend_sym_node;

php-src/Zend/zend_types.h

Lines 86 to 87 in bff7a56

	typedef struct _zend_class_entry zend_class_entry;
	typedef union _zend_function zend_function;

php-src/Zend/zend_globals.h

Lines 68 to 69 in bff7a56

	typedef struct _zend_vm_stack *zend_vm_stack;
	typedef struct _zend_ini_entry zend_ini_entry;

These findings make it difficult for me to understand your objections.

[dstogov]

@MaxKellermann some of if these declarations are done on purpose (to break dependencies), or declare recursive data structures (e.g. zend_class_entry and zend_function declarations contain pointers to them selves), others might stay in respect to the old code ( I didn't try to change source code without a practical reason).

[MaxKellermann]

@dstogov, look at commit 8a6d73b which adds a forward declaration of zend_ini_entry to zend_globals.h, because the commit adds _zend_executor_globals.error_reporting_ini_entry. You could have just added #include "zend_ini.h", but you decided to add a forward declaration. I agree with that, that was a good idea. But it contradicts with what I understood is your opinion about forward declarations. I don't get it.

If the majority of voters decides that forward declarations are bad, we need to change this forward declaration to #include, don't we?

P.S. this commit demonstrates how 2 of the forward declarations added by you can easily be avoided: CM4all@ef0c995
(Most of the "noise" in this commit is due to the chaotic nature of PHP's headers, it really fixes header bugs necessary to remove the forward declarations, something my work seeks to fix.)

[MaxKellermann]

The RFC was accepted by a majority of 52%, but a supermajority was necessary, therefore the RFC was declined and this PR will not be merged.
https://wiki.php.net/rfc/include_cleanup