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

Functions without prorotypes cause -Wstrict-prototypes warnings #633

Closed
MaskRay opened this issue Apr 9, 2022 · 23 comments
Closed

Functions without prorotypes cause -Wstrict-prototypes warnings #633

MaskRay opened this issue Apr 9, 2022 · 23 comments

Comments

@MaskRay
Copy link

MaskRay commented Apr 9, 2022

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 MaskRay changed the title K&R-style argument list causes -Wdeprecated-non-prototype for clang 15.0 Functions without prorotypes cause -Wstrict-prototypes warnings Apr 9, 2022
lambdageek added a commit to lambdageek/runtime that referenced this issue Apr 22, 2022
aarongable pushed a commit to chromium/chromium that referenced this issue May 2, 2022
Fix an instance in libevent.

Sink -Wno-deprecated-non-prototype into zlib, it's the only remaining library where this warning fires. (upstream bug madler/zlib#633)

Bug: 1314867
Change-Id: I2547ba1b358ab90ec6dece4f0879b2ebe6f59820
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615937
Reviewed-by: Nico Weber <[email protected]>
Commit-Queue: Arthur Eubanks <[email protected]>
Cr-Commit-Position: refs/heads/main@{#998593}
zeng450026937 pushed a commit to zeng450026937/build that referenced this issue May 4, 2022
Fix an instance in libevent.

Sink -Wno-deprecated-non-prototype into zlib, it's the only remaining library where this warning fires. (upstream bug madler/zlib#633)

Bug: 1314867
Change-Id: I2547ba1b358ab90ec6dece4f0879b2ebe6f59820
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615937
Reviewed-by: Nico Weber <[email protected]>
Commit-Queue: Arthur Eubanks <[email protected]>
Cr-Commit-Position: refs/heads/main@{#998593}
NOKEYCHECK=True
GitOrigin-RevId: cbba1d4d16d3dfeb51421749cafa34c2666f8b26
zeng450026937 pushed a commit to zeng450026937/base that referenced this issue May 4, 2022
Fix an instance in libevent.

Sink -Wno-deprecated-non-prototype into zlib, it's the only remaining library where this warning fires. (upstream bug madler/zlib#633)

Bug: 1314867
Change-Id: I2547ba1b358ab90ec6dece4f0879b2ebe6f59820
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615937
Reviewed-by: Nico Weber <[email protected]>
Commit-Queue: Arthur Eubanks <[email protected]>
Cr-Commit-Position: refs/heads/main@{#998593}
NOKEYCHECK=True
GitOrigin-RevId: cbba1d4d16d3dfeb51421749cafa34c2666f8b26
github-actions bot pushed a commit to xmake-mirror/chromium_zlib that referenced this issue May 9, 2022
Fix an instance in libevent.

Sink -Wno-deprecated-non-prototype into zlib, it's the only remaining library where this warning fires. (upstream bug madler/zlib#633)

Bug: 1314867
Change-Id: I2547ba1b358ab90ec6dece4f0879b2ebe6f59820
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3615937
Reviewed-by: Nico Weber <[email protected]>
Commit-Queue: Arthur Eubanks <[email protected]>
Cr-Commit-Position: refs/heads/main@{#998593}
NOKEYCHECK=True
GitOrigin-RevId: cbba1d4d16d3dfeb51421749cafa34c2666f8b26
@Markus87
Copy link

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?

@fredgan
Copy link

fredgan commented Jun 23, 2022

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

@MaskRay
Copy link
Author

MaskRay commented Sep 13, 2022

#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
Copy link
Author

MaskRay commented Sep 13, 2022

@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.

jkotas added a commit to jkotas/runtime that referenced this issue Oct 1, 2022
jkotas added a commit to dotnet/runtime that referenced this issue Oct 1, 2022
* reinstate strict-prototype warning

* More fixes

* Need to keep the warning disabled for zlib

madler/zlib#633

Co-authored-by: Jan Kotas <[email protected]>
@madler
Copy link
Owner

madler commented Oct 6, 2022

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
Copy link
Owner

madler commented Oct 11, 2022

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.

@MaskRay
Copy link
Author

MaskRay commented Oct 12, 2022

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

@AaronBallman
Copy link

@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.

@madler
Copy link
Owner

madler commented Oct 12, 2022

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.

@AaronBallman
Copy link

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.

@madler
Copy link
Owner

madler commented Oct 12, 2022

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?

@AaronBallman
Copy link

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.

@madler
Copy link
Owner

madler commented Oct 12, 2022

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.

@AaronBallman
Copy link

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.

@erichkeane
Copy link

erichkeane commented Oct 12, 2022

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.

dridi added a commit to varnishcache/varnish-cache that referenced this issue Jan 30, 2023
As I suspected, the -Werror setup for libvgz C flags were needed to
properly discard them for suncc. Both warnings are conditionally not
turned into errors, so -Wno-unknown-warning-option should no longer
be needed.

Refs madler/zlib#633
@MaskRay
Copy link
Author

MaskRay commented Feb 6, 2023

A few months passed and many projects have tried working around the issue (as can be seen from pages referencing this URL. I know some projects moving to a fork to not deal with this issue). I hope that we will have a fix in the upstream.

For modern compilers, the declaration looks like

int f (int x);
int f(x)
int x;
{
    return x * x;
}
% clang -fsyntax-only a.c  # warning since 15.0.0
a.c:2:5: warning: a function definition without a prototype is deprecated in all versions of C and is not supported in C2x [-Wdeprecated-non-prototype]
int f(x)
    ^
1 warning generated.
% gcc -fsyntax-only -Wall -Wextra -Wstrict-prototypes a.c   # no warning as of 12.2

To both fix the warning with Clang and support non-ISO C compilers, we can duplicate the prototype:

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

// zlib.h
int f OF((int x));

// .c
#ifdef __STDC__
int f(int x)
#else
int f(x)
int x;
#endif
{
    return x * x;
}

The above is tedious. I'd suggest that we just drop the legacy syntax. @madler you may use a preprocessor to convert the following to the above, if the C compiler is a non-ISO one.

// .c
int f(int x)
{
    return x * x;
}

bsdjhb pushed a commit to bsdjhb/cheribsd that referenced this issue Feb 7, 2023
…OTYPE

The zlib project has issue madler/zlib#633 to
document its continued use of old K&R-style function definitions.

Suggested by:		delphij@
Sponsored by:		Netflix
moz-v2v-gh pushed a commit to mozilla/gecko-dev that referenced this issue Feb 17, 2023
zlib still uses K&R-style function declarations. As of clang 15, these
are warned about by default, as they've been removed in the latest draft
of C2x; this results in thousands(?) of lines of warnings when building
zlib in-tree.

An issue has been filed at madler/zlib#633,
but the author has stated that they are unwilling to change this until
C2x has actually been published.

For now, just silence those warnings when building zlib.

Differential Revision: https://phabricator.services.mozilla.com/D169842
jamienicol pushed a commit to jamienicol/gecko that referenced this issue Feb 20, 2023
zlib still uses K&R-style function declarations. As of clang 15, these
are warned about by default, as they've been removed in the latest draft
of C2x; this results in thousands(?) of lines of warnings when building
zlib in-tree.

An issue has been filed at madler/zlib#633,
but the author has stated that they are unwilling to change this until
C2x has actually been published.

For now, just silence those warnings when building zlib.

Differential Revision: https://phabricator.services.mozilla.com/D169842
romainguy added a commit to google/filament that referenced this issue Apr 4, 2023
zlib will fix this issue later (see madler/zlib#633).
For now we will just turn off the warning.
romainguy added a commit to google/filament that referenced this issue Apr 4, 2023
* Suppress numerous libz warnings

zlib will fix this issue later (see madler/zlib#633).
For now we will just turn off the warning.

* Fix Windows build
@madler
Copy link
Owner

madler commented Apr 14, 2023

A few months passed and many projects have tried working around the issue

Really? How much did they try? It couldn't be simpler to work around this issue. As I suggested above, simply turn off the warning: -Wno-deprecated-non-prototype.

I have pushed a commit on the develop branch that adds this work around to configure. 5799c14 The next version of zlib will remove the K&R declarations entirely.

@fredgan
Copy link

fredgan commented Apr 14, 2023

Looking forward to the new zlib. Do you have the plan when the new version will be released? @madler

@madler
Copy link
Owner

madler commented Apr 14, 2023

Soon.

@MaskRay
Copy link
Author

MaskRay commented Aug 16, 2023

Neither make CC=clang CFLAGS=-Wstrict-prototypes (clang main branch) nor make CC=gcc CFLAGS=-Wstrict-prototypes (gcc 12.2.0) issues any warning on the develop branch. Commits in April 2023 like e9d5486 are relevant. Thank you!

Assuming that the next release is v1.2.14, v1.2.14 will be the first release fixing these warnings.

edit: There is no v1.2.14. v1.3 (09155ea) contains the fixes.

@Neustradamus
Copy link

Merged commit about K&R removal:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants