Perl / perl5

🐪 The Perl programming language
https://dev.perl.org/perl5/
Other
1.91k stars 542 forks source link

pp_sys.c: 34d254cefc4 introduced new build-time warnings #17539

Closed jkeenan closed 4 years ago

jkeenan commented 4 years ago

Commit 34d254cefc4 on Feb 04 introduced new build-time warnings. Here are the new warnings found when running make on Linux, no threads, no debugging, etc.

cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c89 -O2 -Wall -Werror=pointer-arith -Wextra -Wc++-compat -Wwrite-strings -Werror=declaration-after-statement pp_sys.c
In file included from perl.h:5473:0,
                 from pp_sys.c:31:
pp_sys.c: In function ‘Perl_pp_getpriority’:
pp_sys.c:4628:23: warning: enum conversion when passing argument 1 of ‘getpriority’ is invalid in C++ [-Wc++-compat]
     SETi( getpriority(which, who) );
                       ^
pp.h:393:23: note: in definition of macro ‘TARGi’
         IV TARGi_iv = i;                                                \
                       ^
pp_sys.c:4628:5: note: in expansion of macro ‘SETi’
     SETi( getpriority(which, who) );
     ^
In file included from pp_sys.c:52:0:
/usr/include/x86_64-linux-gnu/sys/resource.h:93:12: note: expected ‘__priority_which_t {aka enum __priority_which}’ but argument is of type ‘int’
 extern int getpriority (__priority_which_t __which, id_t __who) __THROW;
            ^~~~~~~~~~~
In file included from perl.h:5473:0,
                 from pp_sys.c:31:
pp_sys.c: In function ‘Perl_pp_setpriority’:
pp_sys.c:4643:23: warning: enum conversion when passing argument 1 of ‘setpriority’ is invalid in C++ [-Wc++-compat]
     SETi( setpriority(which, who, niceval) >= 0 );
                       ^
pp.h:393:23: note: in definition of macro ‘TARGi’
         IV TARGi_iv = i;                                                \
                       ^
pp_sys.c:4643:5: note: in expansion of macro ‘SETi’
     SETi( setpriority(which, who, niceval) >= 0 );
     ^
In file included from pp_sys.c:52:0:
/usr/include/x86_64-linux-gnu/sys/resource.h:97:12: note: expected ‘__priority_which_t {aka enum __priority_which}’ but argument is of type ‘int’
 extern int setpriority (__priority_which_t __which, id_t __who, int __prio)
            ^~~~~~~~~~~
cc -c -DPERL_CORE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -std=c89 -O2 -Wall -Werror=pointer-arith -Wextra -Wc++-compat -Wwrite-strings -Werror=declaration-after-statement doop.c

In more compact form:

  {
    char   => 23,
    group  => "Wc++-compat",
    line   => 4628,
    source => "pp_sys.c",
    text   => "enum conversion when passing argument 1 of \xE2\x80\x98getpriority\xE2\x80\x99 is invalid in C++",
  },
  {
    char   => 23,
    group  => "Wc++-compat",
    line   => 4643,
    source => "pp_sys.c",
    text   => "enum conversion when passing argument 1 of \xE2\x80\x98setpriority\xE2\x80\x99 is invalid in C++",
  },

@ilmari , can you take a look?

Thank you very much. Jim Keenan

hvds commented 4 years ago

I get that here too, on my system it stems from this definition in /usr/include/x86_64-linux-gnu/sys/resource.h:

/* The X/Open standard defines that all the functions below must use
   `int' as the type for the first argument.  When we are compiling with
   GNU extensions we change this slightly to provide better error
   checking.  */
#if defined __USE_GNU && !defined __cplusplus
typedef enum __priority_which __priority_which_t;
#else
typedef int __priority_which_t;
#endif

So again the "XXX is invalid in C++" warning is irrelevant: were we compiling with C++, the headers would have declared the type as int rather than as the enum.

I reckon this would be both difficult and useless to work around, I feel the answer is to turn off -Wc++-compat and rely on real C++ builds to tell us if we really have real problems compiling under C++.

[update] Ah, the commit in question removed the work around we already had, and it really wasn't that difficult. However it is still useless.

Hugo

demerphq commented 4 years ago

On Sun, 9 Feb 2020 at 18:07, Hugo van der Sanden notifications@github.com wrote:

I get that here too, on my system it stems from this definition in /usr/include/x86_64-linux-gnu/sys/resource.h:

/ The X/Open standard defines that all the functions below must use `int' as the type for the first argument. When we are compiling with GNU extensions we change this slightly to provide better error checking. /

if defined __USE_GNU && !defined __cplusplus

typedef enum __priority_which __priority_which_t;

else

typedef int __priority_which_t;

endif

So again the "XXX is invalid in C++" warning is irrelevant: were we compiling with C++, the headers would have declared the type as int rather than as the enum.

I reckon this would be both difficult and useless to work around, I feel the answer is to turn off -Wc++-compat and rely on real C++ builds to tell us if we really have real problems compiling under C++.

I saw it too. I ignored it because I think Ilmari is working on it, I saw some comments about it on #p5p irc.

From Feb 07:

(11:35:37) ilmari: argh, -Wc++-compat doesn't know that [gs]etpriority() doesn't have the first argument as an enum under c++ (11:36:01) ilmari: so it's fine with actual g++, but with gcc it warns :(

cheers, Yves

-- perl -Mre=debug -e "/just|another|perl|hacker/"

ilmari commented 4 years ago

I think the best thing to do here is to revert my commit, and add a comment that explains the -Wc++compat issue to the next person who comes along with a broom.

I'll do that shortly, unless someone else has a better idea.

ilmari commented 4 years ago

I've now done that in commit 96d019622c8664693f6966a219cfdfd9c81ac025.

jkeenan commented 4 years ago

Confirming that the -Wc++-compat warnings are gone. thanks.