[python] gcc-warnings

Hi ! I'm experimenting with the Boost.Python bindings and get the warning: boost/python/detail/unwind_type.hpp:152: warning: unused parameter 'p' when building my extension library with gcc. Full output "errort.txt" attached. My gcc is: gcc -v gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4) The attached patch "unwind_type.hpp.diff" fixes the problem for the "not msvc" case. All Boost.Python tests pass. The interesting thing is that this warning is not produced by _any_ of the existing test case. Question: Ok to commit ? Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !

on Sat Dec 01 2007, Juergen Hunold <juergen.hunold-AT-ivembh.de> wrote:
I'm experimenting with the Boost.Python bindings and get the warning:
boost/python/detail/unwind_type.hpp:152: warning: unused parameter 'p'
when building my extension library with gcc. Full output "errort.txt" attached. My gcc is: gcc -v gcc version 4.1.2 (Ubuntu 4.1.2-0ubuntu4)
The attached patch "unwind_type.hpp.diff" fixes the problem for the "not msvc" case. All Boost.Python tests pass.
Question: Ok to commit ?
Please do.
The interesting thing is that this warning is not produced by _any_ of the existing test case.
Well, that's pretty interesting. It would be better if your patch would also introduce *something* in one of the tests that triggers the warning, if not a whole new test unto itself. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com

Hi Dave ! On Samstag 01 Dezember 2007, David Abrahams wrote:
on Sat Dec 01 2007, Juergen Hunold <juergen.hunold-AT-ivembh.de> wrote:
Question: Ok to commit ?
Please do.
Well, I'll ask again because ...
Well, that's pretty interesting. It would be better if your patch would also introduce *something* in one of the tests that triggers the warning, if not a whole new test unto itself.
Those warning are the results of a higher compiler warning level. We use -Wextra to catch those unused parameters. Adding this flag to the requirements of the Boost.Python testsuite flagged the warning in a _lot_ of tests. And it discovered much more unused parameters. "unused_python.diff" contains a patch which removes those and adds "-Wextra" to the gcc compiler flags. I don't now if gcc versions < 4.x support this, older versions might only support the less descriptive "-W" option. And I have "fixed" those annoying warnings about missing virtual destructors, too. Please find "virtual_destructor.diff" attached. Ok to commit ? Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !

on Sat Dec 01 2007, Juergen Hunold <juergen.hunold-AT-ivembh.de> wrote:
And I have "fixed" those annoying warnings about missing virtual destructors, too. Please find "virtual_destructor.diff" attached.
Ok to commit ?
Yes please, as long as you've checked carefully to make sure that things like assert() and #ifdef don't cause those names to be needed in some other build configuration. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com

Hi Dav ! On Samstag 01 Dezember 2007, David Abrahams wrote:
on Sat Dec 01 2007, Juergen Hunold <juergen.hunold-AT-ivembh.de>
Ok to commit ?
Yes please, as long as you've checked carefully to make sure that things like assert() and #ifdef don't cause those names to be needed in some other build configuration.
Done. Revisions 41544,41549,41550 I found some more unused parameters and missing virtual d'tors while doing the full test runs. The last warning ist a BOOST_ASSERT in boost/python/converter/implicit.hpp This could be fixed by using the new BOOST_VERIFY macro instead. Patch attached. Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !

on Sat Dec 01 2007, Juergen Hunold <juergen.hunold-AT-ivembh.de> wrote:
Hi Dav !
On Samstag 01 Dezember 2007, David Abrahams wrote:
on Sat Dec 01 2007, Juergen Hunold <juergen.hunold-AT-ivembh.de>
Ok to commit ?
Yes please, as long as you've checked carefully to make sure that things like assert() and #ifdef don't cause those names to be needed in some other build configuration.
Done. Revisions 41544,41549,41550
I found some more unused parameters and missing virtual d'tors while doing the full test runs.
Uh, wait: missing virtual d'tors? Sorry, I should have paid more attention. That warning is bogus and I certainly don't want anyone "fixing" it if there are no other virtual functions in the class. And even if there are other virtual functions, I think it's a suspicious change to make. Please back out any added virtual dtors!
The last warning ist a BOOST_ASSERT in boost/python/converter/implicit.hpp
This could be fixed by using the new BOOST_VERIFY macro instead.
Patch attached.
Uh, no thanks. I *want* the test to compile away in release mode. See http://lists.boost.org/Archives/boost/2006/06/106919.php . I think one of these already exists somewhere in Boost.Python.
Index: boost/python/converter/implicit.hpp =================================================================== --- boost/python/converter/implicit.hpp (revision 41535) +++ boost/python/converter/implicit.hpp (working copy) @@ -32,7 +32,7 @@
arg_from_python<Source> get_source(obj); bool convertible = get_source.convertible(); - BOOST_ASSERT(convertible); + BOOST_VERIFY(convertible);
new (storage) Target(get_source());
_______________________________________________ Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost
-- Dave Abrahams Boost Consulting http://www.boost-consulting.com

Hi Dave ! On Sonntag 02 Dezember 2007, David Abrahams wrote:
Uh, wait: missing virtual d'tors? Sorry, I should have paid more attention. That warning is bogus and I certainly don't want anyone "fixing" it if there are no other virtual functions in the class.
No, they all have "real" virtual functions.
And even if there are other virtual functions, I think it's a suspicious change to make. Please back out any added virtual dtors!
I agree that this warning is bogus. The evil thing is that you can't disabled it, at least on gcc :-(( I've reverted revisions 41544 and 41549, rerun the tests and attached the compressed test output. The nasty thing is that it is triggered during template instation which will polute the output with even more bogus messages. Please take a closer look at both the patch and the compiler log. I'll revert the changes in the meantime. I love atomic commits ;-))
The last warning ist a BOOST_ASSERT in boost/python/converter/implicit.hpp
This could be fixed by using the new BOOST_VERIFY macro instead.
Patch attached.
Uh, no thanks. I *want* the test to compile away in release mode. See http://lists.boost.org/Archives/boost/2006/06/106919.php . I think one of these already exists somewhere in Boost.Python.
I thought that BOOST_VERIFY http://lists.boost.org/Archives/boost/2007/10/129462.php would do the "right thing" is this case. Yours template based solution might be better, so I'll take a look at it. Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !

on Sun Dec 02 2007, Juergen Hunold <juergen.hunold-AT-ivembh.de> wrote:
Hi Dave !
On Sonntag 02 Dezember 2007, David Abrahams wrote:
Uh, wait: missing virtual d'tors? Sorry, I should have paid more attention. That warning is bogus and I certainly don't want anyone "fixing" it if there are no other virtual functions in the class.
No, they all have "real" virtual functions.
And even if there are other virtual functions, I think it's a suspicious change to make. Please back out any added virtual dtors!
I agree that this warning is bogus. The evil thing is that you can't disabled it, at least on gcc :-((
I've reverted revisions 41544 and 41549, rerun the tests and attached the compressed test output. The nasty thing is that it is triggered during template instation which will polute the output with even more bogus messages. Please take a closer look at both the patch and the compiler log. I'll revert the changes in the meantime. I love atomic commits ;-))
I don't mind the patch so much if there are real virtual functions there. Do whatever you think best. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com

Hi Dave ! On Montag 03 Dezember 2007, David Abrahams wrote:
I don't mind the patch so much if there are real virtual functions there. Do whatever you think best.
Well, those tests test polymorphic behaviour, so they have to have virtual functions :-)) I have re-committed this in revision 41650. All tests compile warning-free now. Thanks ! Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !

David Abrahams:
Uh, no thanks. I *want* the test to compile away in release mode. See http://lists.boost.org/Archives/boost/2006/06/106919.php . I think one of these already exists somewhere in Boost.Python.
The expression is a simple bool variable, its disappearance in release mode doesn't buy you much. Presumably, had you wanted the test to compile away, you'd have used BOOST_ASSERT( get_source.convertible() );
Index: boost/python/converter/implicit.hpp =================================================================== --- boost/python/converter/implicit.hpp (revision 41535) +++ boost/python/converter/implicit.hpp (working copy) @@ -32,7 +32,7 @@
arg_from_python<Source> get_source(obj); bool convertible = get_source.convertible(); - BOOST_ASSERT(convertible); + BOOST_VERIFY(convertible);

on Sun Dec 02 2007, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:
David Abrahams:
Uh, no thanks. I *want* the test to compile away in release mode. See http://lists.boost.org/Archives/boost/2006/06/106919.php . I think one of these already exists somewhere in Boost.Python.
The expression is a simple bool variable, its disappearance in release mode doesn't buy you much. Presumably, had you wanted the test to compile away, you'd have used
BOOST_ASSERT( get_source.convertible() );
Index: boost/python/converter/implicit.hpp =================================================================== --- boost/python/converter/implicit.hpp (revision 41535) +++ boost/python/converter/implicit.hpp (working copy) @@ -32,7 +32,7 @@
arg_from_python<Source> get_source(obj); bool convertible = get_source.convertible(); - BOOST_ASSERT(convertible); + BOOST_VERIFY(convertible);
Good point. Okay, that change is fine. -- Dave Abrahams Boost Consulting http://www.boost-consulting.com

Hi Dave ! On Montag 03 Dezember 2007, David Abrahams wrote:
on Sun Dec 02 2007, "Peter Dimov" <pdimov-AT-pdimov.com> wrote:
David Abrahams: The expression is a simple bool variable, its disappearance in release mode doesn't buy you much. Presumably, had you wanted the test to compile away, you'd have used
BOOST_ASSERT( get_source.convertible() );
arg_from_python<Source> get_source(obj); bool convertible = get_source.convertible(); - BOOST_ASSERT(convertible); + BOOST_VERIFY(convertible);
Good point. Okay, that change is fine.
Comitted in revision 41649. Thanks ! Yours, Jürgen -- * Dipl.-Math. Jürgen Hunold ! Ingenieurgesellschaft für * voice: ++49 511 262926 57 ! Verkehrs- und Eisenbahnwesen mbH * fax : ++49 511 262926 99 ! Lister Straße 15 * juergen.hunold@ivembh.de ! www.ivembh.de * * Geschäftsführer: ! Sitz des Unternehmens: Hannover * Prof. Dr.-Ing. Thomas Siefer ! Amtsgericht Hannover, HRB 56965 * PD Dr.-Ing. Alfons Radtke !
participants (3)
-
David Abrahams
-
Juergen Hunold
-
Peter Dimov