[PATCH] Fix -Wshadow=local warnings in rtl.h

[Richard Sandiford]

Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> Hi,
>
> this fixes -Wshadow=local warnings in the RTL_FLAG_CHECKx macros,
> which happen when this macro is used recursively in a macro
> argument.  The __typeof (RTX) const _rtx in the inner macro
> expansions shadows the outer macro expansions.
>
> So reworked the macro to not use statement expressions but
> use templates instead.  Since the 7-argument overload is not
> used anywhere removed RTL_FLAG_CHECK7 for now.
>
>
> Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
> Is it OK for trunk?
>
>
> Thanks
> Bernd.
>
> 2019-10-03  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>
> 	* rtl.h (RTL_FLAG_CHECK): New variadic macro.
> 	(RTL_FLAG_CHECK1-6): Use RTL_FLAG_CHECK.
> 	(RTL_FLAG_CHECK7): Remove.
> 	(rtl_flag_check, check_rtl_code): New helper functions.
>
> Index: gcc/rtl.h
> ===================================================================
> --- gcc/rtl.h	(revision 276484)
> +++ gcc/rtl.h	(working copy)
> @@ -1249,66 +1249,18 @@ extern void rtvec_check_failed_bounds (const_rtvec
>  #define RTX_FLAG(RTX, FLAG)	((RTX)->FLAG)
>  
>  #if defined ENABLE_RTL_FLAG_CHECKING && (GCC_VERSION >= 2007)
> -#define RTL_FLAG_CHECK1(NAME, RTX, C1) __extension__			\
> -({ __typeof (RTX) const _rtx = (RTX);					\
> -   if (GET_CODE (_rtx) != C1)						\
> -     rtl_check_failed_flag  (NAME, _rtx, __FILE__, __LINE__,		\
> -			     __FUNCTION__);				\
> -   _rtx; })
> +#define RTL_FLAG_CHECK(NAME, RTX, ...) 					\
> +  ((__typeof (&*(RTX))) rtl_flag_check (check_rtl_code<__VA_ARGS__>,	\
> +					NAME, RTX, __FILE__, __LINE__,	\
> +					__FUNCTION__))

Yet another comment on this one, sorry, but why not just make
rtl_flag_check a template function instead of defining two versions
of it and using __typeof?  Is the idea is to make sure that GET_CODE
is only applied to genuine rtxes rather than some random structure
with a field called "code"?  If so, I think we should do that in
GET_CODE itself, and do it regardless of whether rtl checking is
enabled.  E.g. something like:

#define GET_CODE(RTX) ((rtx_code) static_cast<const_rtx> (RTX)->code)

(Wonder how much code that will break :-))

And if we do use templates instead of const_rtx/rtx variants,
it might be simpler to keep the checking and assert together:

#define RTL_FLAG_CHECK(NAME, RTX, ...) 				\
  (rtl_flag_check<__VA_ARGS__> (NAME, RTX, __FILE__, __LINE__,	\
				__FUNCTION__))

template<rtx_code C1, typename T>
inline T
rtl_flag_check (const char *name, T rtl, const char *file, int line,
		const char *func)
{ 
  if (GET_CODE (rtl) != C1)
    rtl_check_failed_flag (name, rtl, file, line, func);
  return rtl;
}

...etc...

which could also address some of the fears around cost.

Personally I'm not fussed about keeping the checking and assert
together though, just a suggestion.

Richard