Compatibility with preloading in PHP 7.4

[brendt]

There's an issue with the \Carbon\Traits\Date trait preventing being able to properly preload a laravel project (and any project using carbon).

The issue has to do with unknown constants, more specifically: https://github.com/briannesbitt/Carbon/blob/master/src/Carbon/Traits/Date.php#L541-L556

I've been trying to preload Laravel over the past few weeks, and got it working on the latest 7.4 dev build (as of writing that's 7.4.2-dev).

This is the preload script I'm using: https://github.com/brendt/aggregate.stitcher.io/blob/master/preload.php

More information about what goes wrong can be found in https://bugs.php.net/bug.php?id=78918

I was not able to test preloading Laravel due to

Carbon/src/Carbon/Traits/Date.php

Lines 541 to 556 in 298d9f0

	protected static $days = [
	// @call isDayOfWeek
	self::SUNDAY => 'Sunday',
	// @call isDayOfWeek
	self::MONDAY => 'Monday',
	// @call isDayOfWeek
	self::TUESDAY => 'Tuesday',
	// @call isDayOfWeek
	self::WEDNESDAY => 'Wednesday',
	// @call isDayOfWeek
	self::THURSDAY => 'Thursday',
	// @call isDayOfWeek
	self::FRIDAY => 'Friday',
	// @call isDayOfWeek
	self::SATURDAY => 'Saturday',
	];

. This references constants that are not defined in the trait and are only available in the class using the trait. This will need to either be fixed, or the trait and it's dependencies skipped.

Commenting out these lines gets preloading to work. I'm not sure what the best solution is, but as of now this small piece of code prevents us from preloading Laravel (once 7.4.2 is released)

[kylekatarnls]

Hi I tried different ways to run opcache (enabling it in CLI and in Apache) using your script with all PHP files of Carbon and I get "[Preloader] Preloaded Carbon\Traits\Date" and the rest of the script working well even if I don't comment the days array.

I run PHP 7.4.0.

I have no clue of why OPCache fail on this code and would it expect to be used instead.

I tried randomly a change that would be acceptable using CarbonInterface instead of self.

Please try to require "nesbot/carbon": "dev-master" and tell me if you're able to preload your app with this.

If not I will need more input:

• A case to reproduce the bug on Travis that involves only Carbon package (outside laravel framework)
• Clear details the assertion failure (maybe opcache guidelines to respect)

Also @nikic talked about class_alias in the PHP bug track, which does not sound related to Carbon code you mention.

He's also talking about fixing things PHP side. So I'm not sure it's something we should handle in Carbon.

[kylekatarnls]

@brendt, @frinux, @vladyslavstartsev, @chrysanthos, @YevhenKushnir

I'm ready to help, but removing code is not a solution. If one of you may test requiring "nesbot/carbon": "dev-master" I can merge it if this solves the problem. Else I would consider any PR someone would submit that would make the code compatible with preloading.

But for now, as the code mentioned above is valid PHP code, I consider this is an OPCache bug, not a Carbon bug. @brendt you suggest to Nikita to close https://bugs.php.net/bug.php?id=78918 as you get preloading working commenting the problematic code. But I think it's not a reason to close the bug, OPCache fails on this code, this deserves an explanation about why and what to do instead at least, or simply a patch to OPCache to support this code.

[nikic]

@kylekatarnls The problem here is that you are using self::SUNDAY, while the trait itself has no const SUNDAY. Preloading requires all initializer expressions in preloaded classes to be fully evaluated, which is not possible in this case, because the referenced class constant simply does not exist.

Your code is of course not wrong in itself, as you do not expect Date::$days to be directly accessed, rather than being accessed in a class that uses the trait (where, through an implicit contract, you require const SUNDAY to be defined). Unfortunately, PHP currently does not prevent such direct accesses (which might change for PHP 8, see php/php-src#4829), which means we need to account for this possibility. If this weren't the case, we would not be forced to evaluate initializers inside traits.

Your change in the dev-opcache-preloading branch should work fine.

[kylekatarnls]

Thanks @nikic for the confirmation, if this syntax may become invalid in PHP 8, I'm OK to anticipate this change and use our interface instead. It's theoretically a breaking change as this trait may have been used with those constants set to different values. But this should be pretty rare and those users can easily override the whole protected static $days so merging this change and adding a quick note about this update should be just fine.

[kylekatarnls]

This is supposed to be fixed by #1972. You can test it requiring "nesbot/carbon": "dev-master"

This will be released in 2.29.0.

[brendt]

Hi Kyle, thanks for looking into this! I can confirm it works now with these changes. It might be best to 2.29.0 around the time or before PHP 7.4.2 is released, since that's the release which gets preloading to work.