[MDEV-21907] Enable -Wconversion for InnoDB and Mariabackup Created: 2020-03-10 Updated: 2021-10-25 Resolved: 2020-03-12 |
|
| Status: | Closed |
| Project: | MariaDB Server |
| Component/s: | Storage Engine - InnoDB |
| Affects Version/s: | 5.5, 10.0, 10.1, 10.2, 10.3, 10.4, 10.5 |
| Fix Version/s: | 10.5.2 |
| Type: | Bug | Priority: | Major |
| Reporter: | Marko Mäkelä | Assignee: | Marko Mäkelä |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | portability, ulong | ||
| Issue Links: |
|
||||||||||||||||||||||||
| Description |
|
To improve code quality and to avoid surprise build failures on Windows, we should fix any -Wconversion warnings that are reported by gcc or clang. gcc appears to be stricter than clang here; it will require explicit bitmask operations to suppress warnings about assignments to bit fields in structs. As a first step, we should enable -Wconversion in InnoDB and Mariabackup. This will require some headers in include/ and sql/ to be fixed as well. The Mariabackup build also includes some compilation units in libmysqld. |
| Comments |
| Comment by Marko Mäkelä [ 2020-03-11 ] |
|
I pushed the first wave of this to 10.5, to fix most -Wconversion that was reported for cmake --build . --target innobase code by CMAKE_CXX_COMPILER=clang++-10. This excludes the following:
As part of this, the type of innodb_purge_threads was changed to INT UNSIGNED on all platforms and the documentation string was adjusted to the following:
I do not intend to close this ticket before enabling -Wconversion for both clang and gcc. |
| Comment by Marko Mäkelä [ 2020-03-11 ] |
|
I got a working build on GCC 10.0.1 and clang 10.0.0, and pushed it to buildbot for wider validation. |
| Comment by Marko Mäkelä [ 2020-03-12 ] |
|
It turns out that GCC 10 requires far fewer type casts to silence -Wconversion than its precursors. GCC 5 is by far the worst; I ended up disabling -Wconversion with GCC pragmas for some sections of code where I was not able to suppress the warnings. With GCC 6 and later, everything worked rather reasonably. clang does not seem to warn about potential loss of data when assigning to a bit-field whose size is not 8, 16, or 32 bytes, but GCC does that, and starting with GCC 6 the warning can be suppressed by adding bitwise AND with a constant where all the bits of the bitfield are set. |
| Comment by Eric Herman [ 2021-10-25 ] |
|
Follow-on to do this globally: https://jira.mariadb.org/browse/MDEV-26896 |