Uploaded image for project: 'MariaDB Server'
  1. MariaDB Server
  2. MDEV-21907

Enable -Wconversion for InnoDB and Mariabackup

Details

    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.

      Attachments

        Issue Links

          Activity

            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:

            • code included from include/ or sql/ directories
            • generated Bison or Flex code for the internal parsers in InnoDB
            • the type mismatch for the second argument of MDL_context::acquire_lock(), global_system_variables.lock_wait_timeout

            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:

            Number of tasks for purging transaction history

            I do not intend to close this ticket before enabling -Wconversion for both clang and gcc.

            marko Marko Mäkelä added a comment - 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: code included from include/ or sql/ directories generated Bison or Flex code for the internal parsers in InnoDB the type mismatch for the second argument of MDL_context::acquire_lock() , global_system_variables.lock_wait_timeout 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: Number of tasks for purging transaction history I do not intend to close this ticket before enabling -Wconversion for both clang and gcc .

            I got a working build on GCC 10.0.1 and clang 10.0.0, and pushed it to buildbot for wider validation.

            marko Marko Mäkelä added a comment - I got a working build on GCC 10.0.1 and clang 10.0.0, and pushed it to buildbot for wider validation.

            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.

            marko Marko Mäkelä added a comment - 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.
            Eric_Herman Eric Herman added a comment -

            Follow-on to do this globally: https://jira.mariadb.org/browse/MDEV-26896

            Eric_Herman Eric Herman added a comment - Follow-on to do this globally: https://jira.mariadb.org/browse/MDEV-26896

            People

              marko Marko Mäkelä
              marko Marko Mäkelä
              Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved:

                Git Integration

                  Error rendering 'com.xiplink.jira.git.jira_git_plugin:git-issue-webpanel'. Please contact your Jira administrators.