Bug #79865 [Fbk]: Requiring a file twice with `require` doesn't reload anonymous classes

From: Date: Fri, 17 Jul 2020 12:22:49 +0000
Subject: Bug #79865 [Fbk]: Requiring a file twice with `require` doesn't reload anonymous classes
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-228096@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=79865&edit=1 ID: 79865 Updated by: nikic@php.net Reported by: d28b312d at opayq dot com Summary: Requiring a file twice with require doesn't reload anonymous classes Status: Feedback Type: Bug Package: *General Issues Operating System: N/A PHP Version: Irrelevant Block user comment: N Private report: N New Comment: Just tried your test case on current 7.4 HEAD and I get as expected: Title: foo Get Name: foo Title: bar Get Name: bar So I think this is just a matter of an outdated PHP version. Previous Comments: ------------------------------------------------------------------------ [2020-07-17 10:49:44] nikic@php.net Pretty sure this has already been fixed (this falls under the "RTD key collision" problem internally). Are you sure that you're on the latest PHP 7.4 release? ------------------------------------------------------------------------ [2020-07-17 10:45:33] d28b312d at opayq dot com > This makes sense to me. It's a class definition, even if it doesn't have a name. And > just like how anonymous functions don't get redefined, anonymous classes shouldn't either. When you re-include a file with a concrete class definition it will emit a warning saying the class can't be redefined. With this you get no output at all, to me that's really bad for the developer who may think their function or class has been reloaded. I'm not quite sure why they don't get reloaded as they're anonymous and so shouldn't get cached surely? If you want a concrete class then make them one? > In other words, what do you believe should happen? Should test.php not be allowed to execute a > second time? What is the bad code that PHP should warn about? And what is the correct alternative > for the developer? I think that it should re-evaluate and reload the class and/or functions if they're anonymous. But failing that at least emit a warning to developers of a re-definition that isn't going to happen. Doing something silently makes for awful debugging. The file clearly has different contents to before (as can be seen by the echo) but the class/functions don't get reloaded with the changes, which would be very unexpected to most developers I'd think. If you want to cache stuff, then you might as well cache the whole file and not change the echo statements either, in which case what's the point of require vs require_once if sometimes it'll behave like a require_once for some statements and not others. ------------------------------------------------------------------------ [2020-07-15 22:03:07] requinix@php.net This makes sense to me. It's a class definition, even if it doesn't have a name. And just like how anonymous functions don't get redefined, anonymous classes shouldn't either. In other words, what do you believe should happen? Should test.php not be allowed to execute a second time? What is the bad code that PHP should warn about? And what is the correct alternative for the developer? ------------------------------------------------------------------------ [2020-07-15 21:04:56] d28b312d at opayq dot com Description: ------------ Requiring a file twice (using require _not_ require_once), will reload a file and run all the code but won't redefine anonymous classes in that code. The file is loaded and the code to define the object is run, but as the hash is the same it doesn't redefine it. The echo statement above the anonymous class _is_ run both times and is different both times, as expected. There is _no_ warning and _no_ change, but the rest of the code still continues to run. This seems like very unexpected behaviour. This is the vld output gist: https://gist.github.com/ElvenSpellmaker/940fa8b4fd5523a81a42fb9686266d67 From that gist, the first time the class is loaded: ------------------------------------------------------------------------------------- 3 0 E > ECHO 'Title%3A+foo' 1 ECHO '%0A' 5 2 DECLARE_ANON_CLASS 9.88131e-324 3 NEW $1 0 4 DO_FCALL 0 5 > RETURN $1 9 6* > RETURN 1 And the second time: ------------------------------------------------------------------------------------- 3 0 E > ECHO 'Title%3A+bar' 1 ECHO '%0A' 5 2 DECLARE_ANON_CLASS 9.88131e-324 3 NEW $1 0 4 DO_FCALL 0 5 > RETURN $1 9 6* > RETURN 1 As you can see there doesn't appear to be a point where it skips the redefine, but I assume as the hash comes back as '9.88131e-324' both times it doesn't actually declare the class again. Test script: --------------- index.php: <?php $class = require 'test.php'; echo 'Get Name: ' . $class->getName(); # Replace foo with bar to try and test a class reload $foo = file_get_contents('test.php'); $foo = str_replace('foo', 'bar', $foo); file_put_contents('test.php', $foo); echo "\n"; $class2 = require 'test.php'; echo 'Get Name: ' . $class2->getName(); # Reset file $foo = str_replace('bar', 'foo', $foo); file_put_contents('test.php', $foo); --- test.php: <?php echo 'Title: foo', "\n"; return new class { private $name = 'foo'; public function getName() { return $this->name; } }; Expected result: ---------------- Title: foo Get Name: foo Title: bar Get Name: bar Actual result: -------------- Title: foo Get Name: foo Title: bar Get Name: foo ------------------------------------------------------------------------ -- Edit this bug report at https://bugs.php.net/bug.php?id=79865&edit=1

« previous php.bugs (#228096) next »