Page MenuHomePhabricator

Wrap C APIs with pragmas enforcing -Werror=strict-prototypes
ClosedPublic

Authored by dexonsmith on Nov 14 2019, 5:55 PM.

Details

Summary

Force -Werror=strict-prototypes so that C API tests fail to compile if
we add a non-prototype declaration. This should help avoid regressions
like bddecba4b333f7772029b4937d2c34f9f2fda6ca was fixing.

Diff Detail

Event Timeline

dexonsmith created this revision.Nov 14 2019, 5:55 PM
Herald added a project: Restricted Project. · View Herald Transcript

Note that we have C files including these headers in llvm/tools/llvm-c-test and clang/tools/c-index-test, and I tested that dropping a (void) to just () triggers the error when compiling those.

aaron.ballman added inline comments.Nov 15 2019, 11:51 AM
clang/include/clang-c/ExternC.h
19–20

I think this needs some extra compiler guards, unfortunately. I don't think MSVC supports _Pragma (IIRC, they rather oddly decided to call it __pragma instead). Do we want to restrict this pragma to only clang compilers? Do we need to do something to support GCC as well?

Adding guards for _MSC_VER.

dexonsmith marked 2 inline comments as done.Nov 15 2019, 1:11 PM
dexonsmith added inline comments.
clang/include/clang-c/ExternC.h
19–20

Thanks for pointing this out; guarded against _MSC_VER in the updated diff.

Supporting GCC as a follow-up sounds nice to me if someone is interested in sorting that out, but covering it in Clang like this is sufficient to get our bots to reject commits that regress.

dexonsmith marked an inline comment as done.

s/C_STRING_PROTOTYPES_/C_STRICT_PROTOTYPES_/ to fix some typos.

Bigcheese accepted this revision.Nov 19 2019, 11:24 AM

lgtm as long as other compilers don't warn on unknown pragmas by default.

This revision is now accepted and ready to land.Nov 19 2019, 11:24 AM

lgtm as long as other compilers don't warn on unknown pragmas by default.

godbolt.org says that GCC doesn't have -Wunknown-pragmas by default, but it is in -Wall. I'd better guard this more closely with __clang__.

dexonsmith closed this revision.Nov 19 2019, 1:19 PM

lgtm as long as other compilers don't warn on unknown pragmas by default.

godbolt.org says that GCC doesn't have -Wunknown-pragmas by default, but it is in -Wall. I'd better guard this more closely with __clang__.

Pushed as 8c48405069085a2c8b6b80816eda99e5dad31fc1 with this fixup.