-
Notifications
You must be signed in to change notification settings - Fork 7.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add -Wno-implicit-fallthrough flags conditionally #13331
Conversation
ed618c1
to
8d0b781
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
RHEL 7 is EOL in June of this year.
Does it really make sense to fix this?
Also the main reason for this flag, other than opcache, is because we don't control the bundle libraries. For opcache I don't remember the reason, so it might make sense to remove the flag and see what happens again.
I m a tad on the fence for the change too.
|
I don't mind this change either, but considering that GCC 8.1 supports C17 and RHEL 8 comes with GCC 8.x I would imagine that we might bump our minimum C requirement soon anyway (mainly for the atomics supports I'd gather). |
Hm, these flags are difficult :D I'll wait a bit with this. I've checked some of the compilers and basically all decent compilers today should support this flag. Even Solaris Studio compiler, Intel compiler, and MS Visual Studio with 2019 in its |
I wish you would merge this PR so that I wouldn't have to keep maintaining our patch. |
Older GCC versions (< 7.0) don't support the -Wno-implicit-fallthrough compiler flag. This adds the flag conditionally in case some other compiler will run into same issue. Fixes phpGH-13330
8d0b781
to
be8edd5
Compare
* PHP-8.2: Append -Wno-implicit-fallthrough flag conditionally (#13331)
* PHP-8.3: Append -Wno-implicit-fallthrough flag conditionally (#13331)
Sure, let's add this conditionally then. Added to PHP-8.2 and up. In the upcoming PHP-8.4 this compile flag was even reduced down to the minimum where it is still needed. The ext/date is still pending the merge, so somewhere right after the PHP 8.4 #14187 |
Older GCC versions (< 7.0) don't support the -Wno-implicit-fallthrough compiler flag. This adds the flag conditionally in case some other compiler will run into same issue.
Fixes #13330