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

C++ builds fail with modern C++ compilers #21073

Closed
tonycoz opened this issue May 4, 2023 · 10 comments · Fixed by #21096
Closed

C++ builds fail with modern C++ compilers #21073

tonycoz opened this issue May 4, 2023 · 10 comments · Fixed by #21096

Comments

@tonycoz
Copy link
Contributor

tonycoz commented May 4, 2023

Module: Compress::Raw::Bzip2

Other modules may also be a problem.

Description

With a modern C++ compiler supporting and defaulting to C++ 17, the register storage class modifier is no longer supported, which causes the build to fail:

 c++ -DNO_MATHOMS -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings -O2   -DVERSION=\"2.204\" -DXS_VERSION=\"2.204\" -fPIC "-I../.."  -DBZ_NO_STDIO  compress.c
compress.c:190:13: error: ISO C++17 does not allow 'register' storage class specifier [-Wregister]
            register UChar  rtmp;
            ^~~~~~~~~

Note that at least one C99 construct we use isn't valid in C++:

In file included from globals.c:25:
In file included from ./perl.h:6191:
./opcode.h:1004:2: warning: array designators are a C99 extension [-Wc99-designator]
        [OP_NULL]               = Perl_pp_null,
        ^~~~~~~~~

(designated struct initialization is supported in C++20 but not arrays)

Steps to Reproduce

# requires clang-16, installed here from the llvm deb archive
./Configure -des -Dusedevel -Dcc=clang++-16 -Accflags='-g -fno-omit-frame-pointer -fsanitize=address -fno-common -x c++' -Aldflags=-fsanitize=address -Accflags=-DNO_MATHOMS && make

Expected behavior

Successful build.

Workaround

Build with -std=c++11, which changes the message to a warning:

clang++-16 -c  -I./zlib-src -g -fno-omit-frame-pointer -fsanitize=address -fno-common -x c++ -std=c++11 -DNO_MATHOMS -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wno-declaration-after-statement -Wc++-compat -Wwrite-strings -O2   -DVERSION=\"2.204\" -DXS_VERSION=\"2.204\" -fPIC "-I../.."  -DNO_VIZ -DZ_SOLO   -DZ_PREFIX  -DGZIP_OS_CODE=3  -DPerl_crz_BUILD_ZLIB=1 deflate.c
deflate.c:1282:5: warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]
    register Bytef *scan = s->window + s->strstart; /* current string */
    ^~~~~~~~~

Perl configuration

# from miniperl
Summary of my perl5 (revision 5 version 37 subversion 12) configuration:
  Commit id: 6bbe21a508a82ef4a3849bd69a435b3cf8d5bf40
  Platform:
    osname=linux
    osvers=5.10.0-21-amd64
    archname=x86_64-linux
    uname='linux venus 5.10.0-21-amd64 #1 smp debian 5.10.162-1 (2023-01-21) x86_64 gnulinux '
    config_args='-des -Dusedevel -Dcc=clang++-16 -Accflags=-g -fno-omit-frame-pointer -fsanitize=address -fno-common -x c++ -Aldflags=-fsanitize=address -Accflags=-DNO_MATHOMS'
    hint=recommended
    useposix=true
    d_sigaction=define
    useithreads=undef
    usemultiplicity=undef
    use64bitint=define
    use64bitall=define
    uselongdouble=undef
    usemymalloc=n
    default_inc_excludes_dot=define
  Compiler:
    cc='clang++-16'
    ccflags ='-g -fno-omit-frame-pointer -fsanitize=address -fno-common -x c++ -DNO_MATHOMS -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64'
    optimize='-O2'
    cppflags='-g -fno-omit-frame-pointer -fsanitize=address -fno-common -x c++ -DNO_MATHOMS -I/usr/local/include'
    ccversion=''
    gccversion='Debian Clang 16.0.3 (++20230502062952+da3cd333bea5-1~exp1~20230502183046.82)'
    gccosandvers=''
    intsize=4
    longsize=8
    ptrsize=8
    doublesize=8
    byteorder=12345678
    doublekind=3
    d_longlong=define
    longlongsize=8
    d_longdbl=define
    longdblsize=16
    longdblkind=3
    ivtype='long'
    ivsize=8
    nvtype='double'
    nvsize=8
    Off_t='off_t'
    lseeksize=8
    alignbytes=8
    prototype=define
  Linker and Libraries:
    ld='clang++-16'
    ldflags =' -fsanitize=address -L/usr/local/lib'
    libpth=/usr/lib/llvm-16/lib/clang/16/lib /usr/local/lib /usr/lib/x86_64-linux-gnu /usr/lib /usr/lib64
    libs=-lpthread -lgdbm -ldl -lm -lcrypt -lutil -lc -lgdbm_compat
    perllibs=-lpthread -ldl -lm -lcrypt -lutil -lc
    libc=libc-2.31.so
    so=so
    useshrplib=false
    libperl=libperl.a
    gnulibc_version='2.31'
  Dynamic Linking:
    dlsrc=dl_dlopen.xs
    dlext=so
    d_dlsymun=undef
    ccdlflags='-Wl,-E'
    cccdlflags='-fPIC'
    lddlflags='-shared -O2 -L/usr/local/lib'

Characteristics of this binary (from libperl): 
  Compile-time options:
    HAS_LONG_DOUBLE
    HAS_STRTOLD
    HAS_TIMES
    NO_MATHOMS
    PERLIO_LAYERS
    PERL_COPY_ON_WRITE
    PERL_DONT_CREATE_GVSV
    PERL_EXTERNAL_GLOB
    PERL_HASH_FUNC_SIPHASH13
    PERL_HASH_USE_SBOX32
    PERL_IS_MINIPERL
    PERL_MALLOC_WRAP
    PERL_OP_PARENT
    PERL_PRESERVE_IVUV
    PERL_USE_DEVEL
    PERL_USE_SAFE_PUTENV
    USE_64_BIT_ALL
    USE_64_BIT_INT
    USE_LARGE_FILES
    USE_LOCALE
    USE_LOCALE_COLLATE
    USE_LOCALE_CTYPE
    USE_LOCALE_NUMERIC
    USE_LOCALE_TIME
    USE_PERLIO
    USE_PERL_ATOF
    USE_SITECUSTOMIZE
  Built under linux
  Compiled at May  4 2023 10:48:51
  %ENV:
    PERLBREW_BASHRC_VERSION="0.43"
    PERLBREW_HOME="/home/tony/.perlbrew"
    PERLBREW_MANPATH=""
    PERLBREW_PATH="/home/tony/perl5/perlbrew/bin"
    PERLBREW_ROOT="/home/tony/perl5/perlbrew"
    PERLBREW_VERSION="0.67"
  @INC:
    /home/tony/dev/perl/git/perl6/cpan/AutoLoader/lib
    /home/tony/dev/perl/git/perl6/dist/Carp/lib
    /home/tony/dev/perl/git/perl6/dist/PathTools
    /home/tony/dev/perl/git/perl6/dist/PathTools/lib
    /home/tony/dev/perl/git/perl6/cpan/ExtUtils-Install/lib
    /home/tony/dev/perl/git/perl6/cpan/ExtUtils-MakeMaker/lib
    /home/tony/dev/perl/git/perl6/cpan/ExtUtils-Manifest/lib
    /home/tony/dev/perl/git/perl6/cpan/File-Path/lib
    /home/tony/dev/perl/git/perl6/ext/re
    /home/tony/dev/perl/git/perl6/dist/Term-ReadLine/lib
    /home/tony/dev/perl/git/perl6/dist/Exporter/lib
    /home/tony/dev/perl/git/perl6/ext/File-Find/lib
    /home/tony/dev/perl/git/perl6/cpan/Text-Tabs/lib
    /home/tony/dev/perl/git/perl6/dist/constant/lib
    /home/tony/dev/perl/git/perl6/cpan/version/lib
    /home/tony/dev/perl/git/perl6/cpan/Getopt-Long/lib
    /home/tony/dev/perl/git/perl6/cpan/Text-ParseWords/lib
    /home/tony/dev/perl/git/perl6/cpan/ExtUtils-PL2Bat/lib
    /home/tony/dev/perl/git/perl6/lib
@tonycoz
Copy link
Contributor Author

tonycoz commented May 4, 2023

While we could fix this particular failure by asking upstreams not to use register, it doesn't fix the more general problem that C isn't C++: the designated array initializer means we can't perform similar testing with C++ compilers don't provide this extension.

I remember two reasons that have been mentioned for testing with C++:

  1. testing that our headers are C++ compatible
  2. stricter type checking from C++

I think 1. is useful, but rather than requiring that the whole of perl build with C++, I think it would be better to limit that to the headers, this reduces the load on our upstreams and lets us use C features in most code without the worry that they aren't supported in C++.

I think the stricter type checking is close to nonsense, the only stricter type check I'm aware of for our purposes is the requirement to cast void * to other types, the other types of strictness only apply when using C++ style casts.

@khwilliamson
Copy link
Contributor

I do my development using g++. This is a habit from when C++ was new, and was billed as a better C, even if one didn't use the OO capabilities, and the C++ compiler was merely a preprocessor that translated the source to C (Cfront). Since then the languages have diverged more. I don't know the current state of the advantages of C++ compilers over C, if any.

@tonycoz
Copy link
Contributor Author

tonycoz commented May 8, 2023

and was billed as a better C, even if one didn't use the OO capabilities

This can be true, though I think it mostly depended on using C++ features (like new and delete).

Some of the problem with C vs C++ is some compilers only warn on code the C standard considers ill-formed code, like passing an int * when a STRLEN * is required, while I recall the Turbo C compiler I started with would throw an error on such code (but it's been a long time.)

gcc warns on such code, g++ throws an error, even though it's ill-formed in both languages.

But in terms of progress on this, I can see us making a local change to remove register and submit it upstream, but it's not going to fix the other issue.

The win32 Makefiles' USE_CPLUSPLUS support appears to be broken, but if I manually supply -TP I get:

        cl -c -I.. -nologo -GF -W3 -MD -I..\lib\CORE -I.\include -I. -I.. -DWIN32 -D_CONSOLE -DNO_STRICT -DWIN64 -D_CRT_SECURE_NO_DEPRECATE -D_CRT_NONSTDC_NO_DEPRECATE -D_WINSOCK_DEPRECATED_NO_WARNINGS -DPERLDLL -DPERL_CORE   -O1 -Zi -GL -fp:precise -TP -DPERL_TEXTMODE_SCRIPTS -DMULTIPLICITY -DPERL_IMPLICIT_SYS -Fo..\globals.obj ..\globals.c
globals.c
C:\Users\Tony\dev\perl\git\perl\opcode.h(571): error C3480: 'OP_NULL': a lambda capture must be a variable from an enclosing function scope
C:\Users\Tony\dev\perl\git\perl\opcode.h(571): error C3260: '=': skipping unexpected token(s) before lambda body
C:\Users\Tony\dev\perl\git\perl\opcode.h(996): warning C4467: usage of ATL attributes is deprecated
C:\Users\Tony\dev\perl\git\perl\opcode.h(996): error C2337: 'OP_NULL': attribute not found
C:\Users\Tony\dev\perl\git\perl\opcode.h(573): error C2059: syntax error: '='
C:\Users\Tony\dev\perl\git\perl\opcode.h(996): error C2059: syntax error: ';'
C:\Users\Tony\dev\perl\git\perl\opcode.h(1425): error C3480: 'OP_NULL': a lambda capture must be a variable from an enclosing function scope
C:\Users\Tony\dev\perl\git\perl\opcode.h(1425): error C3260: '=': skipping unexpected token(s) before lambda body
C:\Users\Tony\dev\perl\git\perl\opcode.h(1850): error C2337: 'OP_NULL': attribute not found
C:\Users\Tony\dev\perl\git\perl\opcode.h(1428): error C2059: syntax error: '='
C:\Users\Tony\dev\perl\git\perl\opcode.h(1850): error C2059: syntax error: ';'
C:\Users\Tony\dev\perl\git\perl\opcode.h(2274): error C3480: 'OP_NULL': a lambda capture must be a variable from an enclosing function scope
C:\Users\Tony\dev\perl\git\perl\opcode.h(2274): error C3260: '=': skipping unexpected token(s) before lambda body
..\globals.c(45): fatal error C1004: unexpected end-of-file found
N
gcc vs g++
tony@venus:.../perl/git$ gcc -c pointers.c 
pointers.c: In function ‘g’:
pointers.c:6:7: warning: passing argument 1 of ‘f’ from incompatible pointer type [-Wincompatible-pointer-types]
    6 |     f(&t);
      |       ^~
      |       |
      |       size_t * {aka long unsigned int *}
pointers.c:3:15: note: expected ‘int *’ but argument is of type ‘size_t *’ {aka ‘long unsigned int *’}
    3 | extern void f(int *);
      |               ^~~~~
tony@venus:.../perl/git$ g++ -c pointers.c 
pointers.c: In function ‘void g(size_t)’:
pointers.c:6:7: error: cannot convert ‘size_t*’ {aka ‘long unsigned int*’} to int*’
    6 |     f(&t);
      |       ^~
      |       |
      |       size_t* {aka long unsigned int*}
pointers.c:3:15: note:   initializing argument 1 of ‘void f(int*)’
    3 | extern void f(int *);
      |               ^~~~~

@Leont
Copy link
Contributor

Leont commented May 8, 2023

The win32 Makefiles' USE_CPLUSPLUS support appears to be broken, but if I manually supply -TP I get:

Then we should possibly just revert ab28d72. As far as I can tell it doesn't really do anything functionally anyway.

@khwilliamson
Copy link
Contributor

@Leont's idea seems reasonable to me

@Leont
Copy link
Contributor

Leont commented May 12, 2023

But in terms of progress on this, I can see us making a local change to remove register and submit it upstream, but it's not going to fix the other issue.

Adding a strategically placed #define register may be the easiest solution until upstream updates their code.

@leonerd
Copy link
Contributor

leonerd commented May 12, 2023

There's two separate problems:

  • register
  • Use of C99 designated initialisers

We could just revert ab28d72 to undo the initialisers, but that still leaves register. I don't see a good reason not to revert that one anyway, at least it half-fixes the issue.

@leonerd
Copy link
Contributor

leonerd commented May 12, 2023

Ahh except it seems a trivial revert isn't sufficient because it conflicts. and in any case that original commit looks like it should have changed opcode.h. et.al. but they don't appear modified in the same commit. Some more investigation required - @Leont do you want to look into it? Maybe just undo the [NAME] = ... part of the code, as it seems to have kept the right enum order

@thesamesam
Copy link

While we could fix this particular failure by asking upstreams not to use register, it doesn't fix the more general problem that C isn't C++: the designated array initializer means we can't perform similar testing with C++ compilers don't provide this extension.
[...]
I think 1. is useful, but rather than requiring that the whole of perl build with C++, I think it would be better to limit that to the headers, this reduces the load on our upstreams and lets us use C features in most code without the worry that they aren't supported in C++.

This is what most projects I'm familiar with do. systemd has a batch of tests for this, for example.

I think the stricter type checking is close to nonsense, the only stricter type check I'm aware of for our purposes is the requirement to cast void * to other types, the other types of strictness only apply when using C++ style casts.

RIght, C and C++ have different semantics, and while it might complain about some things, Perl is a C project, and any C++ warnings can't be taken literally because while the syntax is often the same, there's different rules about conversions and the rest of it.

tonycoz added a commit to tonycoz/perl5 that referenced this issue May 14, 2023
@tonycoz
Copy link
Contributor Author

tonycoz commented May 16, 2023

There are some additional constructs causing problems with MSVC C++ builds:

  • compound literals of the form (typename){ .field1= ..., ... } aren't available in C++. There's a construct* in C++ with a similar effect, but that syntax isn't C99 compatible: typename{ .field1 = ..., ... } (it has different semantics but I don't think they matter for us.
  • two cpan/ and one dist/ module guarded the perl headers with extern "C"{ ... }, which broke a window header that uses C++ constructs (templates) in C++ builds

There were some other more generic C++ issues.

Fixed by #21096, though I haven't updated version numbers (two modules are in cpan/)

* bearing in mind I haven't written C++ in a long, long time.

tonycoz added a commit to tonycoz/perl5 that referenced this issue May 16, 2023
The second came up while trying to get perl to build as C++ in
MSVC 2022.

Addresses Perl#21073 (but isn't a fix)
tonycoz added a commit to tonycoz/perl5 that referenced this issue May 18, 2023
clang-16 rejects this, MSVC warns about it.

Part of fixing Perl#21073
tonycoz added a commit to tonycoz/digest-md5 that referenced this issue May 22, 2023
These aren't needed with modern perls, and it's only useful to
build modern perls as C++.

This was causing an error in a system header when building blead
as C++ with MSVC:

    C:\Program Files (x86)\Windows Kits\10\include\10.0.22000.0\um\wspiapi.h(53):
    error C2894: templates cannot be declared to have 'C' linkage

See Perl/perl5#21073 for more information
tonycoz added a commit to tonycoz/mime-base64 that referenced this issue May 22, 2023
We often build blead perl using a C++ compiler, but C++17
removed the register storage specifier, so some newer C++ compilers
which default to C++17 now throw an error.

See Perl/perl5#21073 for more discussion
tonycoz added a commit to tonycoz/Time-Piece that referenced this issue May 22, 2023
These aren't needed with modern perls, and it's only useful to
build modern perls as C++.

This was causing an error in a system header when building blead
as C++ with MSVC:

    C:\Program Files (x86)\Windows Kits\10\include\10.0.22000.0\um\wspiapi.h(53):
    error C2894: templates cannot be declared to have 'C' linkage

See Perl/perl5#21073 for more information
tonycoz added a commit to tonycoz/perl5 that referenced this issue May 22, 2023
clang-16 rejects this, MSVC warns about it.

Part of fixing Perl#21073
Leont pushed a commit that referenced this issue May 24, 2023
clang-16 rejects this, MSVC warns about it.

Part of fixing #21073
tonycoz added a commit that referenced this issue Jul 3, 2023
The second came up while trying to get perl to build as C++ in
MSVC 2022.

Addresses #21073 (but isn't a fix)
khwilliamson pushed a commit to khwilliamson/perl5 that referenced this issue Jul 10, 2023
clang-16 rejects this, MSVC warns about it.

Part of fixing Perl#21073
khwilliamson pushed a commit to khwilliamson/perl5 that referenced this issue Jul 10, 2023
The second came up while trying to get perl to build as C++ in
MSVC 2022.

Addresses Perl#21073 (but isn't a fix)
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.

6 participants