This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH] handle enumerated types in -Wformat-overflow (PR 80397)
- From: Jeff Law <law at redhat dot com>
- To: Martin Sebor <msebor at gmail dot com>, Gcc Patch List <gcc-patches at gcc dot gnu dot org>
- Date: Thu, 22 Jun 2017 22:43:12 -0600
- Subject: Re: [PATCH] handle enumerated types in -Wformat-overflow (PR 80397)
- Authentication-results: sourceware.org; auth=none
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; dmarc=none (p=none dis=none) header.from=redhat.com
- Authentication-results: ext-mx04.extmail.prod.ext.phx2.redhat.com; spf=pass smtp.mailfrom=law at redhat dot com
- Dkim-filter: OpenDKIM Filter v2.11.0 mx1.redhat.com BAED07D4E0
- Dmarc-filter: OpenDMARC Filter v1.3.2 mx1.redhat.com BAED07D4E0
- References: <d9207f14-6a82-7707-ebf5-c9bb5efd6623@gmail.com> <ac9ff471-c3b0-1754-2bbb-db7472cc0229@redhat.com> <14993ff5-5acb-823e-7ee7-15bed145c1e9@gmail.com> <87abef8c-3d44-62e0-ea8f-670c3dda5cb7@redhat.com> <2b92ee70-2710-fe8e-6cf9-e08cf4fba64f@gmail.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