Fix -Wshadow=local warnings in genautomata.c
diff mbox series

Message ID VI1PR03MB45289E8FD7DD111FE1F0E2BAE49E0@VI1PR03MB4528.eurprd03.prod.outlook.com
State New
Headers show
Series
  • Fix -Wshadow=local warnings in genautomata.c
Related show

Commit Message

Bernd Edlinger Oct. 4, 2019, 7:20 p.m. UTC
Hi,

this is probably on the border to obvious.

The REGEXP_xxx macros in genautomata are invoked
recursively, and the local values are all named _regexp
and shadow each other.


Fixed by using different names _regexp1..6 for each
macro.


Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?


Thanks
Bernd.

Comments

Richard Sandiford Oct. 16, 2019, 3:11 p.m. UTC | #1
Sorry for the slow reply.

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> Hi,
>
> this is probably on the border to obvious.
>
> The REGEXP_xxx macros in genautomata are invoked
> recursively, and the local values are all named _regexp
> and shadow each other.
>
>
> Fixed by using different names _regexp1..6 for each
> macro.

Sorry to repeat the complaint about numerical suffixes, but I think
we'd need better names.  E.g. _regexp_unit or _re_unit for REGEXP_UNIT
and similarly for the other macros.  But a similar fix to rtl.h might
be better.

Thanks,
Richard

> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
>
> 2019-10-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	* genautomata.c (REGEXP_UNIT, REGEXP_RESERV, REGEXP_SEQUENCE,
> 	REGEXP_REPEAT, REGEXP_ALLOF, REGEXP_ONEOF): Rename local vars.
>
> Index: gcc/genautomata.c
> ===================================================================
> --- gcc/genautomata.c	(revision 276484)
> +++ gcc/genautomata.c	(working copy)
> @@ -984,46 +984,46 @@ decl_mode_check_failed (enum decl_mode mode, const
>  
>  
>  #define REGEXP_UNIT(r) __extension__					\
> -(({ struct regexp *const _regexp = (r);					\
> -     if (_regexp->mode != rm_unit)					\
> -       regexp_mode_check_failed (_regexp->mode, "rm_unit",		\
> +(({ struct regexp *const _regex1 = (r);					\
> +     if (_regex1->mode != rm_unit)					\
> +       regexp_mode_check_failed (_regex1->mode, "rm_unit",		\
>  			       __FILE__, __LINE__, __FUNCTION__);	\
> -     &(_regexp)->regexp.unit; }))
> +     &(_regex1)->regexp.unit; }))
>  
>  #define REGEXP_RESERV(r) __extension__					\
> -(({ struct regexp *const _regexp = (r);					\
> -     if (_regexp->mode != rm_reserv)					\
> -       regexp_mode_check_failed (_regexp->mode, "rm_reserv",		\
> +(({ struct regexp *const _regex2 = (r);					\
> +     if (_regex2->mode != rm_reserv)					\
> +       regexp_mode_check_failed (_regex2->mode, "rm_reserv",		\
>  			       __FILE__, __LINE__, __FUNCTION__);	\
> -     &(_regexp)->regexp.reserv; }))
> +     &(_regex2)->regexp.reserv; }))
>  
>  #define REGEXP_SEQUENCE(r) __extension__				\
> -(({ struct regexp *const _regexp = (r);					\
> -     if (_regexp->mode != rm_sequence)					\
> -       regexp_mode_check_failed (_regexp->mode, "rm_sequence",		\
> +(({ struct regexp *const _regex3 = (r);					\
> +     if (_regex3->mode != rm_sequence)					\
> +       regexp_mode_check_failed (_regex3->mode, "rm_sequence",		\
>  			       __FILE__, __LINE__, __FUNCTION__);	\
> -     &(_regexp)->regexp.sequence; }))
> +     &(_regex3)->regexp.sequence; }))
>  
>  #define REGEXP_REPEAT(r) __extension__					\
> -(({ struct regexp *const _regexp = (r);					\
> -     if (_regexp->mode != rm_repeat)					\
> -       regexp_mode_check_failed (_regexp->mode, "rm_repeat",		\
> +(({ struct regexp *const _regex4 = (r);					\
> +     if (_regex4->mode != rm_repeat)					\
> +       regexp_mode_check_failed (_regex4->mode, "rm_repeat",		\
>  			       __FILE__, __LINE__, __FUNCTION__);	\
> -     &(_regexp)->regexp.repeat; }))
> +     &(_regex4)->regexp.repeat; }))
>  
>  #define REGEXP_ALLOF(r) __extension__					\
> -(({ struct regexp *const _regexp = (r);					\
> -     if (_regexp->mode != rm_allof)					\
> -       regexp_mode_check_failed (_regexp->mode, "rm_allof",		\
> +(({ struct regexp *const _regex5 = (r);					\
> +     if (_regex5->mode != rm_allof)					\
> +       regexp_mode_check_failed (_regex5->mode, "rm_allof",		\
>  			       __FILE__, __LINE__, __FUNCTION__);	\
> -     &(_regexp)->regexp.allof; }))
> +     &(_regex5)->regexp.allof; }))
>  
>  #define REGEXP_ONEOF(r) __extension__					\
> -(({ struct regexp *const _regexp = (r);					\
> -     if (_regexp->mode != rm_oneof)					\
> -       regexp_mode_check_failed (_regexp->mode, "rm_oneof",		\
> +(({ struct regexp *const _regex6 = (r);					\
> +     if (_regex6->mode != rm_oneof)					\
> +       regexp_mode_check_failed (_regex6->mode, "rm_oneof",		\
>  			       __FILE__, __LINE__, __FUNCTION__);	\
> -     &(_regexp)->regexp.oneof; }))
> +     &(_regex6)->regexp.oneof; }))
>  
>  static const char *regexp_name (enum regexp_mode);
>  static void regexp_mode_check_failed (enum regexp_mode, const char *,
Martin Sebor Oct. 16, 2019, 3:43 p.m. UTC | #2
On 10/16/19 9:11 AM, Richard Sandiford wrote:
> Sorry for the slow reply.
> 
> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>> Hi,
>>
>> this is probably on the border to obvious.
>>
>> The REGEXP_xxx macros in genautomata are invoked
>> recursively, and the local values are all named _regexp
>> and shadow each other.
>>
>>
>> Fixed by using different names _regexp1..6 for each
>> macro.
> 
> Sorry to repeat the complaint about numerical suffixes, but I think
> we'd need better names.  E.g. _regexp_unit or _re_unit for REGEXP_UNIT
> and similarly for the other macros.  But a similar fix to rtl.h might
> be better.

Should the warning trigger when the shadowing name results from
macro expansion?  The author of a macro can't (in general) know
what context it's going to be used, and when different macros
come from two different third party headers, it would seem
pointless to force their users to jump through hoops just to
avoid the innocuous shadowing.  Such as in this example:

#define Abs(x) \
   __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))

#define Min(x, y) \
   __extension__ (({ __typeof__ (x) _x = x; __typeof__ (y) _y = y; _x < 
_y ? _x : _y; }))

int f (int x, int y)
{
   return Abs (Min (x, y));   // -Wshadow for _x?
}

Martin

> Thanks,
> Richard
> 
>> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
>> Is it OK for trunk?
>>
>>
>> Thanks
>> Bernd.
>>
>> 2019-10-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* genautomata.c (REGEXP_UNIT, REGEXP_RESERV, REGEXP_SEQUENCE,
>> 	REGEXP_REPEAT, REGEXP_ALLOF, REGEXP_ONEOF): Rename local vars.
>>
>> Index: gcc/genautomata.c
>> ===================================================================
>> --- gcc/genautomata.c	(revision 276484)
>> +++ gcc/genautomata.c	(working copy)
>> @@ -984,46 +984,46 @@ decl_mode_check_failed (enum decl_mode mode, const
>>   
>>   
>>   #define REGEXP_UNIT(r) __extension__					\
>> -(({ struct regexp *const _regexp = (r);					\
>> -     if (_regexp->mode != rm_unit)					\
>> -       regexp_mode_check_failed (_regexp->mode, "rm_unit",		\
>> +(({ struct regexp *const _regex1 = (r);					\
>> +     if (_regex1->mode != rm_unit)					\
>> +       regexp_mode_check_failed (_regex1->mode, "rm_unit",		\
>>   			       __FILE__, __LINE__, __FUNCTION__);	\
>> -     &(_regexp)->regexp.unit; }))
>> +     &(_regex1)->regexp.unit; }))
>>   
>>   #define REGEXP_RESERV(r) __extension__					\
>> -(({ struct regexp *const _regexp = (r);					\
>> -     if (_regexp->mode != rm_reserv)					\
>> -       regexp_mode_check_failed (_regexp->mode, "rm_reserv",		\
>> +(({ struct regexp *const _regex2 = (r);					\
>> +     if (_regex2->mode != rm_reserv)					\
>> +       regexp_mode_check_failed (_regex2->mode, "rm_reserv",		\
>>   			       __FILE__, __LINE__, __FUNCTION__);	\
>> -     &(_regexp)->regexp.reserv; }))
>> +     &(_regex2)->regexp.reserv; }))
>>   
>>   #define REGEXP_SEQUENCE(r) __extension__				\
>> -(({ struct regexp *const _regexp = (r);					\
>> -     if (_regexp->mode != rm_sequence)					\
>> -       regexp_mode_check_failed (_regexp->mode, "rm_sequence",		\
>> +(({ struct regexp *const _regex3 = (r);					\
>> +     if (_regex3->mode != rm_sequence)					\
>> +       regexp_mode_check_failed (_regex3->mode, "rm_sequence",		\
>>   			       __FILE__, __LINE__, __FUNCTION__);	\
>> -     &(_regexp)->regexp.sequence; }))
>> +     &(_regex3)->regexp.sequence; }))
>>   
>>   #define REGEXP_REPEAT(r) __extension__					\
>> -(({ struct regexp *const _regexp = (r);					\
>> -     if (_regexp->mode != rm_repeat)					\
>> -       regexp_mode_check_failed (_regexp->mode, "rm_repeat",		\
>> +(({ struct regexp *const _regex4 = (r);					\
>> +     if (_regex4->mode != rm_repeat)					\
>> +       regexp_mode_check_failed (_regex4->mode, "rm_repeat",		\
>>   			       __FILE__, __LINE__, __FUNCTION__);	\
>> -     &(_regexp)->regexp.repeat; }))
>> +     &(_regex4)->regexp.repeat; }))
>>   
>>   #define REGEXP_ALLOF(r) __extension__					\
>> -(({ struct regexp *const _regexp = (r);					\
>> -     if (_regexp->mode != rm_allof)					\
>> -       regexp_mode_check_failed (_regexp->mode, "rm_allof",		\
>> +(({ struct regexp *const _regex5 = (r);					\
>> +     if (_regex5->mode != rm_allof)					\
>> +       regexp_mode_check_failed (_regex5->mode, "rm_allof",		\
>>   			       __FILE__, __LINE__, __FUNCTION__);	\
>> -     &(_regexp)->regexp.allof; }))
>> +     &(_regex5)->regexp.allof; }))
>>   
>>   #define REGEXP_ONEOF(r) __extension__					\
>> -(({ struct regexp *const _regexp = (r);					\
>> -     if (_regexp->mode != rm_oneof)					\
>> -       regexp_mode_check_failed (_regexp->mode, "rm_oneof",		\
>> +(({ struct regexp *const _regex6 = (r);					\
>> +     if (_regex6->mode != rm_oneof)					\
>> +       regexp_mode_check_failed (_regex6->mode, "rm_oneof",		\
>>   			       __FILE__, __LINE__, __FUNCTION__);	\
>> -     &(_regexp)->regexp.oneof; }))
>> +     &(_regex6)->regexp.oneof; }))
>>   
>>   static const char *regexp_name (enum regexp_mode);
>>   static void regexp_mode_check_failed (enum regexp_mode, const char *,
Jakub Jelinek Oct. 16, 2019, 3:50 p.m. UTC | #3
On Wed, Oct 16, 2019 at 09:43:49AM -0600, Martin Sebor wrote:
> Should the warning trigger when the shadowing name results from
> macro expansion?  The author of a macro can't (in general) know
> what context it's going to be used, and when different macros
> come from two different third party headers, it would seem
> pointless to force their users to jump through hoops just to
> avoid the innocuous shadowing.  Such as in this example:
> 
> #define Abs(x) \
>   __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))
> 
> #define Min(x, y) \
>   __extension__ (({ __typeof__ (x) _x = x; __typeof__ (y) _y = y; _x < _y ?
> _x : _y; }))
> 
> int f (int x, int y)
> {
>   return Abs (Min (x, y));   // -Wshadow for _x?
> }

The counter example would be:
#define F(x) \
  __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))
#define G(x) \
  __extension__ (({ __typeof__ (x) _x = x; F(_x); }))
where a -Wshadow diagnostics could point the author at a serious bug,
because in the expansion it will be __typeof__ (_x) _x = _x; ...

	Jakub
Martin Sebor Oct. 16, 2019, 4:03 p.m. UTC | #4
On 10/16/19 9:50 AM, Jakub Jelinek wrote:
> On Wed, Oct 16, 2019 at 09:43:49AM -0600, Martin Sebor wrote:
>> Should the warning trigger when the shadowing name results from
>> macro expansion?  The author of a macro can't (in general) know
>> what context it's going to be used, and when different macros
>> come from two different third party headers, it would seem
>> pointless to force their users to jump through hoops just to
>> avoid the innocuous shadowing.  Such as in this example:
>>
>> #define Abs(x) \
>>    __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))
>>
>> #define Min(x, y) \
>>    __extension__ (({ __typeof__ (x) _x = x; __typeof__ (y) _y = y; _x < _y ?
>> _x : _y; }))
>>
>> int f (int x, int y)
>> {
>>    return Abs (Min (x, y));   // -Wshadow for _x?
>> }
> 
> The counter example would be:
> #define F(x) \
>    __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))
> #define G(x) \
>    __extension__ (({ __typeof__ (x) _x = x; F(_x); }))
> where a -Wshadow diagnostics could point the author at a serious bug,
> because in the expansion it will be __typeof__ (_x) _x = _x; ...

True.  I don't suppose there is a way to make it so the warning
triggers for the counter example and not for the original, is
there?

Martin

PS The counterexample nicely illustrates why -Wself-init should
be in -Wall like in Clang or MSVC, or at least in -Wextra like in
ICC.  Let me take it as a reminder to submit a patch for GCC 10.
Joseph Myers Oct. 16, 2019, 4:35 p.m. UTC | #5
On Wed, 16 Oct 2019, Jakub Jelinek wrote:

> The counter example would be:
> #define F(x) \
>   __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))
> #define G(x) \
>   __extension__ (({ __typeof__ (x) _x = x; F(_x); }))
> where a -Wshadow diagnostics could point the author at a serious bug,
> because in the expansion it will be __typeof__ (_x) _x = _x; ...

And this is not theoretical, 
<https://sourceware.org/bugzilla/show_bug.cgi?id=15667> 
<https://sourceware.org/ml/libc-alpha/2013-06/msg00851.html> was a real 
bug in glibc soft-fp where shadowing of variables called _c1 and _c2 in 
two macros resulted in wrong code.
Jakub Jelinek Oct. 16, 2019, 4:47 p.m. UTC | #6
On Wed, Oct 16, 2019 at 10:03:51AM -0600, Martin Sebor wrote:
> > The counter example would be:
> > #define F(x) \
> >    __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))
> > #define G(x) \
> >    __extension__ (({ __typeof__ (x) _x = x; F(_x); }))
> > where a -Wshadow diagnostics could point the author at a serious bug,
> > because in the expansion it will be __typeof__ (_x) _x = _x; ...
> 
> True.  I don't suppose there is a way to make it so the warning
> triggers for the counter example and not for the original, is
> there?

Maybe look through the macro nesting context and if the shadowing
declaration comes from the same macro as shadowed declaration
or macro included directly or indirectly from the macro with shadowed
declaration, warn, otherwise not?
This might still not warn in case where the scope of the shadowing
declaration is created from multiple macros ({ coming from one,
}) from another one, but otherwise could work.
Perhaps -Wshadow-local needs multiple modes, the default one that
will have this macro handling and full one (=2) which would warn
regardless of macro definitions.

	Jakub
Segher Boessenkool Oct. 16, 2019, 7:31 p.m. UTC | #7
On Wed, Oct 16, 2019 at 10:03:51AM -0600, Martin Sebor wrote:
> PS The counterexample nicely illustrates why -Wself-init should
> be in -Wall like in Clang or MSVC, or at least in -Wextra like in
> ICC.  Let me take it as a reminder to submit a patch for GCC 10.

c-family/c-gimplify.c says:

      /* This is handled mostly by gimplify.c, but we have to deal with
         not warning about int x = x; as it is a GCC extension to turn off
         this warning but only if warn_init_self is zero.  */

A lot of code will start to warn if you turn on -Winit-self by default
(in -Wall or -W), since forever we have had this GCC extension, and
people expect it.  (Or so I fear, feel free to prove me wrong :-) )


Segher
Eric Gallager Oct. 16, 2019, 11:14 p.m. UTC | #8
On 10/16/19, Jakub Jelinek <jakub@redhat.com> wrote:
> On Wed, Oct 16, 2019 at 10:03:51AM -0600, Martin Sebor wrote:
>> > The counter example would be:
>> > #define F(x) \
>> >    __extension__ (({ __typeof__ (x) _x = x; _x < 0 ? -_x : _x; }))
>> > #define G(x) \
>> >    __extension__ (({ __typeof__ (x) _x = x; F(_x); }))
>> > where a -Wshadow diagnostics could point the author at a serious bug,
>> > because in the expansion it will be __typeof__ (_x) _x = _x; ...
>>
>> True.  I don't suppose there is a way to make it so the warning
>> triggers for the counter example and not for the original, is
>> there?
>
> Maybe look through the macro nesting context and if the shadowing
> declaration comes from the same macro as shadowed declaration
> or macro included directly or indirectly from the macro with shadowed
> declaration, warn, otherwise not?
> This might still not warn in case where the scope of the shadowing
> declaration is created from multiple macros ({ coming from one,
> }) from another one, but otherwise could work.
> Perhaps -Wshadow-local needs multiple modes, the default one that
> will have this macro handling and full one (=2) which would warn
> regardless of macro definitions.
>

I'm worried about the proliferation of the number of '=' signs here...
there's already confusion as to whether the first '=' represents
levels or just a different spelling of names, adding a second would
only compound the confusion.

> 	Jakub
>
Jeff Law Oct. 17, 2019, 12:53 a.m. UTC | #9
On 10/16/19 9:43 AM, Martin Sebor wrote:
> On 10/16/19 9:11 AM, Richard Sandiford wrote:
>> Sorry for the slow reply.
>>
>> Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
>>> Hi,
>>>
>>> this is probably on the border to obvious.
>>>
>>> The REGEXP_xxx macros in genautomata are invoked
>>> recursively, and the local values are all named _regexp
>>> and shadow each other.
>>>
>>>
>>> Fixed by using different names _regexp1..6 for each
>>> macro.
>>
>> Sorry to repeat the complaint about numerical suffixes, but I think
>> we'd need better names.  E.g. _regexp_unit or _re_unit for REGEXP_UNIT
>> and similarly for the other macros.  But a similar fix to rtl.h might
>> be better.
> 
> Should the warning trigger when the shadowing name results from
> macro expansion?  The author of a macro can't (in general) know
> what context it's going to be used, and when different macros
> come from two different third party headers, it would seem
> pointless to force their users to jump through hoops just to
> avoid the innocuous shadowing.  Such as in this example:
One could make the argument that if you want to avoid shadowing, then
you should avoid code within macros for this precise reason.  And if
you're getting code within macros from 3rd parties, then well, you're in
for a world of pain if you're going to try to be shadow-free.



jeff

Patch
diff mbox series

2019-10-04  Bernd Edlinger  <bernd.edlinger@hotmail.de>

	* genautomata.c (REGEXP_UNIT, REGEXP_RESERV, REGEXP_SEQUENCE,
	REGEXP_REPEAT, REGEXP_ALLOF, REGEXP_ONEOF): Rename local vars.

Index: gcc/genautomata.c
===================================================================
--- gcc/genautomata.c	(revision 276484)
+++ gcc/genautomata.c	(working copy)
@@ -984,46 +984,46 @@  decl_mode_check_failed (enum decl_mode mode, const
 
 
 #define REGEXP_UNIT(r) __extension__					\
-(({ struct regexp *const _regexp = (r);					\
-     if (_regexp->mode != rm_unit)					\
-       regexp_mode_check_failed (_regexp->mode, "rm_unit",		\
+(({ struct regexp *const _regex1 = (r);					\
+     if (_regex1->mode != rm_unit)					\
+       regexp_mode_check_failed (_regex1->mode, "rm_unit",		\
 			       __FILE__, __LINE__, __FUNCTION__);	\
-     &(_regexp)->regexp.unit; }))
+     &(_regex1)->regexp.unit; }))
 
 #define REGEXP_RESERV(r) __extension__					\
-(({ struct regexp *const _regexp = (r);					\
-     if (_regexp->mode != rm_reserv)					\
-       regexp_mode_check_failed (_regexp->mode, "rm_reserv",		\
+(({ struct regexp *const _regex2 = (r);					\
+     if (_regex2->mode != rm_reserv)					\
+       regexp_mode_check_failed (_regex2->mode, "rm_reserv",		\
 			       __FILE__, __LINE__, __FUNCTION__);	\
-     &(_regexp)->regexp.reserv; }))
+     &(_regex2)->regexp.reserv; }))
 
 #define REGEXP_SEQUENCE(r) __extension__				\
-(({ struct regexp *const _regexp = (r);					\
-     if (_regexp->mode != rm_sequence)					\
-       regexp_mode_check_failed (_regexp->mode, "rm_sequence",		\
+(({ struct regexp *const _regex3 = (r);					\
+     if (_regex3->mode != rm_sequence)					\
+       regexp_mode_check_failed (_regex3->mode, "rm_sequence",		\
 			       __FILE__, __LINE__, __FUNCTION__);	\
-     &(_regexp)->regexp.sequence; }))
+     &(_regex3)->regexp.sequence; }))
 
 #define REGEXP_REPEAT(r) __extension__					\
-(({ struct regexp *const _regexp = (r);					\
-     if (_regexp->mode != rm_repeat)					\
-       regexp_mode_check_failed (_regexp->mode, "rm_repeat",		\
+(({ struct regexp *const _regex4 = (r);					\
+     if (_regex4->mode != rm_repeat)					\
+       regexp_mode_check_failed (_regex4->mode, "rm_repeat",		\
 			       __FILE__, __LINE__, __FUNCTION__);	\
-     &(_regexp)->regexp.repeat; }))
+     &(_regex4)->regexp.repeat; }))
 
 #define REGEXP_ALLOF(r) __extension__					\
-(({ struct regexp *const _regexp = (r);					\
-     if (_regexp->mode != rm_allof)					\
-       regexp_mode_check_failed (_regexp->mode, "rm_allof",		\
+(({ struct regexp *const _regex5 = (r);					\
+     if (_regex5->mode != rm_allof)					\
+       regexp_mode_check_failed (_regex5->mode, "rm_allof",		\
 			       __FILE__, __LINE__, __FUNCTION__);	\
-     &(_regexp)->regexp.allof; }))
+     &(_regex5)->regexp.allof; }))
 
 #define REGEXP_ONEOF(r) __extension__					\
-(({ struct regexp *const _regexp = (r);					\
-     if (_regexp->mode != rm_oneof)					\
-       regexp_mode_check_failed (_regexp->mode, "rm_oneof",		\
+(({ struct regexp *const _regex6 = (r);					\
+     if (_regex6->mode != rm_oneof)					\
+       regexp_mode_check_failed (_regex6->mode, "rm_oneof",		\
 			       __FILE__, __LINE__, __FUNCTION__);	\
-     &(_regexp)->regexp.oneof; }))
+     &(_regex6)->regexp.oneof; }))
 
 static const char *regexp_name (enum regexp_mode);
 static void regexp_mode_check_failed (enum regexp_mode, const char *,