[boost] Compiler warnings after boost update

[Auer, Jens]

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...@cgi.com<mailto:jens...@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.

_______________________________________________
Unsubscribe & other changes: http://lists.boost.org/mailman/listinfo.cgi/boost

[Jonathan Wakely]

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.

[Pete Bartlett]

Jens wrote:
>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.

>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.

[Tim Blechmann]

-----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-----

[Auer, Jens]

> 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

[Auer, Jens]

> >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...@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.

[Andrey Semashev]

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.

[Auer, Jens]

> 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

[Vladimir Prus]

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

[Andrey Semashev]

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.

[John Maddock]

> 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.

[Auer, Jens]

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...@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.

[Agustín K-ballo Bergé]

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

[Daniela Engert]

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

[Paul A. Bristow]

> -----Original Message-----
> From: Boost [mailto:boost-...@lists.boost.org] On Behalf Of Jonathan Wakely
> Sent: 22 September 2015 22:22
> To: bo...@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