This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

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


Bernd Edlinger <bernd.edlinger@hotmail.de> writes:
> On 10/4/19 2:38 PM, Richard Sandiford wrote:
>> 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:
>> 
>
> Actually I wanted to do it with a template, and invoke it using __typeof(RTX).
>
> BUT with that I ran into a lmitation of the template vs. block statements
> See PR#91803:  We cannot instantiate a template on the
> type of a statement expression, only when the expression is completely
> written down without statement expressions.
>
> So even if we remove that limitation, it will be impossible to use
> templates dependent on __typeof(statement-expressions) as we need
> to bootstrap from rather old gcc versions.
>
> However if we are able to replace all statement-expressions in
> rtl.h then it will probably be possible to use a template here,
> without that type cast.
>
> I still need a const and a non-const version of that function because
> otherwise the -Werror=cast-qual warning will kill me.
>
>
>> #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...
>> 
>
> I somehow expected the typename T need to be given int the template arguments
> like rtl_flag_check<__typeof(RTX), __VA_ARGS__> (NAME, RTX,
>
> Could be that there is way to make that work without __typeof ?

Yeah, the typename will be deduced automatically in the example above.
I don't think we need __typeof here.

Richard


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]