Php7 Problem with TemplateProcessor Destructor #2554
Conversation
Fix PHPOffice#2548. A particularly perplexing problem accidentally introduced by PR PHPOffice#2475. Problem does not arise for Php8, and does not arise for Php7 unit tests. But, running *not* under Phpunit auspices with Php7 can cause a warning message at destructor time if the `save` function has been used. A very artificial test is introduced to test this situation.
| $result = false; | ||
| } | ||
| if ($result === false) { |
There was a problem hiding this comment.
| $result = false; | |
| } | |
| if ($result === false) { |
There was a problem hiding this comment.
No, your suggestion changes the program logic. We should get false for the result of close. Php8 changed that, and I have filed a bug with Php about it. For Php7, we still get false, and we want to throw an exception (to avoid a compatibility break for Php7, which is what this change is all about). We also want to throw an exception when Php8 (erroneously) throws an exception for close. The way to handle both is to evaluate $result outside the try-catch, as I have currently coded it.
There was a problem hiding this comment.
No, your suggestion changes the program logic. We should get
falsefor the result ofclose. Php8 changed that, and I have filed a bug with Php about it. For Php7, we still get false, and we want to throw an exception (to avoid a compatibility break for Php7, which is what this change is all about). We also want to throw an exception when Php8 (erroneously) throws an exception forclose. The way to handle both is to evaluate$resultoutside the try-catch, as I have currently coded it.
I would like to ask, how can I get Progi1984 to review my code?
There was a problem hiding this comment.
There is a big backlog, and I think he is being very careful and methodical in how he's trying to catch up. I'm sure he will get to your PR, perhaps not as quickly as you would like, but hopefully not too long.
Description
A particularly perplexing problem accidentally introduced by PR #2475. Problem does not arise for Php8, and does not arise for Php7 unit tests. But, running not under Phpunit auspices with Php7 can cause a warning message at destructor time if the
savefunction has been used. A very artificial test is introduced to test this situation.Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context.
Fixes # 2548
Checklist:
composer run-script check --timeout=0and no errors were reported