Re: [PATCH] handle enumerated types in -Wformat-overflow (PR 80397)

[Jeff Law <law at redhat dot com>]

On 05/08/2017 08:38 PM, Martin Sebor wrote:
> On 04/28/2017 12:35 PM, Jeff Law wrote:
>> On 04/26/2017 11:05 AM, Martin Sebor wrote:
>>> On 04/24/2017 03:35 PM, Jeff Law wrote:
>>>> On 04/11/2017 12:57 PM, Martin Sebor wrote:
>>>>> In a review of my fix for bug 80364 Jakub pointed out that to
>>>>> determine whether an argument to an integer directive is of
>>>>> an integer type the gimple-ssa-sprintf pass tests the type code
>>>>> for equality to INTEGER_TYPE when it should instead be using
>>>>> INTEGRAL_TYPE_P().  This has the effect of the pass being unable
>>>>> to use the available range of arguments of enumerated types,
>>>>> resulting in both false positives and false negatives, and in
>>>>> some cases, in emitting suboptimal code.
>>>>>
>>>>> The attached patch replaces those tests with INTEGRAL_TYPE_P().
>>>>>
>>>>> Since this is not a regression I submit it for GCC 8.
>>>> You might consider using POINTER_TYPE_P as well.
>>>
>>> You mean rather than (TREE_CODE (type) == POINTER_TYPE)?  Those
>>> I believe are vestiges of the %p handling that was removed sometime
>>> last year, and (unless you are recommending I remove them as part
>>> of this patch) should probably be removed during the next cleanup.Yes,
>>> that can be a follow-up cleanup.
>>
>> For the future, if you find yourself writing something like
>> TREE_CODE (TREE_TYPE (x)) == POINTER_TYPE, you're usually going to be
>> better off using POINTER_TYPE_P (TREE_TYPE (x)).  That allows the code
>> to work with C++ references as well as C pointers.
> 
> I'll keep it in mind, thanks.  Should I take this as approval
> of the patch as is or are there some changes you'd like me to
> make?
Yes, Jakub's approval still stands.  My comment was more for future
reference.

Removal of the dead code WRT %p handling seems wise, but makes the most
sense as a follow-up.

Sorry for the long delay,

jeff