GCC's -Wstrict-prototypes warning not implemented in Clang

[llvmbot]

Bugzilla Link	20796
Resolution	FIXED
Resolved on	Dec 07, 2016 04:53
Version	trunk
OS	Linux
Reporter	LLVM Bugzilla Contributor
CC	@hyp,@zmodem,@nico,@zygoloid

Extended Description

"-Wstrict-prototypes" appears to be a no-op in Clang.

$ cat strict_protos.c void foo() { }

$ gcc -c strict_protos.c -Wstrict-prototypes strict_protos.c:2:6: warning: function declaration isn’t a prototype [-Wstrict-prototypes] void foo() { ^

Using Chromium's build of Clang:

$ .../third_party/llvm-build/Release+Asserts/bin/clang -c strict_protos.c -Wstrict-prototypes $ .../third_party/llvm-build/Release+Asserts/bin/clang --version clang version 3.5.0 (trunk 209387) Target: x86_64-unknown-linux-gnu Thread model: posix

[hyp]

I committed the fix in r288896.

[llvmbot]

I've proposed a patch for the issue here: http://reviews.llvm.org/D16533

[llvmbot]

I believe this bug is very valid, and the warning should indeed be implemented and even enabled by default. This situation applies to the blocks extension as well, and many Objective-C codebases use () when they mean to use (void), thinking the former means no parameters.

Examples: ReactiveCocoa: https://github.com/ReactiveCocoa/ReactiveCocoa/blob/e16f47cf9cb568136ebd81430b24af274c3c27c7/ReactiveCocoa/Objective-C/RACStream.h#L171 Mixpanel: https://github.com/mixpanel/mixpanel-iphone/blob/bdd0f5a7f33ea0bfc4193f84ae2ed80258215bab/Mixpanel/Mixpanel.h#L559 FBSDKCoreKit: https://github.com/facebook/facebook-ios-sdk/blob/61b636f61d67337d59741177289cdfe5ec0e5667/FBSDKCoreKit/FBSDKCoreKit/FBSDKGraphRequestConnection.m#L709 AFNetworking: https://github.com/AFNetworking/AFNetworking/blob/fbd2bc8ac9353e6b9d2871b2a6a5cbc75b5ba841/AFNetworking/AFHTTPRequestOperation.m

[ec04fc15-fa35-46f2-80e1-5d271f2ef708]

Maybe we should just not warn on K&R functions for now (i.e. remove the !FTI.hasPrototype –I doubt anyone would miss that?) and go with that approach, with a fixme on a note about inserting void in the parameter list. How's that sound?

Maybe I'm missing something; could you not emit the warning in all cases, and only emit the fixit if there's no declared parameters?

[ec04fc15-fa35-46f2-80e1-5d271f2ef708]

Here's a bunch of test cases. Anything interesting that's missing? [...] void foo0() {} // expected-warning{{function declaration isn’t a prototype}}

Why should this warn? Per 6.7.5.3/12:

"An empty list in a function declarator that is part of a definition of that function specifies that the function has no parameters."

[nico]

If it weren't for that last case, this would do it:

Nicos-MacBook-Pro:clang thakis$ svn diff lib/Sema/SemaType.cpp Index: lib/Sema/SemaType.cpp

--- lib/Sema/SemaType.cpp (revision 216819) +++ lib/Sema/SemaType.cpp (working copy) @@ -2852,6 +2852,9 @@

   FunctionType::ExtInfo EI(getCCForDeclaratorChunk(S, D, FTI, chunkIndex));

• if ((!FTI.hasPrototype || !FTI.NumParams) && !LangOpts.CPlusPlus)
• S.Diag(DeclType.Loc, diag::warn_strict_prototypes);

• if (!FTI.NumParams && !FTI.isVariadic && !LangOpts.CPlusPlus) { // Simple void foo(), where the incoming T is the result type. T = Context.getFunctionNoProtoType(T, EI);

Maybe we should just not warn on K&R functions for now (i.e. remove the !FTI.hasPrototype –I doubt anyone would miss that?) and go with that approach, with a fixme on a note about inserting void in the parameter list. How's that sound?

[nico]

The last one is annoying:

// RUN: %clang_cc1 -fsyntax-only -Wstrict-prototypes -verify %s

void foo0(); // expected-warning{{function declaration isn’t a prototype}} void foo1(void);

void foo0() {} // expected-warning{{function declaration isn’t a prototype}} void foo1(void) {}

void (foo2)(); // expected-warning{{function declaration isn’t a prototype}} void (foo3)(void);

void foo4(int a) { (void)(void()())foo0; // expected-warning{{function declaration isn’t a prototype}} (void)(void()(void))foo0; }

// FIXME: test this doesn't get a note with a fixme, while the rest does void foo5(); // expected-warning{{function declaration isn’t a prototype}} void foo5(p, p2) void *p; {} // expected-warning{{function declaration isn’t a prototype}}

void foo6(int p, int p2); void foo6(p, p2) int p; int p2; {}

[nico]

Maybe this too:

void foo4(int a) { (void)(void()())foo0; // expected-warning{{function declaration isn’t a prototype}} (void)(void()(void))foo0; }

[nico]

Here's a bunch of test cases. Anything interesting that's missing?

$ cat test/Sema/warn-strict-prototypes.c // RUN: %clang_cc1 -fsyntax-only -Wstrict-prototypes -verify %s

void foo0(); // expected-warning{{function declaration isn’t a prototype}} void foo1(void);

void foo0() {} // expected-warning{{function declaration isn’t a prototype}} void foo1(void) {}

void (foo2)(); // expected-warning{{function declaration isn’t a prototype}} void (foo3)(void);

void foo4(int a) {}

// FIXME: test this doesn't get a note with a fixme, while the rest does int foo5(p, p2); // expected-warning{{function declaration isn’t a prototype}} int foo5(p, p2) void *p; {} // expected-warning{{function declaration isn’t a prototype}}

[llvmbot]

-Wmissing-prototypes warns about things that -Wstrict-prototypes doesn't, that I'm not interested in, like this:

void bar(void) { } / Without declaration before /

Also, -Wmissing-prototypes fails to warn about things that are problematic, such as this:

void (*func_ptr)();

BTW, I filed this issue because thakis@chromium.org asked me to (in https://codereview.chromium.org/508823007/).

[ec04fc15-fa35-46f2-80e1-5d271f2ef708]

You can get something similar from -Wmissing-prototypes:

:1:18: warning: no previous prototype for function 'foo' [-Wmissing-prototypes] void foo(); void foo() {} ^ :1:6: note: this declaration is not a prototype; add 'void' to make it a prototype for a zero-parameter function void foo(); void foo() {} ^ void

[llvmbot]

A better example would have been a function declaration rather than a definition:

void foo(); // foo()'s arguments are unspecified

because, in C (but not C++), it is semantically different from:

void foo(void); // foo() takes no arguments

I enabled this warning for NaCl code because it catches the occasional bug where functions are called with the wrong number of arguments (see https://code.google.com/p/nativeclient/issues/detail?id=3114).