[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D131351#3718728 , @mizvekov wrote: > In D131351#3718725 , @aaron.ballman > wrote: > >> However, we don't care about the type mismatch when doing a redeclaration or >> when

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D131351#3718982 , @mstorsjo wrote: > A different but slightly similar case, where I've run into these errors too, > FWIW: On ARM, when compiling with a hardfloat target, Clang does consider a > function pointer with

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-12 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. A different but slightly similar case, where I've run into these errors too, FWIW: On ARM, when compiling with a hardfloat target, Clang does consider a function pointer with `__attribute__((pcs("aapcs-vfp")))` different from one without, even if the default still in

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-12 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. In D131351#3718725 , @aaron.ballman wrote: > In D131351#3718421 , @mstorsjo > wrote: > >> I found another case of this warning, which is kinda borderline whether it >> really is an

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-12 Thread Matheus Izvekov via Phabricator via cfe-commits
mizvekov added a comment. In D131351#3718725 , @aaron.ballman wrote: > However, we don't care about the type mismatch when doing a redeclaration or > when dropping the attribute, as in: > > __attribute__((noreturn)) void my_exit(void); > void

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-12 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D131351#3718421 , @mstorsjo wrote: > I found another case of this warning, which is kinda borderline whether it > really is an error not: > > #include > static _Noreturn void my_exit(void) { > exit(42); > } >

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-12 Thread Martin Storsjö via Phabricator via cfe-commits
mstorsjo added a comment. I found another case of this warning, which is kinda borderline whether it really is an error not: #include static _Noreturn void my_exit(void) { exit(42); } __attribute__((noreturn)) void (*handler)(void) = my_exit; The fix is simple though, just be

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-11 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D131351#3714968 , @brooksmoses wrote: > For the record, so far we've seen this showing up in the following: > > - A case in a notoriously warning-heavy third-party library where we'd > backported a file from a newer

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-10 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added a comment. Nit: I think it is useful mentioning `-Wincompatible-function-pointer-types` in the commit message, not just in `clang/docs/ReleaseNotes.rst`, so that `git log --grep incompatible-function-pointer-types` will reveal something when the user knows

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-10 Thread Brooks Moses via Phabricator via cfe-commits
brooksmoses added a comment. For the record, so far we've seen this showing up in the following: - A case in a notoriously warning-heavy third-party library where we'd backported a file from a newer version and didn't quite fix all the internal API mismatches. - A ten-year-old bug in a local

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-10 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman added a comment. In D131351#3714062 , @nikic wrote: > This seems to break the test-suite build: > > FAILED: CTMark/ClamAV/CMakeFiles/clamscan.dir/libclamav_readdb.c.o > /root/llvm-compile-time-tracker/llvm-test-suite-build/tools/timeit

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-10 Thread Nikita Popov via Phabricator via cfe-commits
nikic added a comment. This seems to break the test-suite build: FAILED: CTMark/ClamAV/CMakeFiles/clamscan.dir/libclamav_readdb.c.o /root/llvm-compile-time-tracker/llvm-test-suite-build/tools/timeit --summary CTMark/ClamAV/CMakeFiles/clamscan.dir/libclamav_readdb.c.o.time

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-10 Thread Aaron Ballman via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rGaf01f717c48f: Default implicit function pointer conversions diagnostic to be an error (authored by aaron.ballman). Changed prior to commit: https://reviews.llvm.org/D131351?vs=451094=451561#toc

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-09 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman updated this revision to Diff 451094. aaron.ballman added a comment. Herald added a project: clang-tools-extra. Rebased and fixing up clang-tools-extra tests caught by the precommit CI. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131351/new/

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-08 Thread Erich Keane via Phabricator via cfe-commits
erichkeane accepted this revision. erichkeane added a comment. Yep, i think this is a good direction, and code changes look correct to me. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131351/new/ https://reviews.llvm.org/D131351

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-07 Thread Corentin Jabot via Phabricator via cfe-commits
cor3ntin accepted this revision. cor3ntin added a comment. This revision is now accepted and ready to land. LGTM. I agree that this address potential security issues. I'm curious how many people will be affected, but hopefully by doing that early in the clang 16 cycle we might find out! Thanks

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

2022-08-07 Thread Aaron Ballman via Phabricator via cfe-commits
aaron.ballman created this revision. aaron.ballman added reviewers: erichkeane, jyknight, efriedma, clang-language-wg. Herald added a subscriber: jdoerfert. Herald added a project: All. aaron.ballman requested review of this revision. Herald added a project: clang. Implicitly converting between