Compiler warnings after boost update
Hi, we compile with high compiler warnings setting (-Wall, -Wpedantic), and treat compiler warnings as errors. Recently, I had to upgrade the boost library version from 1.53 to at least 1.55. Since there was no package available, I compiled it by hand from the source code. With 1.53, everything compiled fine, but with the newer version my builds now break because the boost library generates a lot of warnings, e.g. /cots/boost-1.55/include/boost/iterator/iterator_concepts.hpp: At global scope: /cots/boost-1.55/include/boost/mpl/assert.hpp:288:54: error: use of old-style cast [-Werror=old-style-cast] boost::mpl::assert_arg( (void (*) pred)0, 1 ) \ ^ /cots/boost-1.55/include/boost/mpl/assert.hpp:59:58: note: in definition of macro ‘BOOST_MPL_AUX_ASSERT_CONSTANT’ # define BOOST_MPL_AUX_ASSERT_CONSTANT(T, expr) enum { expr } /cots/boost-1.55/include/boost/range/algorithm/equal.hpp: In function ‘bool boost::range::equal(const SinglePassRange1&, const SinglePassRange2&, BinaryPredicate)’: /cots/boost-1.55/include/boost/concept/detail/general.hpp:71:20: error: typedef ‘boost_concept_check185’ locally defined but not used [-Werror=unused-local-typedefs] BOOST_PP_CAT(boost_concept_check,__LINE__) /cots/boost-1.55/include/boost/property_tree/xml_parser.hpp:52:59: required from ‘void boost::property_tree::xml_parser::read_xml(std::basic_istream<typename Ptree::key_type::value_type>&, Ptree&, int) [with Ptree = boost::property_tree::basic_ptree<std::basic_string<char>, std::basic_string<char> >; typename Ptree::key_type::value_type = char; typename Ptree::key_type::value_type = char]’ /home/auerj/MDAF/src/commons/src/crypto/KeyFileReader.cpp:43:34: required from here /cots/boost-1.55/include/boost/property_tree/detail/rapidxml.hpp:1204:33: error: zero as null pointer constant [-Werror=zero-as-null-pointer-constant] if (first_attribute()) ^ /cots/boost-1.55/include/boost/property_tree/detail/rapidxml.hpp:1204:33: error: zero as null pointer constant [-Werror=zero-as-null-pointer-constant] /cots/boost-1.55/include/boost/property_tree/detail/rapidxml.hpp:1211:45: error: zero as null pointer constant [-Werror=zero-as-null-pointer-constant] These are just some examples, there are literally hundreds of warnings. It is enough to just include ptree.hpp, but other headers have the same issue. I am wondering why this came up after the update. Is it a configuration problem during the build of the boost libraries, or is it an issue in the boost libraries? I find it hard to believe that the code quality changed so much between the versions. What is the policy of the boost project concerning compiler warnings in installation headers? I would assume that the libraries should compile without warnings on supported compilers, either because there are no issues, or because warnings are disabled in the boost headers. Best wishes, Jens -- Jens Auer | CGI | Software-Engineer CGI (Germany) GmbH & Co. KG Rheinstraße 95 | 64295 Darmstadt | Germany T: +49 6151 36860 154 jens.auer@cgi.com<mailto:jens.auer@cgi.com> Unsere Pflichtangaben gemäß § 35a GmbHG / §§ 161, 125a HGB finden Sie unter de.cgi.com/pflichtangaben<http://de.cgi.com/pflichtangaben>. CONFIDENTIALITY NOTICE: Proprietary/Confidential information belonging to CGI Group Inc. and its affiliates may be contained in this message. If you are not a recipient indicated or intended in this message (or responsible for delivery of this message to such person), or you think for any reason that this message may have been addressed to you in error, you may not use or copy or deliver this message to anyone else. In such case, you should destroy this message and are asked to notify the sender by reply e-mail.
On 22 September 2015 at 12:52, Auer, Jens wrote:
Hi,
we compile with high compiler warnings setting (-Wall, -Wpedantic), and treat compiler warnings as errors. Recently, I had to upgrade the boost library version from 1.53 to at least 1.55. Since there was no package available, I compiled it by hand from the source code. With 1.53, everything compiled fine, but with the newer version my builds now break because the boost library generates a lot of warnings, e.g. /cots/boost-1.55/include/boost/iterator/iterator_concepts.hpp: At global scope: /cots/boost-1.55/include/boost/mpl/assert.hpp:288:54: error: use of old-style cast [-Werror=old-style-cast] boost::mpl::assert_arg( (void (*) pred)0, 1 ) \ ^ /cots/boost-1.55/include/boost/mpl/assert.hpp:59:58: note: in definition of macro ‘BOOST_MPL_AUX_ASSERT_CONSTANT’ # define BOOST_MPL_AUX_ASSERT_CONSTANT(T, expr) enum { expr }
/cots/boost-1.55/include/boost/range/algorithm/equal.hpp: In function ‘bool boost::range::equal(const SinglePassRange1&, const SinglePassRange2&, BinaryPredicate)’: /cots/boost-1.55/include/boost/concept/detail/general.hpp:71:20: error: typedef ‘boost_concept_check185’ locally defined but not used [-Werror=unused-local-typedefs] BOOST_PP_CAT(boost_concept_check,__LINE__)
/cots/boost-1.55/include/boost/property_tree/xml_parser.hpp:52:59: required from ‘void boost::property_tree::xml_parser::read_xml(std::basic_istream<typename Ptree::key_type::value_type>&, Ptree&, int) [with Ptree = boost::property_tree::basic_ptree<std::basic_string<char>, std::basic_string<char> >; typename Ptree::key_type::value_type = char; typename Ptree::key_type::value_type = char]’ /home/auerj/MDAF/src/commons/src/crypto/KeyFileReader.cpp:43:34: required from here /cots/boost-1.55/include/boost/property_tree/detail/rapidxml.hpp:1204:33: error: zero as null pointer constant [-Werror=zero-as-null-pointer-constant] if (first_attribute()) ^ /cots/boost-1.55/include/boost/property_tree/detail/rapidxml.hpp:1204:33: error: zero as null pointer constant [-Werror=zero-as-null-pointer-constant] /cots/boost-1.55/include/boost/property_tree/detail/rapidxml.hpp:1211:45: error: zero as null pointer constant [-Werror=zero-as-null-pointer-constant]
These are just some examples, there are literally hundreds of warnings. It is enough to just include ptree.hpp, but other headers have the same issue.
I am wondering why this came up after the update. Is it a configuration problem during the build of the boost libraries, or is it an issue in the boost libraries? I find it hard to believe that the code quality changed so much between the versions.
What is the policy of the boost project concerning compiler warnings in installation headers? I would assume that the libraries should compile without warnings on supported compilers, either because there are no issues, or because warnings are disabled in the boost headers.
If only. I believe it's up to individual library authors to decide what their policy is regarding warnings.
-----Original Message----- From: Boost [mailto:boost-bounces@lists.boost.org] On Behalf Of Jonathan Wakely Sent: 22 September 2015 22:22 To: boost@lists.boost.org Subject: Re: [boost] Compiler warnings after boost update
On 22 September 2015 at 12:52, Auer, Jens wrote:
Hi,
we compile with high compiler warnings setting (-Wall, -Wpedantic), and treat compiler warnings as errors.
I doubt if 'warnings as errors' is a realistic target.
I am wondering why this came up after the update. Is it a configuration problem during the build of the boost libraries, or is it an issue in the boost libraries? I find it hard to believe that the code quality changed so much between the versions.
Compilers change too... <snip>
What is the policy of the boost project concerning compiler warnings in installation headers? I
would assume that the libraries should compile without warnings on supported compilers, either because there are no issues, or because warnings are disabled in the boost headers.
If only.
I believe it's up to individual library authors to decide what their policy is regarding warnings.
There are some suggestions and recommendations at https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines but ultimately it's up to authors to try to get things as warning free as possible. Sadly, some are better than others :-( Paul PS You might find that you can use the suggested methods above to allow you to produce your own warning-free version? You could then provide a pull-request to 'fix' warnings for others. --- Paul A. Bristow Prizet Farmhouse Kendal UK LA8 8AB +44 (0) 1539 561830
I am wondering why this came up after the update. Is it a configuration
Jens wrote: problem during the build >of the boost libraries, or is it an issue in the boost libraries? I find it hard to believe that >the code quality changed so much between the versions. I'm sure it's a configuration change on your side. All Boost versions I've compiled emit at least some warnings when compiling at pedantic/wall/wextra levels.
What is the policy of the boost project concerning compiler warnings in installation headers? I >would assume that the libraries should compile without warnings on supported compilers, either >because there are no issues, or because warnings are disabled in the boost headers.
Locally we've taken the view to treat Boost as system headers ( -Isystem rather than -I ) and close our eyes to the warnings to enable us to compile our code at our chosen warning settings.
I am wondering why this came up after the update. Is it a configuration problem during the build >of the boost libraries, or is it an issue in the boost libraries? I find it hard to believe that >the code quality changed so much between the versions.
I'm sure it's a configuration change on your side. All Boost versions I've compiled emit at least some warnings when compiling at pedantic/wall/wextra levels. I managed to find the difference between the boost package in centos and an installed vanilla version. The centos source rpm contains about 40 patches which also fix some of the warnings. I think these would be good candidates to integrate them into the boost main branch.
If there is general interest, I would ask the package maintainer if they want to create a pull Request, or create one myself.
Locally we've taken the view to treat Boost as system headers ( -Isystem rather than -I ) and close our eyes to the warnings to enable us to compile our code at our chosen warning settings. Thanks for hint. I have also taken this road now, but it feels not right. IMHO, the boost Libraries should be warning free, either because there are no warnings in the supported compilers, or because they are suppressed. I think the current policy on warnings should be changed to demand that changed and added public API code compiles warning free on all supported compilers with the highest settings available. Currently, if I were to fix the warnings, the next commit could introduce new warnings, or even change my fixes and create other warnings. To have some incremental improvement, I would suggest to define the current state as a baseline, and enforce that new code does not add warnings, and that changes are warning free. This can be automated easily, e.g. with SonarQube. This way, warnings should be eliminated in the long run.
Best wishes, Jens -- Dr. Jens Auer | CGI | Software Engineer CGI Deutschland Ltd. & Co. KG Rheinstraße 95 | 64295 Darmstadt | Germany T: +49 6151 36860 154 jens.auer@cgi.com Unsere Pflichtangaben gemäß § 35a GmbHG / §§ 161, 125a HGB finden Sie unter de.cgi.com/pflichtangaben. CONFIDENTIALITY NOTICE: Proprietary/Confidential information belonging to CGI Group Inc. and its affiliates may be contained in this message. If you are not a recipient indicated or intended in this message (or responsible for delivery of this message to such person), or you think for any reason that this message may have been addressed to you in error, you may not use or copy or deliver this message to anyone else. In such case, you should destroy this message and are asked to notify the sender by reply e-mail.
On 24-Sep-15 1:07 PM, Auer, Jens wrote:
I think the current policy on warnings should be changed to demand that changed and added public API code compiles warning free on all supported compilers with the highest settings available
Jens, I am afraid this is not practical: - Compilers often produce spurious warnings, and working around such spurious warnings is not necessary the best time investment, at least for library authors. - Compilers often produce new spurious warnings in new versions. - Even if a warning is sensible, addressing them might not be best time investment. E.g. clang 7.0 started to warn about missing 'override' - which is sensible, but changing all existing code just because a new compiler release is out is pointless. - Even if two people would agree on a policy, it's unlikely a wide agreement will be reached - Even if everybody agree on policy, I doubt anybody would invest time in tracking new warnings across all libraries and compilers and pinging library authors to fix them. The recommendation to include Boost using -isystem, above, seems a sensible one to me. - Volodya
-----BEGIN PGP SIGNED MESSAGE----- Hash: SHA256
What is the policy of the boost project concerning compiler warnings in installation headers? I would assume that the libraries should compile without warnings on supported compilers, either because there are no issues, or because warnings are disabled in the boost headers.
i can only invite those people who experience these warnings to contribute patches to silence them: if warning-free headers are important for your project, it is best to test new boost versions early (maybe even checkouts of the develop branch) and submit pull requests to the specific libraries before they are released ... -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBCAAGBQJWAqwTAAoJEAIkvWiom07DRhgP/1njqKrsmWfuT8C4f3afnrIM cI6RejwJ19twXq/jokGC5ThL6KzqKKhX4qvScP63mbSAHKBCiTTmv1nzz9yfDLpC 0Iyy3jZ1yzm+hWC+WVHEwpO9V/2ziUPKJ2dKCv6asX9rR5GkWcUe7DLDMAIPntWR b6HeTpBDEcl8KYBjrrz93MpmlIJCwTUhO8mqkUD/soU7woAb4tRLLn7r/9qxvch1 JyNQr38hWQqOjs1es8QBa49sLVD0/6EBkEqBROaLojL8dPO9LKYX2HjXLipPia+6 MY1CUQjYF35TLNVTt1Nh96hWPFktru0iUI/nCJWP/xAIiBhtd0+jhTGaDBW8thL/ cqC6DmzHFqFSSIwHyMls8jauezTj+mLpC+JcgJBKKNQ9B9T7+CfFOOdlU8BOzMFm PPBaDLUYVoFomwyg32lx3/ZIuMDfBwQdc6YSLx/MSwms7hn7URGmqq7TOZDWaNdo DMgNf6EWDhX2PwxZWuY2udsX0CY4GjWJGCKLChVTo9KG/OsO3vW5y3zQXL86OOLE hD/hqGYYiZ7xuAhivcqJ0gsKfRjG6/6l886CIBkzzohG+Z4L9RnZ6agt/LPTF1W4 ePuPfvow9b9CgAK0lRbtYeiXZ5TV79tTZ7ClQhfUB1DkdFJhWU7qVl56mECxn381 zvAkRKmln+NMyIFd7zOq =0fU/ -----END PGP SIGNATURE-----
i can only invite those people who experience these warnings to contribute patches to silence them: if warning-free headers are important for your project, it is best to test new boost versions early (maybe even checkouts of the develop branch) and submit pull requests to the specific libraries before they are released ...
Without a general consensus on warning handling policy, this would become a sysiphus job. Let's say I fix the warnings which currently represent problems in my project. During the development of the next release, other people commit new code with new warnings, or may even change the fixes to emit new warnings. I think there needs be general policy that code should not introduce new warnings, and that changed code is warning free. At my job, we have a similar problem with a legacy system that contains thousands of warnings. We use SonarQube to monitor the warnings, and to enforce that changes are warnings-free. Cheers, Jens
On 23.09.2015 17:52, Auer, Jens wrote:
i can only invite those people who experience these warnings to contribute patches to silence them: if warning-free headers are important for your project, it is best to test new boost versions early (maybe even checkouts of the develop branch) and submit pull requests to the specific libraries before they are released ...
Without a general consensus on warning handling policy, this would become a sysiphus job. Let's say I fix the warnings which currently represent problems in my project. During the development of the next release, other people commit new code with new warnings, or may even change the fixes to emit new warnings. I think there needs be general policy that code should not introduce new warnings, and that changed code is warning free.
IMO, trying to be warning-free on the multitude compilers Boost supports is what can be called a sysiphus job. Not only this is tedious and practically inachievable, it actually makes the code worse - typically, in terms of maintainability, sometimes performance. Striving for no warnings at the highest levels gives false sense of purity but really nothing more than that. But, if you're really interested in keeping Boost warning-free, you should put some effort into it. Run tests regularly, monitor changes, submit pull requests. Nothing is going to change unless someone interested acts.
But, if you're really interested in keeping Boost warning-free, you should put some effort into it. Run tests regularly, monitor changes, submit pull requests. Nothing is going to change unless someone interested acts. As I said, I think that there should be a monitoring system, together with an accepted general warning guideline. The right place to put this would be the boost build server. I will ask the package providers to create a pull request with their changes, and if they Don't have time to do so, I will create one.
Isn't it kind of ironic that every serious coding standard emphasizes that code should compile warning-free at the highest levels, but then the Boost libraries do not appraise this? Although I don't think that the quality is an issue, but it sets an interesting example. What I really don't like is that Boost makes it impossible for my code to compile the warning settings I want. What would you think of a STL delivered with your compiler that generates warnings? Cheers, Jens
On 25.09.2015 09:43, Auer, Jens wrote:
But, if you're really interested in keeping Boost warning-free, you should put some effort into it. Run tests regularly, monitor changes, submit pull requests. Nothing is going to change unless someone interested acts. As I said, I think that there should be a monitoring system, together with an accepted general warning guideline.
I don't think we'll be able to reach agreement on such a guideline.
The right place to put this would be the boost build server.
There is no central Boost build server. Every tester (a person or organization that runs Boost tests) builds Boost himself with the compiler and settings he chooses.
On 9/25/2015 3:43 AM, Auer, Jens wrote:
What I really don't like is that Boost makes it impossible for my code to compile the warning settings I want. What would you think of a STL delivered with your compiler that generates warnings?
It's likely that standard library implementations would generate warnings if not masked with `-isystem` or similar. For instance, libc++ only very recently started to care about warnings, nobody ever sees them due to `#pragma system_header`. Lots of warnings are of the form "you did this perfectly fine thing X, but are you sure you did not mean Y instead?". There tends to be a lot of these in implementations like Boost, the STL, and the standard library. "Did you forget to use this alias?" No, it's triggering a template instantiation that will cause a substitution failure if the arguments violate a precondition. I remember a particular piece of code that exploited properties of the comma operator, it was a single line that handled all possible input cases. Obviously the compiler suspected, "your code appears to be doing exactly what you meant it to do by abusing the comma operator, it's probably wrong", and it suspected so strongly that there was no way to shut down the warning at all while keeping use of the comma operator. This particular popular compiler forced this single line of code to be split into 8 different template specializations, true errors were introduced in this pointless duplication process. Not every warning is valuable, some are just noise, some only attempt to restrict use of the language to a particular subset, others are just simply bogus. Sometimes suppressing them is not worth the cost, sometimes they can't even be suppressed. Some warnings might be worth enforcing, turning them into errors while compiling/running tests. Blindly littering the code with `#pragma`s to silence noise doesn't really help, specially when you already have `-isystem`. Regards, -- Agustín K-ballo Bergé.- http://talesofcpp.fusionfenix.com
IMO, trying to be warning-free on the multitude compilers Boost supports is what can be called a sysiphus job. Not only this is tedious and practically inachievable, it actually makes the code worse - typically, in terms of maintainability, sometimes performance. Striving for no warnings at the highest levels gives false sense of purity but really nothing more than that.
But, if you're really interested in keeping Boost warning-free, you should put some effort into it. Run tests regularly, monitor changes, submit pull requests. Nothing is going to change unless someone interested acts.
This is tricky: I do try and keep my stuff warning free - but every new compiler release introduces warnings for things that weren't considered issues before (meanwhile things that I feel really should be warnings - like incorrect use of noexcept specifications remain curiously diagnostic free). Then you have to ask - which compiler? MSVC and GCC warnings are generally fixable - though I have certainly come up against warnings that seem to be unsilenceable no matter what you do. Intel is another matter - this generally produces pages and pages of "remarks" that aren't even warnings as such. BTW we do have some guidance on this: https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines And a now completely out of date effort to make Boost warning free: https://svn.boost.org/trac/boost/wiki/WarningFixes This might actually be easier to achieve now that we have PR's available as a tool. But it's a big job for sure. Best, John.
Hi John,
This is tricky: I do try and keep my stuff warning free - but every new compiler release introduces warnings for things that weren't considered issues before (meanwhile things that I feel really should be warnings - like incorrect use of noexcept specifications remain curiously diagnostic free). Then you have to ask - which compiler? MSVC and GCC warnings are generally fixable - though I have certainly come up against warnings that seem to be unsilenceable no matter what you do. Intel is another matter - this generally produces pages and pages of "remarks" that aren't even warnings as such.
BTW we do have some guidance on this: https://svn.boost.org/trac/boost/wiki/Guidelines/WarningsGuidelines
And a now completely out of date effort to make Boost warning free: https://svn.boost.org/trac/boost/wiki/WarningFixes
Just to be clear: I am not proposing to start a huge effort to be fix all warnings. I think that is not going to happen because there are more important things to spend the time on. I can live with the Isystem work-around, but it feels fishy that a library such as Boost ignores the recommendation to compile with highest warning levels. My idea would be to 1. define a set of reference compilers and versions, e.g. gcc, msvc 2. define a warning level. The document you quoted says this should be /W4 for MSVC and -Wall -Wpedantic for gcc 3. new code and changes of old code shall be warning free w.r.t. to the above definition. This can be checked automatically. We use SonarQube for this, but it needs a central build server which somebody has to host. I thought that boost had one, but this seems not to be the case. 4. If a warning cannot be fixed, e.g. because it is not actually a problem, it can be silenced. I think that this would gradually improve the situation. Most of the warnings I've seen were about old-style casts, using 0 instead of nullptr or name shadowing.
This might actually be easier to achieve now that we have PR's available as a tool. But it's a big job for sure. Out of curiosity: What are PR's?
Cheers, Jens -- Dr. Jens Auer | CGI | Software Engineer CGI Deutschland Ltd. & Co. KG Rheinstraße 95 | 64295 Darmstadt | Germany T: +49 6151 36860 154 jens.auer@cgi.com Unsere Pflichtangaben gemäß § 35a GmbHG / §§ 161, 125a HGB finden Sie unter de.cgi.com/pflichtangaben. CONFIDENTIALITY NOTICE: Proprietary/Confidential information belonging to CGI Group Inc. and its affiliates may be contained in this message. If you are not a recipient indicated or intended in this message (or responsible for delivery of this message to such person), or you think for any reason that this message may have been addressed to you in error, you may not use or copy or deliver this message to anyone else. In such case, you should destroy this message and are asked to notify the sender by reply e-mail.
Am 25.09.2015 um 11:02 schrieb Auer, Jens:
Just to be clear: I am not proposing to start a huge effort to be fix all warnings. I think that is not going to happen because there are more important things to spend the time on. I can live with the Isystem work-around, but it feels fishy that a library such as Boost ignores the recommendation to compile with highest warning levels.
2. define a warning level. The document you quoted says this should be /W4 for MSVC and -Wall -Wpedantic for gcc
Jens, I wholeheartedly agree with on with respect to compiler warnings. In our company the policy is 'no warnings at /W4'. The last time I spent hundreds of evening hours on getting the Boost test suite warning free at that level was 1.57 and MSVC 12 plus some more hours to achive the same result with MSVC 14. This took me 5 months. In the end, I squashed thousands of mostly harmless warnings, but exhibited real errors (both in the libraries and the tests) which nobody could see in the mess before. And I've fixed quite some code which was not safe with 64 bit. You really need to run the tests compiled with MSVC for 64 bit to find this class of errors (or some other compiler where sizeof(size_t) != sizeof(T*)). Next time I'll do this again will be for 1.60. And I hope I have enough energy left then to create at least PRs for the most problematic stuff. Ciao Dani
participants (10)
-
Agustín K-ballo Bergé -
Andrey Semashev -
Auer, Jens -
Daniela Engert -
John Maddock -
Jonathan Wakely -
Paul A. Bristow -
Pete Bartlett -
Tim Blechmann -
Vladimir Prus