zlib icon indicating copy to clipboard operation
zlib copied to clipboard

Functions without prorotypes cause -Wstrict-prototypes warnings

Open MaskRay opened this issue 2 years ago • 17 comments

As of Clang 14.0.0, compiling zlib with -Wstrict-prototypes is warning free.

The next Clang release 15.0.0 will include https://reviews.llvm.org/D122895 which has improved diagnostics. Notably: the new -Wdeprecated-non-prototype is enabled by default which warns many declarations in zlib:

% make CC=/tmp/RelA/bin/clang -j 30                                                                                                                                                                                                                                                         
/tmp/RelA/bin/clang -O3 -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN -I. -I../../ -c -o example.o ../../test/example.c                                                                                                                                                                             
/tmp/RelA/bin/clang -O3 -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN -include zconf.h -c -o adler32.o ../../adler32.c                                                                                                                                                                              
...                                                                                                                                                   
../../compress.c:22:13: warning: a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]                                                                                                                     
int ZEXPORT compress2 (dest, destLen, source, sourceLen, level)                                                                                                                                                                                                                             
            ^                                                                                                                                                                                                                                                                               
../../compress.c:68:13: warning: a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]                                                                                                                     
int ZEXPORT compress (dest, destLen, source, sourceLen)                                                                                                                                                                                                                                     
            ^                                                                                                                                                                                                                                                                               
../../compress.c:81:15: warning: a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]                                                                                                                     
uLong ZEXPORT compressBound (sourceLen)                                                                                                                                                                                                                                                     
              ^                                                                                                                                                                                                                                                                             
../../uncompr.c:27:13: warning: a function declaration without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]                                                                                                                      
int ZEXPORT uncompress2 (dest, destLen, source, sourceLen)                                       

[...]

There are also a few -Wstrict-prototypes -Wno-deprecated-non-prototype diagnostics.

Note: -pedantic will imply -Wstrict-prototypes.


As of GCC 11, its -Wstrict-prototypes warns about some declarations (a small subset of Clang's list):

../../crc32.c:117:16: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
  117 | local z_word_t byte_swap(word)
      |                ^~~~~~~~~
gcc -O3 -fPIC -D_LARGEFILE64_SOURCE=1 -DHAVE_HIDDEN -include zconf.h -DPIC -c -o objs/gzclose.o ../../gzclose.c
../../crc32.c:717:15: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
  717 | local z_crc_t crc_word(data)
      |               ^~~~~~~~
../../crc32.c:726:16: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
  726 | local z_word_t crc_word_big(data)
      |                ^~~~~~~~~~~~
../../crc32.c:1093:15: warning: function declaration isn’t a prototype [-Wstrict-prototypes]
 1093 | uLong ZEXPORT crc32_combine_gen64(len2)
      |               ^~~~~~~~~~~~~~~~~~~

MaskRay avatar Apr 09 '22 18:04 MaskRay

I "fixed" those warnings in a local copy of zlib (in wasm/emsdk those are errors). Fixing all warnings that gcc gave me with -Werror=missing-parameter-type also fixed the ones from clang15. Is this something you could use or am I missing the point?

Markus87 avatar May 11 '22 10:05 Markus87

@MaskRay @MaskRay Maybe just these 4 function's decarlations are missing. Please try with my PR #667

fredgan avatar Jun 23 '22 08:06 fredgan

#667 is an improvement but there are still many -Wdeprecated-non-prototype with recent clang and the just-released clang 15.0.0.

As the tip of the iceberg:

:1152:12: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
../../trees.c:1169:12: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
local void bi_windup(s)
           ^
../../trees.c:1169:12: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
../../inflate.c:1373:13: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
int ZEXPORT inflateGetHeader(strm, head)
            ^
../../inflate.c:1401:16: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
local unsigned syncsearch(have, buf, len)
               ^

MaskRay avatar Sep 13 '22 17:09 MaskRay

@Markus87 https://github.com/Markus87/zlib master branch fixes all Clang warnings!

@madler It would be great to merge that change:)

I understand that there may be straggling systems which do not support ANSI function declarations but some script (converse of zlib2ansi) can help them or they can stick with older releases. The current state actively impacts all modern systems packaging zlib.

MaskRay avatar Sep 13 '22 17:09 MaskRay

I do not plan to remove the pre-ISO C compatibility at this time. I would recommend simply turning off that warning.

By the way, every function does have an ISO C prototype that precedes it, if it is an ISO C compiler. The complaint is not even correct.

madler avatar Oct 06 '22 23:10 madler

If and when C2X is final and removes K&R function definitions, then I will remove them from zlib. I will leave this issue open until C2X is final.

madler avatar Oct 11 '22 19:10 madler

@AaronBallman may comment on the state of K&R function definitions.

MaskRay avatar Oct 12 '22 03:10 MaskRay

@AaronBallman may comment on the state of K&R function definitions.

They were removed for C2x (WG14 N2841 and WG14 N2432) which is just coming up to the Committee Draft balloting stage (basically, one step before voting the standard out). Unless there is a national body comment which convinces WG14 to revert the changes, this will be in C2x. Given that K&R C functions without prototypes have been obsolescent in C since C was standardized by ISO (so 30+ years of "not a best practice"), I would be surprised if that would happen, but it is still a possibility. We should know for sure by the end of 2023 at the absolute latest.

By the way, every function does have an ISO C prototype that precedes it, if it is an ISO C compiler. The complaint is not even correct.

That doesn't mean the things being diagnosed aren't being declared without a prototype. That syntax is broken by C2x, so the preceding prototypes don't matter. The diagnostic looks correct to me from the places I spot-checked, such as https://github.com/madler/zlib/blob/master/uncompr.c#L27 and https://github.com/madler/zlib/blob/master/crc32.c#L117 -- that code is obsolescent C and won't compile in C2x.

AaronBallman avatar Oct 12 '22 11:10 AaronBallman

Yes, I understand that the proposed change is to make the K&R syntax obsolete. However the text of the warning is incorrect, since those are not function declarations without a prototype. The warning should instead say something like function definitions with identifier lists are not permitted in C2X.

madler avatar Oct 12 '22 14:10 madler

Yes, I understand that the proposed change is to make the K&R syntax obsolete.

K&R C functions have always been obsolete (see C89 3.9.5p1: "The use of function definitions with separate parameter identifier and declaration lists (not prototype-format parameter type and identifier declarators) is an obsolescent feature."). The change to C2x (not just proposed but already accepted by the committee) is to reuse that design space.

However the text of the warning is incorrect, since those are not function declarations without a prototype. The warning should instead say something like function definitions with identifier lists are not permitted in C2X.

Please see C89 3.1.2.1p1: "(A function prototype is a declaration of a function that declares the types of its parameters.)" (The same text is in C17 6.2.1p2, which might be easier to find a copy of than C89.) You can also see this in the function call expression wording (C17 6.5.2.2p8) which says: "No other conversions are performed implicitly; in particular, the number and types of arguments are not compared with those of the parameters in a function definition that does not include a function prototype declarator."

The functions I cited do not use a function prototype declarator, they use an identifier list and a separate declaration list.

AaronBallman avatar Oct 12 '22 15:10 AaronBallman

They have been deprecated, not obsolete, where by obsolete, I mean they would simply not be allowed, resulting in an error, not a warning. Maybe there's a better word. Perhaps forbidden.

Those functions do in fact each have a prototype using the ISO format that precede the K&R declaration. So they are not functions without a prototype.

By the way, this is a question just for my curiosity in case you know. I get that the use of declarations after a function parameter list will be forbidden. However they talk about "reusing" that design space. What would they reuse it for?

madler avatar Oct 12 '22 15:10 madler

Those functions do in fact each have a prototype using the ISO format that precede the K&R declaration. So they are not functions without a prototype.

[citation needed]

(I think you're maybe talking about type composition rules, which is not where the diagnostic is coming from. There's literally no way to spell void f(a) such that it has a prototype. The composite function pointer type you get out of such a declaration may have a prototype though. e.g., void f(int a); void g(); typeof(rand() ? f : g) what_am_i; yields a pointer to function declaration with a prototype.)

By the way, this is a question just for my curiosity in case you know. I get that the use of declarations after a function parameter list will be forbidden. However they talk about "reusing" that design space. What would they reuse it for?

void f(); used to mean "callable with zero or more arguments" and now it means "callable with exactly zero arguments" as it does in C++. That was a significant source of bugs, as you might imagine.

AaronBallman avatar Oct 12 '22 15:10 AaronBallman

int f(int x);
int f(x)
int x;
{
    return x * x;
}

f() has a prototype, and the function defintion uses the K&R syntax. All of the functions in zlib are like this.

madler avatar Oct 12 '22 15:10 madler

I don't know that I have more to add to the conversation to try to clarify the situation, so I'll bow out at this point.

AaronBallman avatar Oct 12 '22 17:10 AaronBallman

As another compiler engineer, I can perhaps clarify:

I think the confusion here is the difference between 'function' and 'function declaration' (and lesser-so, function definition). I believe the diagnostic is correct, since it is talking about the latter.

To Clarify: A "Function" is specified by 1 or more 'function declarations'. In @madler 's example, both lines 1 and lines 2 are separate 'function declarations' (the second is a function declaration that is also a function definition) that specify a single 'function'.

In @madler 's example, that function DOES have a prototype available. However, the diagnostic is talking about the 'function declaration', of which line 2 DEFINITELY does not have a valid prototype.

IMO, if you are willing to have line#1 in a program (meaning you are using a compiler that supports ANSI declarations), I don't see a reason to not just specify the definition the same way. Any compilers that can parse the first declaration (basically all compilers that implement "The C Programming Language" 2nd Edition by K&R) would ALSO be able to parse the 2nd if given a prototype.

However, a compiler that parses the 1st declaration doesn't necessarily parse the 2nd.

As far as:

I do not plan to remove the pre-ISO C compatibility at this time.

You've already done so, no pre-ANSI C (what I suspect you meant?) compiler can compile zlib as it sits.

erichkeane avatar Oct 12 '22 18:10 erichkeane

IMO, if you are willing to have line#1 in a program (meaning you are using a compiler that supports ANSI declarations), I don't see a reason to not just specify the definition the same way.

For clarity in that discussion, just to demonstrate that you can in fact have a K&R defined function with a prototype, I left out the macro that zlib uses. The reason that both styles are there in zlib is to be able to use prototypes with ISO C compilers, but have the code still be compatible with non-ISO C compilers, both of which were in common use at the time (mid 90's). Simplifying what's in zlib:

#ifdef __STDC__
#  define OF(args) args
#else
#  define OF(args) ()
#endif

int f OF((int x));
int f(x)
int x;
{
    return x * x;
}

You've already done so, no pre-ISO C compiler can compile zlib as it sits.

It can and did. See above.

madler avatar Oct 12 '22 18:10 madler

Ah, I see! I was missing the context of that macro.

I can understand your reluctance to make the change, though this is likely going to have to be a change made in the reasonably-near-future (likely 'months') thanks to compilers removing this (as Clang is preparing to do).

erichkeane avatar Oct 12 '22 19:10 erichkeane