Re: [Cython] GCC 4.6 unused-but-set-variable warnings
[moving this here from cython-users] Nikolaus Rath, 13.06.2011 16:59:
Stefan Behnel writes:
Nikolaus Rath, 13.06.2011 01:18:
Stefan Behnel writes:
Nikolaus Rath, 03.06.2011 23:24:
Cython 0.14 generated code triggers lots of unused-but-set-variable warnings (which are new in GCC 4.6).
Hmm, that's annoying. We actually generate a lot of useless "dangling reset to 0" statements for safety (and also as a bit of a "now unused" hint to the C compiler).
We could get rid of most of them based on control flow analysis, but I actually like having them there, just in case we accidentally generate broken code at some point. A NULL pointer is a lot safer than an arbitrarily dangling address, especially when it comes to CPython heap allocated memory (which doesn't necessarily get freed, so you may not get a quick segfault but a read from dead memory).
I don't quite understand. The way to get rid of the warnings is to remove the unnecessary variables. How can this result in a dangling address? You'll get a syntax error from gcc about using an undeclared variable.
Ah, sorry. I misunderstood the meaning of the warning message as pointing at unnecessary assignments in general. Could you post a C code snippet (and the corresponding Cython code) that triggers this? I don't have gcc 4.6 available for testing.
Sure. It's apparently triggered by unused function arguments:
$ cat test.pyx class Operations(object): def handle_exc(self, fn, exc): pass
$ cython test.pyx
$ gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -I/usr/include/python2.6 -c test.c -o test.o -D_FILE_OFFSET_BITS=64 -I/usr/include/fuse -Werror -Wall -Wextra -Wconversion -Wno-unused-parameter -Wno-sign-conversion -fno-strict-aliasing test.c: test.c:454:13: warning: variable ‘__pyx_v_exc’ set but not used [-Wunused-but-set-variable] test.c:453:13: warning: variable ‘__pyx_v_fn’ set but not used [-Wunused-but-set-variable] test.c:452:13: warning: variable ‘__pyx_v_self’ set but not used [-Wunused-but-set-variable]
http://trac.cython.org/cython_trac/ticket/704 I think this is something that the control flow analysis could deal with. Basically, every parameter that turns out to be unused in the function (and this most likely only happens with parameters) could be ignored, unless it requires a type conversion with potential side effects. In that (unlikely) case, we could still drop the final assignment to the variable itself. Stefan
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
[moving this here from cython-users]
Nikolaus Rath, 13.06.2011 16:59:
Stefan Behnel writes:
Nikolaus Rath, 13.06.2011 01:18:
Stefan Behnel writes:
Nikolaus Rath, 03.06.2011 23:24:
Cython 0.14 generated code triggers lots of unused-but-set-variable warnings (which are new in GCC 4.6).
Hmm, that's annoying. We actually generate a lot of useless "dangling reset to 0" statements for safety (and also as a bit of a "now unused" hint to the C compiler).
We could get rid of most of them based on control flow analysis, but I actually like having them there, just in case we accidentally generate broken code at some point. A NULL pointer is a lot safer than an arbitrarily dangling address, especially when it comes to CPython heap allocated memory (which doesn't necessarily get freed, so you may not get a quick segfault but a read from dead memory).
I don't quite understand. The way to get rid of the warnings is to remove the unnecessary variables. How can this result in a dangling address? You'll get a syntax error from gcc about using an undeclared variable.
Ah, sorry. I misunderstood the meaning of the warning message as pointing at unnecessary assignments in general. Could you post a C code snippet (and the corresponding Cython code) that triggers this? I don't have gcc 4.6 available for testing.
Sure. It's apparently triggered by unused function arguments:
$ cat test.pyx class Operations(object): def handle_exc(self, fn, exc): pass
$ cython test.pyx
$ gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -I/usr/include/python2.6 -c test.c -o test.o -D_FILE_OFFSET_BITS=64 -I/usr/include/fuse -Werror -Wall -Wextra -Wconversion -Wno-unused-parameter -Wno-sign-conversion -fno-strict-aliasing test.c: test.c:454:13: warning: variable ‘__pyx_v_exc’ set but not used [-Wunused-but-set-variable] test.c:453:13: warning: variable ‘__pyx_v_fn’ set but not used [-Wunused-but-set-variable] test.c:452:13: warning: variable ‘__pyx_v_self’ set but not used [-Wunused-but-set-variable]
http://trac.cython.org/cython_trac/ticket/704
I think this is something that the control flow analysis could deal with. Basically, every parameter that turns out to be unused in the function (and this most likely only happens with parameters) could be ignored, unless it requires a type conversion with potential side effects. In that (unlikely) case, we could still drop the final assignment to the variable itself.
There is entry.cf_unused flag it's set when the whole entry is not used. -- vitja.
2011/7/29 Vitja Makarov <vitja.makarov@gmail.com>:
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
[moving this here from cython-users]
Nikolaus Rath, 13.06.2011 16:59:
Stefan Behnel writes:
Nikolaus Rath, 13.06.2011 01:18:
Stefan Behnel writes:
Nikolaus Rath, 03.06.2011 23:24: > > Cython 0.14 generated code triggers lots of unused-but-set-variable > warnings (which are new in GCC 4.6).
Hmm, that's annoying. We actually generate a lot of useless "dangling reset to 0" statements for safety (and also as a bit of a "now unused" hint to the C compiler).
We could get rid of most of them based on control flow analysis, but I actually like having them there, just in case we accidentally generate broken code at some point. A NULL pointer is a lot safer than an arbitrarily dangling address, especially when it comes to CPython heap allocated memory (which doesn't necessarily get freed, so you may not get a quick segfault but a read from dead memory).
I don't quite understand. The way to get rid of the warnings is to remove the unnecessary variables. How can this result in a dangling address? You'll get a syntax error from gcc about using an undeclared variable.
Ah, sorry. I misunderstood the meaning of the warning message as pointing at unnecessary assignments in general. Could you post a C code snippet (and the corresponding Cython code) that triggers this? I don't have gcc 4.6 available for testing.
Sure. It's apparently triggered by unused function arguments:
$ cat test.pyx class Operations(object): def handle_exc(self, fn, exc): pass
$ cython test.pyx
$ gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -I/usr/include/python2.6 -c test.c -o test.o -D_FILE_OFFSET_BITS=64 -I/usr/include/fuse -Werror -Wall -Wextra -Wconversion -Wno-unused-parameter -Wno-sign-conversion -fno-strict-aliasing test.c: test.c:454:13: warning: variable ‘__pyx_v_exc’ set but not used [-Wunused-but-set-variable] test.c:453:13: warning: variable ‘__pyx_v_fn’ set but not used [-Wunused-but-set-variable] test.c:452:13: warning: variable ‘__pyx_v_self’ set but not used [-Wunused-but-set-variable]
http://trac.cython.org/cython_trac/ticket/704
I think this is something that the control flow analysis could deal with. Basically, every parameter that turns out to be unused in the function (and this most likely only happens with parameters) could be ignored, unless it requires a type conversion with potential side effects. In that (unlikely) case, we could still drop the final assignment to the variable itself.
There is entry.cf_unused flag it's set when the whole entry is not used.
((6af313c...)) vitja@vitja-laptop:~/work/cython-vitek$ python cython.py -X warn.unused_arg=True test.pyx warning: test.pyx:2:19: Unused argument 'self' warning: test.pyx:2:25: Unused argument 'fn' warning: test.pyx:2:29: Unused argument 'exc' -- vitja.
Vitja Makarov, 29.07.2011 08:38:
2011/7/29 Vitja Makarov:
2011/7/29 Stefan Behnel:
Nikolaus Rath, 13.06.2011 16:59:
$ gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -I/usr/include/python2.6 -c test.c -o test.o -D_FILE_OFFSET_BITS=64 -I/usr/include/fuse -Werror -Wall -Wextra -Wconversion -Wno-unused-parameter -Wno-sign-conversion -fno-strict-aliasing test.c: test.c:454:13: warning: variable ‘__pyx_v_exc’ set but not used [-Wunused-but-set-variable] test.c:453:13: warning: variable ‘__pyx_v_fn’ set but not used [-Wunused-but-set-variable] test.c:452:13: warning: variable ‘__pyx_v_self’ set but not used [-Wunused-but-set-variable]
http://trac.cython.org/cython_trac/ticket/704
I think this is something that the control flow analysis could deal with. Basically, every parameter that turns out to be unused in the function (and this most likely only happens with parameters) could be ignored, unless it requires a type conversion with potential side effects. In that (unlikely) case, we could still drop the final assignment to the variable itself.
There is entry.cf_unused flag it's set when the whole entry is not used.
((6af313c...)) vitja@vitja-laptop:~/work/cython-vitek$ python cython.py -X warn.unused_arg=True test.pyx warning: test.pyx:2:19: Unused argument 'self' warning: test.pyx:2:25: Unused argument 'fn' warning: test.pyx:2:29: Unused argument 'exc'
Cool. That's what I expected. :) Want to give it a try? The argument unpacking code has traditionally been my domain, and it's somewhat complex, but it should still be easy to stick this in. Note that there is more than one place where this needs to be handled, as the code is structured into different cases based on the type of signature. Otherwise, I can see if I find a bit of time over the weekend. Stefan
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 08:38:
2011/7/29 Vitja Makarov:
2011/7/29 Stefan Behnel:
Nikolaus Rath, 13.06.2011 16:59:
$ gcc -pthread -fno-strict-aliasing -DNDEBUG -g -fwrapv -O2 -Wall -Wstrict-prototypes -fPIC -I/usr/include/python2.6 -c test.c -o test.o -D_FILE_OFFSET_BITS=64 -I/usr/include/fuse -Werror -Wall -Wextra -Wconversion -Wno-unused-parameter -Wno-sign-conversion -fno-strict-aliasing test.c: test.c:454:13: warning: variable ‘__pyx_v_exc’ set but not used [-Wunused-but-set-variable] test.c:453:13: warning: variable ‘__pyx_v_fn’ set but not used [-Wunused-but-set-variable] test.c:452:13: warning: variable ‘__pyx_v_self’ set but not used [-Wunused-but-set-variable]
http://trac.cython.org/cython_trac/ticket/704
I think this is something that the control flow analysis could deal with. Basically, every parameter that turns out to be unused in the function (and this most likely only happens with parameters) could be ignored, unless it requires a type conversion with potential side effects. In that (unlikely) case, we could still drop the final assignment to the variable itself.
There is entry.cf_unused flag it's set when the whole entry is not used.
((6af313c...)) vitja@vitja-laptop:~/work/cython-vitek$ python cython.py -X warn.unused_arg=True test.pyx warning: test.pyx:2:19: Unused argument 'self' warning: test.pyx:2:25: Unused argument 'fn' warning: test.pyx:2:29: Unused argument 'exc'
Cool. That's what I expected. :)
Want to give it a try? The argument unpacking code has traditionally been my domain, and it's somewhat complex, but it should still be easy to stick this in. Note that there is more than one place where this needs to be handled, as the code is structured into different cases based on the type of signature.
Otherwise, I can see if I find a bit of time over the weekend.
I will try to find some time. Btw this issue isn't critical and even isn't a bug at all. -- vitja.
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:08:
this issue isn't critical and even isn't a bug at all.
Agreed. It's nothing that needs to be done for 0.15. I just thought you might be interested. :D
Yeah, I tried to do this once but I've found some problems with buffer variables. What to do about local variables: def foo(): a = 1 'a' is unused here -- vitja.
Vitja Makarov, 29.07.2011 10:44:
2011/7/29 Stefan Behnel:
Vitja Makarov, 29.07.2011 10:08:
this issue isn't critical and even isn't a bug at all.
Agreed. It's nothing that needs to be done for 0.15. I just thought you might be interested. :D
Yeah, I tried to do this once but I've found some problems with buffer variables.
What to do about local variables:
def foo(): a = 1
'a' is unused here
That's up to the user to fix. However, there may be restrictions regarding the signature (inheritance etc.) that the users cannot control, so unused *parameters* must not produce warnings. Stefan
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:44:
2011/7/29 Stefan Behnel:
Vitja Makarov, 29.07.2011 10:08:
this issue isn't critical and even isn't a bug at all.
Agreed. It's nothing that needs to be done for 0.15. I just thought you might be interested. :D
Yeah, I tried to do this once but I've found some problems with buffer variables.
What to do about local variables:
def foo(): a = 1
'a' is unused here
That's up to the user to fix. However, there may be restrictions regarding the signature (inheritance etc.) that the users cannot control, so unused *parameters* must not produce warnings.
Sure. Because of that there is separate warn.unused_args option :) -- vitja.
Vitja Makarov, 29.07.2011 10:55:
2011/7/29 Stefan Behnel<stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:44:
2011/7/29 Stefan Behnel:
Vitja Makarov, 29.07.2011 10:08:
this issue isn't critical and even isn't a bug at all.
Agreed. It's nothing that needs to be done for 0.15. I just thought you might be interested. :D
Yeah, I tried to do this once but I've found some problems with buffer variables.
What to do about local variables:
def foo(): a = 1
'a' is unused here
That's up to the user to fix. However, there may be restrictions regarding the signature (inheritance etc.) that the users cannot control, so unused *parameters* must not produce warnings.
Sure. Because of that there is separate warn.unused_args option :)
With the caveat that gcc 4.6 produces a warning with -Wall for them, because it cannot know that they originally were parameters in the Cython code. Stefan
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:55:
2011/7/29 Stefan Behnel<stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:44:
2011/7/29 Stefan Behnel:
Vitja Makarov, 29.07.2011 10:08:
this issue isn't critical and even isn't a bug at all.
Agreed. It's nothing that needs to be done for 0.15. I just thought you might be interested. :D
Yeah, I tried to do this once but I've found some problems with buffer variables.
What to do about local variables:
def foo(): a = 1
'a' is unused here
That's up to the user to fix. However, there may be restrictions regarding the signature (inheritance etc.) that the users cannot control, so unused *parameters* must not produce warnings.
Sure. Because of that there is separate warn.unused_args option :)
With the caveat that gcc 4.6 produces a warning with -Wall for them, because it cannot know that they originally were parameters in the Cython code.
I tried to implement this for c[p]def functions: For required args I've added CYTHON_UNUSED qualifier and removed unused optionals. -- vitja.
2011/8/2 Vitja Makarov <vitja.makarov@gmail.com>:
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:55:
2011/7/29 Stefan Behnel<stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:44:
2011/7/29 Stefan Behnel:
Vitja Makarov, 29.07.2011 10:08: > > this issue isn't critical and even isn't a bug at all.
Agreed. It's nothing that needs to be done for 0.15. I just thought you might be interested. :D
Yeah, I tried to do this once but I've found some problems with buffer variables.
What to do about local variables:
def foo(): a = 1
'a' is unused here
That's up to the user to fix. However, there may be restrictions regarding the signature (inheritance etc.) that the users cannot control, so unused *parameters* must not produce warnings.
Sure. Because of that there is separate warn.unused_args option :)
With the caveat that gcc 4.6 produces a warning with -Wall for them, because it cannot know that they originally were parameters in the Cython code.
I tried to implement this for c[p]def functions:
For required args I've added CYTHON_UNUSED qualifier and removed unused optionals.
Stefan, do you know why skip_dispatch argument is used for module-level cpdef function? There is warning about that too. -- vitja.
2011/8/2 Vitja Makarov <vitja.makarov@gmail.com>:
2011/8/2 Vitja Makarov <vitja.makarov@gmail.com>:
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:55:
2011/7/29 Stefan Behnel<stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:44:
2011/7/29 Stefan Behnel: > > Vitja Makarov, 29.07.2011 10:08: >> >> this issue isn't critical and even isn't a bug at all. > > Agreed. It's nothing that needs to be done for 0.15. I just thought you > might be interested. :D >
Yeah, I tried to do this once but I've found some problems with buffer variables.
What to do about local variables:
def foo(): a = 1
'a' is unused here
That's up to the user to fix. However, there may be restrictions regarding the signature (inheritance etc.) that the users cannot control, so unused *parameters* must not produce warnings.
Sure. Because of that there is separate warn.unused_args option :)
With the caveat that gcc 4.6 produces a warning with -Wall for them, because it cannot know that they originally were parameters in the Cython code.
I tried to implement this for c[p]def functions:
For required args I've added CYTHON_UNUSED qualifier and removed unused optionals.
Stefan, do you know why skip_dispatch argument is used for module-level cpdef function?
There is warning about that too.
I tried to fix the issue here: https://github.com/vitek/cython/commit/dc87be3f546ffe609fdce9514d939c147c99c... And here is my branch for unused_arg: https://github.com/vitek/cython/commits/_unused_args -- vitja.
2011/8/2 Vitja Makarov <vitja.makarov@gmail.com>:
2011/8/2 Vitja Makarov <vitja.makarov@gmail.com>:
2011/8/2 Vitja Makarov <vitja.makarov@gmail.com>:
2011/7/29 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:55:
2011/7/29 Stefan Behnel<stefan_ml@behnel.de>:
Vitja Makarov, 29.07.2011 10:44: > > 2011/7/29 Stefan Behnel: >> >> Vitja Makarov, 29.07.2011 10:08: >>> >>> this issue isn't critical and even isn't a bug at all. >> >> Agreed. It's nothing that needs to be done for 0.15. I just thought you >> might be interested. :D >> > > Yeah, I tried to do this once but I've found some problems with buffer > variables. > > What to do about local variables: > > def foo(): > a = 1 > > 'a' is unused here
That's up to the user to fix. However, there may be restrictions regarding the signature (inheritance etc.) that the users cannot control, so unused *parameters* must not produce warnings.
Sure. Because of that there is separate warn.unused_args option :)
With the caveat that gcc 4.6 produces a warning with -Wall for them, because it cannot know that they originally were parameters in the Cython code.
I tried to implement this for c[p]def functions:
For required args I've added CYTHON_UNUSED qualifier and removed unused optionals.
Stefan, do you know why skip_dispatch argument is used for module-level cpdef function?
There is warning about that too.
I tried to fix the issue here: https://github.com/vitek/cython/commit/dc87be3f546ffe609fdce9514d939c147c99c...
And here is my branch for unused_arg:
I've reverted skip_dispatch argument removal, that doesn't work this way and is more general issue. Now I mark all unused local entries and args with CYTHON_UNUSED. That actually isn't an optimization just some warnings removal. I think real optimization should be done when we start with DefNode refactoring. If you like that I'm gonna start pull request. -- vitja.
Vitja Makarov, 03.08.2011 21:07:
Stefan, do you know why skip_dispatch argument is used for module-level cpdef function?
There is warning about that too.
It seems you already found a way to handle it.
And here is my branch for unused_arg:
I've reverted skip_dispatch argument removal, that doesn't work this way and is more general issue. Now I mark all unused local entries and args with CYTHON_UNUSED.
Cool.
That actually isn't an optimization just some warnings removal.
I think real optimization should be done when we start with DefNode refactoring.
Absolutely.
If you like that I'm gonna start pull request.
It looks good to me, so please do. Stefan
2011/8/4 Stefan Behnel <stefan_ml@behnel.de>:
Vitja Makarov, 03.08.2011 21:07:
Stefan, do you know why skip_dispatch argument is used for module-level cpdef function?
There is warning about that too.
It seems you already found a way to handle it.
And here is my branch for unused_arg:
I've reverted skip_dispatch argument removal, that doesn't work this way and is more general issue. Now I mark all unused local entries and args with CYTHON_UNUSED.
Cool.
That actually isn't an optimization just some warnings removal.
I think real optimization should be done when we start with DefNode refactoring.
Absolutely.
If you like that I'm gonna start pull request.
It looks good to me, so please do.
Thanks, here it is https://github.com/cython/cython/pull/50 -- vitja.
participants (2)
-
Stefan Behnel -
Vitja Makarov