[CDRIVER-2125] Warnings generated by headers when compiling the C++ driver Created: 12/Apr/17 Updated: 27/Oct/23 Resolved: 26/May/17 |
|
| Status: | Closed |
| Project: | C Driver |
| Component/s: | None |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Task | Priority: | Major - P3 |
| Reporter: | Samuel Rossi (Inactive) | Assignee: | A. Jesse Jiryu Davis |
| Resolution: | Works as Designed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Attachments: |
|
||||||||
| Issue Links: |
|
||||||||
| Description |
|
While investigating the warnings produced when compiling the C++ driver, we noticed that a large number of the warnings were actually in the libbson header files rather than from within our own source tree. Specifically, we found the following counts of errors when compiling with clang 3.9 on Linux and the flags `-Wall -Wextra -Wconversion -Wnarrowing -pedantic`: 555 macro expansion producing 'defined' has undefined behavior |
| Comments |
| Comment by A. Jesse Jiryu Davis [ 26/May/17 ] | ||||||||||||||||||||||||||||||
|
Closing this "works as designed", let me know if you need anything from us. | ||||||||||||||||||||||||||||||
| Comment by Samuel Rossi (Inactive) [ 17/May/17 ] | ||||||||||||||||||||||||||||||
|
I'll have to check with David and Rassi, but that works for me. | ||||||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 17/May/17 ] | ||||||||||||||||||||||||||||||
|
New idea. The problem is that the C++ driver's goals, and the flags you use, are becoming entangled with the contents of our header files. So you require us to change our header files in order to accomplish your objectives the way you want. Better for the C++ driver to only need to change its own header files in order to accomplish your goals. I suggest:
| ||||||||||||||||||||||||||||||
| Comment by Hannes Magnusson [ 17/May/17 ] | ||||||||||||||||||||||||||||||
|
Since Wall and Wextra are moving targets based on the specific compiler, language, and compiler version and language version, I suggest by starting with using the flags that we support and test for out of the box: https://github.com/mongodb/libbson/blob/master/build/autotools/MaintainerFlags.m4 Note that we disable certain flags on certain platforms: I suggest we maybe turn this ticket into an epic and sam.rossi file a ticket for each explicit flag that is required to work with a dependency from the cxx project? | ||||||||||||||||||||||||||||||
| Comment by Samuel Rossi (Inactive) [ 17/May/17 ] | ||||||||||||||||||||||||||||||
|
Sorry, I forgot to respond to Hannes's most recent comment. In the medium to long term, we're hoping to get our driver to the point where users can compile with a relatively common set of flags and not get any warnings. We'd like to at least get to this point with -Wall and -Wextra, and ideally we'd want no narrowing or conversion warnings either, although these are more for our own benefit, as I'm guessing most users don't enable those warnings flags. | ||||||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 17/May/17 ] | ||||||||||||||||||||||||||||||
| Comment by Hannes Magnusson [ 24/Apr/17 ] | ||||||||||||||||||||||||||||||
|
Funny. I didn't even know we had BSON_GNUC_CHECK_VERSION. It would have helped us when we had silly version detection bugs recently where we didn't support gcc5, because the minor version had to be more then 8! I suggest to reduce your compiler flags to flags we support when you include bson/mongoc headers. We actively test with the maintainer flags linked in prior comment, and have no current plans to add Wextra or pedantic or any other excessive levels into that mix as it, quite frankly, isn't valuable to us (see also linked ticket). Is there a specific reason you added all these flags when compiling the c++ driver? | ||||||||||||||||||||||||||||||
| Comment by Samuel Rossi (Inactive) [ 24/Apr/17 ] | ||||||||||||||||||||||||||||||
|
Here's one example of the "macro expansion" warning:
The log file itself is quite long, so it's not feasible to paste it, but I've attached it to this issue. The warnings were generated when compiling the C++ driver (which naturally needs to include the installed headers of libbson and libmongoc), so it's possible that these are only generated from a C++ compiler and won't show up when compiling the C driver directly. Unfortunately, I don't think the `--enable-maintainer-flags` flag wouldn't fix this for our builds, as the warnings will occur when we compile our driver with warning flags regardless of way we compiled libbson/libmongoc. The way I generated the log file was by using the following cmake invocation:
and then running `make 2>&1 | tee warnings.log` to capture the compilation output. To get the count of libbson errors and their types, I used the following command:
| ||||||||||||||||||||||||||||||
| Comment by A. Jesse Jiryu Davis [ 21/Apr/17 ] | ||||||||||||||||||||||||||||||
|
sam.rossi could you please paste the warning outputs and tell us how to reproduce the "macro expansion" warning? | ||||||||||||||||||||||||||||||
| Comment by Hannes Magnusson [ 17/Apr/17 ] | ||||||||||||||||||||||||||||||
|
Random sample:
I find these very unlikely to be dealt with. I don't see the the macro expansion warning, is that a c++ specific thing? | ||||||||||||||||||||||||||||||
| Comment by Hannes Magnusson [ 17/Apr/17 ] | ||||||||||||||||||||||||||||||
|
Could you include the actual warning, at least one case each, that showcases the problem? |