Hi Jakub, thanks for looking at this, > On 30 Dec 2021, at 09:33, Jakub Jelinek via Gcc-patches > <gcc-patches@gcc.gnu.org> wrote: >
> The r11-3302-g3696a50beeb73f changes broke the following ObjC testcase. > in_statement is either 0 (not in a looping statement), various IN_* flags > for various kinds of looping statements (or OpenMP structured blocks) or > those flags ored with IN_SWITCH_STMT when a switch appears inside of those > contexts. This is because break binds to switch in that last case, but > continue binds to the looping construct in that case. > The c_finish_bc_stmt function performs diagnostics on incorrect > break/continue uses and then checks if in_statement & IN_OBJC_FOREACH > and in that case jumps to the label provided by the caller, otherwise > emits a BREAK_STMT or CONTINUE_STMT. This is incorrect if we have > ObjC foreach with switch nested in it and break inside of that, > in_statement in that case is IN_OBJC_FOREACH | IN_SWITCH_STMT and > is_break is true. We want to handle it like other breaks inside of > switch, i.e. emit a BREAK_STMT. > > The following patch fixes that. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? I regstrapped this on x86_64-darwin9 and i686-darwin9 and checked this for NeXT and GNU runtimes for m32 and m64 (also on powerpc-darwin9, but without a bootstrap). From the Objective-C perspective, LGTM. thanks Iain > > 2021-12-30 Jakub Jelinek <ja...@redhat.com> > > PR objc/103639 > * c-typeck.c (c_finish_bc_stmt): For break inside of switch inside of > ObjC foreach, emit normal BREAK_STMT rather than goto to label. > > 2021-12-30 Iain Sandoe <i...@sandoe.co.uk> > > PR objc/103639 > * objc.dg/pr103639.m: New test. > > --- gcc/c/c-typeck.c.jj 2021-12-09 15:37:27.657304583 +0100 > +++ gcc/c/c-typeck.c 2021-12-29 16:27:56.693351501 +0100 > @@ -11257,7 +11257,8 @@ c_finish_bc_stmt (location_t loc, tree l > > if (skip) > return NULL_TREE; > - else if (in_statement & IN_OBJC_FOREACH) > + else if ((in_statement & IN_OBJC_FOREACH) > + && !(is_break && (in_statement & IN_SWITCH_STMT))) > { > /* The foreach expander produces low-level code using gotos instead > of a structured loop construct. */ > --- gcc/testsuite/objc.dg/pr103639.m.jj 2021-11-07 16:02:58.901842074 > +0100 > +++ gcc/testsuite/objc.dg/pr103639.m 2021-12-29 21:42:08.253107653 +0100 > @@ -0,0 +1,101 @@ > +/* PR objc/103639 */ > +/* { dg-do run } */ > +/* { dg-skip-if "No NeXT fast enum. pre-Darwin9" { *-*-darwin[5-8]* } { > "-fnext-runtime" } { "" } } */ > +/* { dg-xfail-run-if "Needs OBJC2 ABI" { *-*-darwin* && { lp64 && { ! objc2 > } } } { "-fnext-runtime" } { "" } } */ > +/* { dg-additional-sources > "../objc-obj-c++-shared/nsconstantstring-class-impl.m" } */ > +/* { dg-additional-options "-mno-constant-cfstrings" { target *-*-darwin* } > } */ > +/* { dg-additional-options "-Wno-objc-root-class" } */ > + > +#import "../objc-obj-c++-shared/TestsuiteObject.m" > +#ifndef __NEXT_RUNTIME__ > +#include <objc/NXConstStr.h> > +#else > +#include "../objc-obj-c++-shared/nsconstantstring-class.h" > +#endif > + > +extern int printf (const char *, ...); > +#include <stdlib.h> > + > +/* A mini-array implementation that can be used to test fast > + enumeration. You create the array with some objects; you can > + mutate the array, and you can fast-enumerate it. > + */ > +@interface MyArray : TestsuiteObject > +{ > + unsigned int length; > + id *objects; > + unsigned long mutated; > +} > +- (id) initWithLength: (unsigned int)l objects: (id *)o; > +- (void) mutate; > +- (unsigned long)countByEnumeratingWithState: (struct > __objcFastEnumerationState *)state > + objects:(id *)stackbuf > + count:(unsigned long)len; > +@end > + > +@implementation MyArray : TestsuiteObject > +- (id) initWithLength: (unsigned int)l > + objects: (id *)o > +{ > + length = l; > + objects = o; > + mutated = 0; > + return self; > +} > +- (void) mutate > +{ > + mutated = 1; > +} > +- (unsigned long)countByEnumeratingWithState: (struct > __objcFastEnumerationState*)state > + objects: (id*)stackbuf > + count: (unsigned long)len > +{ > + unsigned long i, batch_size; > + > + /* We keep how many objects we served in the state->state counter. So the > next batch > + will contain up to length - state->state objects. */ > + batch_size = length - state->state; > + > + /* Make obvious adjustments. */ > + if (batch_size < 0) > + batch_size = 0; > + > + if (batch_size > len) > + batch_size = len; > + > + /* Copy the objects. */ > + for (i = 0; i < batch_size; i++) > + stackbuf[i] = objects[i]; > + > + state->state += batch_size; > + state->itemsPtr = stackbuf; > + state->mutationsPtr = &mutated; > + > + return batch_size; > +} > +@end > + > +int check = 0; > + > +int > +main() > +{ > + id *objects = malloc (sizeof (id) * 2); > + objects[0] = @"a"; > + objects[1] = @"b"; > + > + MyArray *array = [[MyArray alloc] initWithLength: 2 objects: objects]; > + > + int someVar = 0; > + for (id object in array) { > + switch (someVar) { > + case 0: > + break; > + } > + ++check; > + } > + > + if (check != 2) > + abort (); > + return 0; > +} > > > Jakub >