Remove HHVM support (second edition) #23216
Conversation
This PR removes HHVM compatiblity following symfony#22758 It also removes some unneeded comments
c18f9bd to
09ad255
Compare
| @@ -34,7 +34,6 @@ public function read($path, $locale) | |||
| // Never enable fallback. We want to know if a bundle cannot be found | |||
| $bundle = new \ResourceBundle($locale, $path, false); | |||
| } catch (\Exception $e) { | |||
| // HHVM compatibility: constructor throws on invalid resource | |||
There was a problem hiding this comment.
shouldn't the try/catch be removed then? (same below)
There was a problem hiding this comment.
There is a config in Intl, that translate errors into exceptions. Before removing the try-catch, this has to be tested with that intl config intl.use_exceptions.
There was a problem hiding this comment.
PHP actually throws an exception. That's why I didn't remove it.
There was a problem hiding this comment.
PHP 7.0 and above throws an exception without that config. That's true.
| $lightTrace = $this->cleanTrace($backtrace, $type, $file, $line, $throw); | ||
| $this->traceReflector->setValue($errorAsException, $lightTrace); | ||
| } else { | ||
| $this->traceReflector->setValue($errorAsException, array()); | ||
| $backtrace = array(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
there are a few more HHVM cases below this line
There was a problem hiding this comment.
I didn't removed them because the code is pretty much tricky and I'm not sure what it's used for so I didn't remove anything. ^^'
There was a problem hiding this comment.
I'd say
diff --git a/src/Symfony/Component/Debug/ErrorHandler.php b/src/Symfony/Component/Debug/ErrorHandler.php
index 6fb2c94..ddcf193 100644
--- a/src/Symfony/Component/Debug/ErrorHandler.php
+++ b/src/Symfony/Component/Debug/ErrorHandler.php
@@ -444,32 +444,25 @@ class ErrorHandler
&& ('trigger_error' === $backtrace[$i - 1]['function'] || 'user_error' === $backtrace[$i - 1]['function'])
) {
// Here, we know trigger_error() has been called from __toString().
- // HHVM is fine with throwing from __toString() but PHP triggers a fatal error instead.
+ // PHP triggers a fatal error when throwing from __toString().
// A small convention allows working around the limitation:
// given a caught $e exception in __toString(), quitting the method with
// `return trigger_error($e, E_USER_ERROR);` allows this error handler
// to make $e get through the __toString() barrier.
foreach ($context as $e) {
- if (($e instanceof \Exception || $e instanceof \Throwable) && $e->__toString() === $message) {
- if (1 === $i) {
- // On HHVM
- $errorAsException = $e;
- break;
- }
+ if ($e instanceof \Throwable && $e->__toString() === $message) {
self::$toStringException = $e;
return true;
}
}
- if (1 < $i) {
- // On PHP (not on HHVM), display the original error message instead of the default one.
- $this->handleException($errorAsException);
+ // Display the original error message instead of the default one.
+ $this->handleException($errorAsException);
- // Stop the process by giving back the error to the native handler.
- return false;
- }
+ // Stop the process by giving back the error to the native handler.
+ return false;
}
}
}
| @@ -39,7 +39,6 @@ public function load($resource, $locale, $domain = 'messages') | |||
| try { | |||
| $rb = new \ResourceBundle($locale, $resource); 8000 | |||
| } catch (\Exception $e) { | |||
| // HHVM compatibility: constructor throws on invalid resource | |||
There was a problem hiding this comment.
shouldn't the try-catch be removed instead? already pointed out
There was a problem hiding this comment.
See my comment for IntlBundleReader.php
| @@ -34,7 +34,6 @@ public function read($path, $locale) | |||
| // Never enable fallback. We want to know if a bundle cannot be found | |||
| $bundle = new \ResourceBundle($locale, $path, false); | |||
| } catch (\Exception $e) { | |||
| // HHVM compatibility: constructor throws on invalid resource | |||
There was a problem hiding this comment.
There is a config in Intl, that translate errors into exceptions. Before removing the try-catch, this has to be tested with that intl config intl.use_exceptions.
| @@ -39,7 +39,6 @@ public function load($resource, $locale, $domain = 'messages') | |||
| try { | |||
| $rb = new \ResourceBundle($locale, $resource); | |||
| } catch (\Exception $e) { | |||
| // HHVM compatibility: constructor throws on invalid resource | |||
There was a problem hiding this comment.
See my comment for IntlBundleReader.php
| @@ -39,7 +39,6 @@ public function load($resource, $locale, $domain = 'messages') | |||
| try { | |||
| $rb = new \ResourceBundle($locale, $resource); | |||
| } catch (\Exception $e) { | |||
| // HHVM compatibility: constructor throws on invalid resource | |||
There was a problem hiding this comment.
See my comment for IntlBundleReader.php
There was a problem hiding this comment.
You mean this one? #23216 (comment) I answered.
This patch is provided by @chalasr
|
Thank you @Nek-. |
This PR was squashed before being merged into the 4.0-dev branch (closes #23216). Discussion ---------- Remove HHVM support (second edition) | Q | A | ------------- | --- | Branch? | master | Bug fix? | no | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | none | License | MIT | Doc PR | not needed This PR removes HHVM compatiblity following #22758 as some things were missing. It also removes some useless comments. Commits ------- 471b84c Remove HHVM support (second edition)
This PR removes HHVM compatiblity following #22758 as some things were missing. It also removes some useless comments.