LLVM Bugzilla is read-only and represents the historical archive of all LLVM issues filled before November 26, 2021. Use github to submit LLVM bugs

Bug 20796 - GCC's -Wstrict-prototypes warning not implemented in Clang
Summary: GCC's -Wstrict-prototypes warning not implemented in Clang
Status: RESOLVED FIXED
Alias: None
Product: clang
Classification: Unclassified
Component: Frontend (show other bugs)
Version: trunk
Hardware: PC Linux
: P normal
Assignee: Unassigned Clang Bugs
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-08-28 23:38 PDT by Mark Seaborn
Modified: 2016-12-07 04:53 PST (History)
7 users (show)

See Also:
Fixed By Commit(s):


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Mark Seaborn 2014-08-28 23:38:37 PDT
"-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
Comment 1 Mark Seaborn 2014-08-29 12:31:47 PDT
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).
Comment 2 Richard Smith 2014-08-29 16:20:44 PDT
You can get something similar from -Wmissing-prototypes:

<stdin>:1:18: warning: no previous prototype for function 'foo' [-Wmissing-prototypes]
void foo(); void foo() {}
                 ^
<stdin>: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
Comment 3 Mark Seaborn 2014-08-29 16:40:54 PDT
-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/).
Comment 4 Nico Weber 2014-08-30 19:05:59 PDT
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}}
Comment 5 Nico Weber 2014-08-30 19:11:15 PDT
Maybe this too:

void foo4(int a) {
  (void)(void(*)())foo0; // expected-warning{{function declaration isn’t a prototype}}
  (void)(void(*)(void))foo0;
}
Comment 6 Nico Weber 2014-08-30 19:43:00 PDT
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; {}
Comment 7 Nico Weber 2014-08-30 19:51:24 PDT
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?
Comment 8 Richard Smith 2014-08-31 00:24:33 PDT
(In reply to comment #4)
> 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."
Comment 9 Richard Smith 2014-08-31 00:28:49 PDT
(In reply to comment #7)
> 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?
Comment 10 Jonathon Mah 2015-10-14 17:41:28 PDT
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
Comment 11 Paul Titei 2016-01-25 10:15:25 PST
I've proposed a patch for the issue here: http://reviews.llvm.org/D16533
Comment 12 Alex Lorenz 2016-12-07 04:53:16 PST
I committed the fix in r288896.