Skip to content

dump.c: Fix broken c++ compilations in #23536

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

Merged
merged 1 commit into from
Aug 4, 2025

Conversation

khwilliamson
Copy link
Contributor

Commit bce5eba broke c++ compilations because a goto now crosses declarations with initialization.

This commit merely separates the declarations from their initialization, and moves the declarations to before the goto.

The alternative could just as easily have been to move the goto to after the initialization, but the initialization would then be (a bit of) extra work, and more importantly, that would break the code flow of immediately using the initialized values.

  • This set of changes does not require a perldelta entry.

Commit bce5eba broke c++ compilations
because a goto now crosses declarations with initialization.

This commit merely separates the declarations from their initialization,
and moves the declarations to before the goto.

The alternative could just as easily have been to move the goto to after
the initialization, but the initialization would then be (a bit of)
extra work, and more importantly, that would break the code flow of
immediately using the initialized values.
Copy link
Contributor

@leonerd leonerd left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@khwilliamson khwilliamson merged commit 5265c66 into Perl:blead Aug 4, 2025
34 checks passed
@khwilliamson khwilliamson deleted the cplus_dump.c branch August 4, 2025 14:14
@jkeenan
Copy link
Contributor

jkeenan commented Aug 4, 2025

@khwilliamson, I configured and build perl using g++ (12.2.0) as the C-compiler at the commit which you claimed broke the c++ build.

$ cd ~/gitwork/perl
$ git checkout bce5eba453b71437b5d508b1e717646587dda0db
$ git show --format=fuller
commit bce5eba453b71437b5d508b1e717646587dda0db (HEAD)
Author:     David Mitchell <[email protected]>
AuthorDate: Sat May 24 10:53:30 2025 +0100
Commit:     David Mitchell <[email protected]>
CommitDate: Mon Aug 4 11:52:38 2025 +0100

    Improve 'perl -Dx' debug output on threaded builds

$ sh ./Configure -des -Dusedevel -Dcc=g++ && make test_prep
...
$ ./perl -Ilib -v | head -2 | tail -1
This is perl 5, version 43, subversion 2 (v5.43.2 (v5.43.1-133-gbce5eba453)) built for x86_64-linux
$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -Dcc=g++';

I found no evidence that the c++ build was broken at this commit.

I then configured and built at the commit you pushed to correct the alleged breakage.

$ cd ../perl2
$ git checkout blead
$ git show --format=fuller
commit 5265c6667a62061ddfafdef3e320bc789da968c9 (HEAD -> blead, origin/blead, origin/HEAD)
Author:     Karl Williamson <[email protected]>
AuthorDate: Mon Aug 4 07:29:25 2025 -0600
Commit:     Karl Williamson <[email protected]>
CommitDate: Mon Aug 4 07:56:52 2025 -0600

     dump.c: Fix broken c++ compilations in
    
    Commit bce5eba453b71437b5d508b1e717646587dda0db broke c++ compilations
    because a goto now crosses declarations with initialization.
...


$ sh ./Configure -des -Dusedevel -Dcc=g++ && make test_prep
$ ./perl -Ilib -v | head -2 | tail -1
This is perl 5, version 43, subversion 2 (v5.43.2 (v5.43.1-141-g5265c6667a)) built for x86_64-linux
$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -Dcc=g++';

Can you post some evidence of the C++ breakage at bce5eba? Thanks.

@jkeenan
Copy link
Contributor

jkeenan commented Aug 4, 2025

After an hour or so I noticed this smoke-test failure: https://perl.develop-help.com/db/5517488, where perl was compiled with g++ but built with threads. I followed that example; this indeed failed to build.

g++ -c -DPERL_CORE -D_REENTRANT -D_GNU_SOURCE -fwrapv -fno-strict-aliasing -pipe -fstack-protector-strong -I/usr/local/include -D_LARGEFILE_SOURCE -D_FILE_OFFSET_BITS=64 -D_FORTIFY_SOURCE=2 -O2 -Wall -Werror=pointer-arith -Werror=vla -Wextra -Wno-long-long -Wwrite-strings -Wno-use-after-free dump.c
dump.c: In function ‘SV* S_get_sv_from_pad(PerlInterpreter*, const OP*, PADOFFSET, CV*)’:
dump.c:1209:4: error: jump to label ‘got_cv’
 1209 |    got_cv:
      |    ^~~~~~
dump.c:1132:14: note:   from here
 1132 |         goto got_cv;
      |              ^~~~~~
dump.c:1142:9: note:   crosses initialization of ‘OP* oproot’
 1142 |     OP *oproot = (OP*)o;
      |         ^~~~~~
dump.c:1141:9: note:   crosses initialization of ‘int n’
 1141 |     int n = 100;
      |         ^
make: *** [makefile:265: dump.o] Error 1

So there was no problem with unthreaded c++ builds.

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 this pull request may close these issues.

3 participants