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

Overzealous -Wstrict-prototypes warnings on function definitions #90596

Open
jmarshall opened this issue Apr 30, 2024 · 6 comments
Open

Overzealous -Wstrict-prototypes warnings on function definitions #90596

jmarshall opened this issue Apr 30, 2024 · 6 comments
Labels
c clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer

Comments

@jmarshall
Copy link
Contributor

jmarshall commented Apr 30, 2024

Consider the following, foo.c:

int foo() {
    return 42;
}

Current clang (19.0.0git c3598b1) with -pedantic or -Wstrict-prototypes produces a warning for this code:

clang -c -Wstrict-prototypes foo.c 
foo.c:1:8: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
    1 | int foo() {
      |        ^
      |         void

The discussion in [RFC] Enabling -Wstrict-prototypes by default in C largely focusses on encouraging users to write declarations as int foo(void); but I am interested in the application of this warning to function definitions.

The diagnostic message above is somewhat confusing as this is a function definition, not just a function declaration. More importantly, I believe there is no confusion about a function definition with an empty parameter list. In particular C17 N2176 §6.7.6.3/14 says [similar text appears in earlier versions; emphasis added]

An identifier list declares only the identifiers of the parameters of the function. An empty list in a function declarator that is part of a definition of that function specifies that the function has no parameters. The empty list in a function declarator that is not part of a definition of that function specifies that no information about the number or types of the parameters is supplied. [Footnote 147: See “future language directions” (6.11.6).]

I would take this to mean that a function definition with an empty parameter list effectively provides a prototype, and therefore that the warning is spurious in this case. It seems unfortunate to encourage users to hypercorrect their code by writing such functions as int foo(void) { return 42; } when the void doesn't add anything in a function definition.

I also tried to find the chapter and verse of the standard saying that this scenario is “deprecated in all versions of C”. I guess this is based on §6.11.6, which says

The use of function declarators with empty parentheses (not prototype-format parameter type declarators) is an obsolescent feature.

Based on the two “…empty list…” sentences in the paragraph quoted above whose footnote refers to §6.11.6 and the fact that the obsolescent feature we're trying to get rid of in §6.11.6 is non-prototype declarations, I think this sentence intends only to refer to the latter and should perhaps read

The use of function declarators (that are not part of definitions of the respective functions) with empty parentheses (not prototype-format parameter type declarators) is an obsolescent feature.

in which case it would not provide motivation for emitting this warning on function definitions. (I did not see a defect report to this effect or otherwise relating to §6.11.6.)

@EugeneZelenko EugeneZelenko added c clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer and removed new issue labels Apr 30, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Apr 30, 2024

@llvm/issue-subscribers-c

Author: John Marshall (jmarshall)

Consider the following, _foo.c_:
int foo() {
    return 42;
}

Current clang (19.0.0git c3598b1) with -pedantic or -Wstrict-prototypes produces a warning for this code:

clang -c -Wstrict-prototypes foo.c 
foo.c:1:8: warning: a function declaration without a prototype is deprecated in all versions of C [-Wstrict-prototypes]
    1 | int foo() {
      |        ^
      |         void

The discussion in [RFC] Enabling -Wstrict-prototypes by default in C largely focusses on encouraging users to write declarations as int foo(void); but I am interested in the application of this warning to function definitions.

The diagnostic message above is somewhat confusing as this is a function definition, not just a function declaration. More importantly, I believe there is no confusion about a function definition with an empty parameter list. In particular C17 N2176 §6.7.6.3/14 says [similar text appears in earlier versions; emphasis added]

> An identifier list declares only the identifiers of the parameters of the function. An empty list in a function declarator that is part of a definition of that function specifies that the function has no parameters. The empty list in a function declarator that is not part of a definition of that function specifies that no information about the number or types of the parameters is supplied. [Footnote 147: See “future language directions” (6.11.6).]

I would take this to mean that a function definition with an empty parameter list effectively provides a prototype, and therefore that the warning is spurious in this case. It seems unfortunate to encourage users to hypercorrect their code to define such functions as int foo(void) { return 42; } when the void doesn't add anything in a function definition.

I also tried to find the chapter and verse of the standard saying that this scenario is “deprecated in all versions of C”. I guess this is based on §6.11.6, which says

> The use of function declarators with empty parentheses (not prototype-format parameter type declarators) is an obsolescent feature.

Based on the two “…empty list…” sentences in the paragraph quoted above whose footnote refers to §6.11.6 and the fact that the obsolescent feature we're trying to get rid of in §6.11.6 is non-prototype declarations, I think this sentence intends only to refer to the latter and should perhaps read

> The use of function declarators (that are not part of definitions of the respective functions) with empty parentheses (not prototype-format parameter type declarators) is an obsolescent feature.

in which case it would not provide motivation for emitting this warning on function definitions. (I did not see a defect report to this effect or otherwise relating to §6.11.6.)

@shafik
Copy link
Collaborator

shafik commented Apr 30, 2024

CC @AaronBallman @uecker

@MitalAshok
Copy link
Contributor

int foo() {
    return 42;
}
int bar(void) {
    return 42;
}

These still declare functions with two different types - int() and int(void).

It's still desireable for there to be a declaration with int(void) to prevent it from being compatible with every other function type, i.e.:

int foo() { return 42; }
int bar(void) { return 42; }
int f(int(int));
int main(void) {
    f(foo);  // Compatible
    // f(bar);  // Error
}

And §6.5.2.2 function call semantics are defined in terms of the type of the function. §6.5.2.2/8 specifically mentions function definitions without a prototype.

int main(void) {
    foo(1);  // Compiles but UB
    // bar(1);  // Error
}

So the (void) does add something.

@jmarshall
Copy link
Contributor Author

Thank you, that's very informative.

(Though now I barely understand what the “An empty list in a function declarator that is part of a definition of that function specifies that the function has no parameters” sentence is there for…)

@AaronBallman
Copy link
Collaborator

There are several cases to consider.

void foo() {}

does not provide a prototype for a function because it does not provide a parameter-type-list, but instead uses the optional identifier-list grammar production. See C17 6.7.6p1 and 6.9.1p7. However, by virtue of it being a definition, it does specify how many arguments may be passed to the function (zero).

void foo(); // #1
// ...
void foo() {} // #2

Neither of these declarations provide a prototype (per the same citations as above). Code between the declaration at (1) and the definition at (2) can call foo() with any number of arguments without diagnosing, but this is undefined behavior because the definition at (2) defines a function that accepts no arguments. Any calls providing arguments after the definition at (2) are diagnosed because the compiler now has complete information about the definition: https://godbolt.org/z/18vWso7d5

Finally, this gem:

void foo(void); // #1
// ...
void foo() {} // #2

The declaration at (1) provides a prototype, the definition at (2) does not provide a prototype, but gets one anyway because of composite type rules (see C17 6.2.7p3). Unlike the previous example, calls to foo() between (1) and (2) which pass an argument will get diagnosed because of the function prototype. Clang issues a -Wstrict-prototypes diagnostic at (2) because we form the prototype-less type before doing declaration merging (which is when we'd give the declaration a prototype) and we diagnose forming that type: https://godbolt.org/z/eq69rP1Wj

It's technically a false positive to diagnose in that case, but I don't think we'll ever fix that bug because of the difficulties it presents to do so. We want to still catch cases like (void (*)()) as a type cast because that forms a function type without a prototype (and similar for typedefs, etc). This is why we diagnose when forming the type itself. There's not a trivial way to retroactively say "nevermind, don't issue that diagnostic about the type because we happened to merge with a function type that does have a prototype"; we'd have to delay issuing diagnostics (potentially until the end of the TU) which can then mess with the order in which diagnostics are issued in a confusing way (e.g., you get warnings about those functions well after you would get other diagnostics related to those functions), and because this is an off-by-default diagnostic in Clang, it seems like the false positive in that case is better than the alternatives.

@brenns10
Copy link

Hello,

Finally, this gem:

void foo(void); // #1
// ...
void foo() {} // #2

I actually hit that exact issue here, and while the wording was initially confusing, I did get to the bottom of it. A nicer worded error would be nice though, since it led me to believe that clang wasn't seeing the initial declaration (which could have been possible due to #ifdef hell in the include files).

Just wanted to provide a data point on encountering this issue in the wild. The explanations here are very helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
c clang:diagnostics New/improved warning or error message in Clang, but not in clang-tidy or static analyzer
Projects
None yet
Development

No branches or pull requests

7 participants