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

Fix g++ build (Add extra scope block for switch case) #20109

Merged
merged 1 commit into from
Aug 17, 2022

Conversation

bram-perl
Copy link

Code in 9fdd7fc broke the g++ builds,

Basically after the commit the code looked like:

     switch (o->op_type) {
     ...
     case OP_SASSIGN:
         ...
         OP* rhs = cBINOPx(o)->op_first;
         OP* lval = cBINOPx(o)->op_last;
         ...
         break;

     case OP_AASSIGN: {

g++ does not allow this and errors with:

peep.c:3897:14: error: jump to case label
 3897 |         case OP_AASSIGN: {
      |              ^~~~~~~~~~
peep.c:3844:17: note:   crosses initialization of 'OP* lval'
 3844 |             OP* lval = cBINOPx(o)->op_last;
      |                 ^~~~
peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
 3843 |             OP* rhs = cBINOPx(o)->op_first;
      |                 ^~~

This happens because rhs and lval are not scoped in the case statement
so it could fall through to the next case.

The solution is to scope them which this commit now does by adding a
separate scope for OP_SASSIGN (similar to OP_AASSIGN).

Fixes #20108

Code in 9fdd7fc broke the g++ builds,

Basically after the commit the code looked like:

         switch (o->op_type) {
         ...
         case OP_SASSIGN:
             ...
             OP* rhs = cBINOPx(o)->op_first;
             OP* lval = cBINOPx(o)->op_last;
             ...
             break;

         case OP_AASSIGN: {

g++ does not allow this and errors with:

	peep.c:3897:14: error: jump to case label
	 3897 |         case OP_AASSIGN: {
	      |              ^~~~~~~~~~
	peep.c:3844:17: note:   crosses initialization of 'OP* lval'
	 3844 |             OP* lval = cBINOPx(o)->op_last;
	      |                 ^~~~
	peep.c:3843:17: note:   crosses initialization of 'OP* rhs'
	 3843 |             OP* rhs = cBINOPx(o)->op_first;
	      |                 ^~~

This happens because `rhs` and `lval` are not scoped in the case statement
so it could fall through to the next case.

The solution is to scope them which this commit now does by adding a
separate scope for `OP_SASSIGN` (similar to `OP_AASSIGN`).

Fixes Perl#20108
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

@jkeenan
Copy link
Contributor

jkeenan commented Aug 17, 2022

$ gitcurr
bram/fix-gh-20108-fix-g++-build

$ git describe
v5.37.2-180-gfa3e3c8122

$ uname -mrs
FreeBSD 12.3-RELEASE-p1 amd64

$ g++ --version
g++ (FreeBSD Ports Collection) 10.3.0
Copyright (C) 2020 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

$ ./perl -Ilib -V:config_args
config_args='-des -Dusedevel -Dcc=g++';

...
$ make test_harness
...
Result: PASS

LGTM

@khwilliamson khwilliamson merged commit 2652cd7 into Perl:blead Aug 17, 2022
@bram-perl bram-perl deleted the bram/fix-gh-20108-fix-g++-build branch September 4, 2022 12:44
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.

9fdd7fc479 breaks build using g++ as compiler
4 participants