Skip to content
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

Drop --with-uncritical-warn-choke configuration option #17554

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

cmb69
Copy link
Member

@cmb69 cmb69 commented Jan 23, 2025

This selection of suppressed warnings is pretty arbitrary, and apparently disabling it does not raise any more warning for whole php-src (except for -Wno-deprecated-declarations). As such it appears to be pretty useless, and it seems to be more appropriate to let users select which warnings to suppress via manually set CFLAGS.

Since we already apply /wd4996[1] when building with MSVC, and there are indeed plenty of deprecation warnings, for now, we apply -Wno-deprecated-declarations for Clang builds unconditionally.

[1] https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996


Note that I'm planning to have a closer look at the deprecations soon. It seems it's possible to just suppress some of these (such as the deprecated POSIX function names) by setting some compile time flags, which appears to a be nice improvement over suppressing all deprecation warnings.

@cmb69
Copy link
Member Author

cmb69 commented Jan 24, 2025

Note that I'm planning to have a closer look at the deprecations soon.

Besides the POSIX function names (which we likely want to ignore), the Unsafe CRT Library functions (which we probably want to address), and some WinSock deprecations (locally suppressed by _WINSOCK_DEPRECATED_NO_WARNINGS; need to have a closer look), the only two issues I've found are #17556 and #17557.

This selection of suppressed warnings is pretty arbitrary, and
apparently disabling it does not raise any more warning for whole
php-src (except for `-Wno-deprecated-declarations`).  As such it
appears to be pretty useless, and it seems to be more appropriate to
let users select which warnings to suppress via manually set `CFLAGS`.

Since we already apply `/wd4996`[1] when building with MSVC, and there
are indeed plenty of deprecation warnings, for now, we apply
`-Wno-deprecated-declarations` for Clang builds unconditionally.

[1] <https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996>
@cmb69 cmb69 force-pushed the cmb/drop-uncritical-warn-choke branch from a374aef to fe4dcfd Compare January 25, 2025 19:12
@cmb69
Copy link
Member Author

cmb69 commented Jan 25, 2025

we apply -Wno-deprecated-declarations for Clang builds unconditionally.

Oops, totally forgot to commit this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants