[PATCH] better implementation for __HALT_COMPILER();

php.internals

Greg Beaver

19 years ago
Hi, With the first release of pecl/phar, I have started thinking about the implementation of __HALT_COMPILER(); Currently, this code results in a fatal error: file1.php: <?php __HALT_COMPILER(); ?> file2.php: <?php __HALT_COMPILER(); ?> file3.php: <?php include 'file1.php'; include 'file2.php'; // fatal error - can't declare __HALT_COMPILER(); more than once in the same request ?> The current implementation simply defines __COMPILER_HALT_OFFSET__ as the first __HALT_COMPILER();, independent of the file. However, it occurred to me that it might be easy to fix this by introducing a hash of halt offsets by file. After talking to Sara on IRC, she proved she is truly an internals goddess by suggesting instead to use a mangled constant name (__COMPILER_HALT_OFFSET__ + filename) and use the existing global constants hash instead. So, with that helpful tip, I whipped up a patch to provide file-specific __COMPILER_HALT_OFFSET__. This does change behavior in that __COMPILER_HALT_OFFSET__ can only be used within the same file that it is defined. However, this is extremely unlikely to break any existing scripts since it was designed for self-contained installers, and to my knowledge, Phar/PHP_Archive and some auxiliary thing called PHK are the only things using it. The benefits to users of phar-like things would be immense, with no performance degradation for anything except for the worst case constant lookup where a user requests the value of an undefined constant (UPPERCASE constant doesn't exist, lowercase constant doesn't exist). Anyways, the patch is very short, so I've attached it. This patch is against PHP_5_2 (since I consider it a bug fix) and I will happily whip up a patch against HEAD if it is acceptable. My assumption here, of course, is that this would go in 5.2.2 at the earliest since Ilia was waiting for a CVS lock to release in order to put up 5.2.1 while I was coding the patch. Thanks, Greg

Greg Beaver

19 years ago
Gregory Beaver wrote:
> __COMPILER_HALT_OFFSET__. This does change behavior in that > __COMPILER_HALT_OFFSET__ can only be used within the same file that it > is defined. However, this is extremely unlikely to break any existing > scripts since it was designed for self-contained installers, and to my > knowledge, Phar/PHP_Archive and some auxiliary thing called PHK are the > only things using it. The benefits to users of phar-like things would
I meant to say "only other things using it" with the primary usage being FUDForum and other single-file installers that have no need to support multiple __HALT_COMPILER();. Greg

LAUPRETRE François (P)

19 years ago
> From: Gregory Beaver [mailto:greg@chiaraquartet.net]
> Currently, this code results in a fatal error
Are you sure it is a fatal error ? I only get an E_NOTICE in 5.1.6. Did it change in 5.2? Please tell me if it is the case, because I would have to update my documentation.
> However, this is extremely unlikely > to break any existing scripts since it was designed for > self-contained installers, and to my knowledge, > Phar/PHP_Archive and some auxiliary thing called PHK are the > only things using it.
PHK uses __HALT_COMPILER() but does not use the __COMPILER_HALT_OFFSET__ constant. So, for PHK, there is no BC issue with your patch. Regards Francois

Antony Dovgal

19 years ago
On 02/07/2007 01:14 PM, LAUPRETRE François (P) wrote:
>> From: Gregory Beaver [mailto:greg@chiaraquartet.net] > >> Currently, this code results in a fatal error > > Are you sure it is a fatal error ? I only get an E_NOTICE in 5.1.6. Did it change in 5.2? Please tell me if it is the case, because I would have to update my documentation.
It's E_COMPILE_ERROR.
>> However, this is extremely unlikely >> to break any existing scripts since it was designed for >> self-contained installers, and to my knowledge, >> Phar/PHP_Archive and some auxiliary thing called PHK are the >> only things using it. > > PHK uses __HALT_COMPILER() but does not use the __COMPILER_HALT_OFFSET__ constant. > So, for PHK, there is no BC issue with your patch.
-- Wbr, Antony Dovgal