On 8/28/16, Prathamesh Kulkarni <prathamesh.kulka...@linaro.org> wrote:
> On 26 August 2016 at 21:25, Jason Merrill <ja...@redhat.com> wrote:
>> On Fri, Aug 26, 2016 at 7:39 AM, Prathamesh Kulkarni
>> <prathamesh.kulka...@linaro.org> wrote:
>>> However with C++FE it appears TYPE_RESTRICT is not set for the
>>> parameters (buf and fmt)
>>> and hence the warning doesn't get emitted for C++.
>>> C FE sets TYPE_RESTRICT for them. I am not sure how to workaround this
>>> issue, and would be grateful for suggestions.
>>
>> In the C++ FE you can see TYPE_RESTRICT on the types of the
>> DECL_ARGUMENTS of the function.
> Thanks for the suggestions, I could indeed see TYPE_RESTRICT set on
> DECL_ARGUMENTS.
> The attached patch warns for both C and C++ with -Wrestrict, and I
> have put it under -Wall.
> I suppose we don't want to warn for null pointer args ?
> for eg:
> void f(int *restrict x, int *y);
>
> void foo(void)
> {
>   f(NULL, NULL) ?
> }
>
> However I suppose warning for pointer constants other than zero is desirable
> ?
> I am not sure whether restrict is valid for obj-c/obj-c++, so I have
> just kept it to C and C++
> in the patch. Should the warning also be included for obj-c and obj-c++ FE's
> ?
> Boostrapped and tested on x86_64-unknown-linux-gnu.
> OK to commit ?
>
> Thanks,
> Prathamesh
>


I tried this patch on my fork of gdb-binutils and got a few warnings
from it. Would it be possible to have the caret point to the argument
mentioned, instead of the function name? And also print the option
name? E.g., instead of the current:

or32-opc.c: In function ‘or32_print_register’:
or32-opc.c:956:3: warning: passing argument 1 to restrict qualified
parameter aliases with argument 3
   sprintf (disassembled, "%sr%d", disassembled, regnum);
   ^~~~~~~

could it look like:

or32-opc.c: In function ‘or32_print_register’:
or32-opc.c:956:3: warning: passing argument 1 to restrict qualified
parameter aliases with argument 3 [-Wrestrict]
   sprintf (disassembled, "%sr%d", disassembled, regnum);
            ^~~~~~~~~~~~

instead?
Just an idea. Either way, I hope this patch goes in.
Thanks,
Eric

Reply via email to