Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Narrower data type and sign conversion warnings in qt_sinks.h #3321

Open
davidebeatrici opened this issue Jan 16, 2025 · 5 comments
Open

Narrower data type and sign conversion warnings in qt_sinks.h #3321

davidebeatrici opened this issue Jan 16, 2025 · 5 comments

Comments

@davidebeatrici
Copy link

include/spdlog/sinks/qt_sinks.h: In instantiation of ‘void spdlog::sinks::qt_color_sink<Mutex>::sink_it_(const spdlog::details::log_msg&) [with Mutex = spdlog::details::null_mutex]’:
include/spdlog/sinks/qt_sinks.h:150:10:   required from here
  150 |     void sink_it_(const details::log_msg &msg) override {
      |          ^~~~~~~~
include/spdlog/sinks/qt_sinks.h:163:71: error: conversion to ‘qsizetype’ {aka ‘long long int’} from ‘size_t’ {aka ‘long unsigned int’} may change the sign of the result [-Werror=sign-conversion]
  163 |                 color_range_start = QString::fromUtf8(str.data(), msg.color_range_start).size();
      |                                                                   ~~~~^~~~~~~~~~~~~~~~~
include/spdlog/sinks/qt_sinks.h:163:94: error: conversion from ‘qsizetype’ {aka ‘long long int’} to ‘int’ may change value [-Werror=conversion]
  163 |                 color_range_start = QString::fromUtf8(str.data(), msg.color_range_start).size();
      |                                     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
include/spdlog/sinks/qt_sinks.h:164:69: error: conversion to ‘qsizetype’ {aka ‘long long int’} from ‘size_t’ {aka ‘long unsigned int’} may change the sign of the result [-Werror=sign-conversion]
  164 |                 color_range_end = QString::fromUtf8(str.data(), msg.color_range_end).size();
      |                                                                 ~~~~^~~~~~~~~~~~~~~
include/spdlog/sinks/qt_sinks.h:164:90: error: conversion from ‘qsizetype’ {aka ‘long long int’} to ‘int’ may change value [-Werror=conversion]
  164 |                 color_range_end = QString::fromUtf8(str.data(), msg.color_range_end).size();
      |                                   ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~
@gabime
Copy link
Owner

gabime commented Jan 16, 2025

Which compiler and version?

@davidebeatrici
Copy link
Author

davidebeatrici commented Jan 17, 2025

Sorry for the delay.

gcc (GCC) 14.2.0 20240801 (OpenMandriva)

With Clang:

OpenMandriva 19.1.6-1 clang version 19.1.6 (/builddir/build/BUILD/llvm-19.1.6-build/llvm-project-llvmorg-19.1.6/clang 1cc1c8b2641ccba94c7b90b5e4c4e500729f7556)
include/spdlog/sinks/qt_sinks.h:163:71: error: implicit conversion changes signedness: 'size_t' (aka 'unsigned long') to 'qsizetype' (aka 'long long') [-Werror,-Wsign-conversion]
  163 |                 color_range_start = QString::fromUtf8(str.data(), msg.color_range_start).size();
      |                                     ~~~~~~~                       ~~~~^~~~~~~~~~~~~~~~~
include/spdlog/sinks/qt_sinks.h:164:69: error: implicit conversion changes signedness: 'size_t' (aka 'unsigned long') to 'qsizetype' (aka 'long long') [-Werror,-Wsign-conversion]
  164 |                 color_range_end = QString::fromUtf8(str.data(), msg.color_range_end).size();
      |                                   ~~~~~~~                       ~~~~^~~~~~~~~~~~~~~
include/spdlog/sinks/qt_sinks.h:163:71: error: implicit conversion changes signedness: 'size_t' (aka 'unsigned long') to 'qsizetype' (aka 'long long') [-Werror,-Wsign-conversion]
  163 |                 color_range_start = QString::fromUtf8(str.data(), msg.color_range_start).size();
      |                                     ~~~~~~~                       ~~~~^~~~~~~~~~~~~~~~~
include/spdlog/sinks/qt_sinks.h:68:5: note: in instantiation of member function 'spdlog::sinks::qt_color_sink<spdlog::details::null_mutex>::sink_it_' requested here
   68 |     qt_color_sink(QTextEdit *qt_text_edit,
      |     ^
include/spdlog/sinks/qt_sinks.h:164:69: error: implicit conversion changes signedness: 'size_t' (aka 'unsigned long') to 'qsizetype' (aka 'long long') [-Werror,-Wsign-conversion]
  164 |                 color_range_end = QString::fromUtf8(str.data(), msg.color_range_end).size();
      |                                   ~~~~~~~                       ~~~~^~~~~~~~~~~~~~~
include/spdlog/sinks/qt_sinks.h:174:45: error: implicit conversion changes signedness: 'const level::level_enum' to 'size_type' (aka 'unsigned long') [-Werror,-Wsign-conversion]
  174 |                              colors_.at(msg.level),  // color to apply
      |                                      ~~ ~~~~^~~~~
include/spdlog/sinks/qt_sinks.h:163:90: error: implicit conversion loses integer precision: 'qsizetype' (aka 'long long') to 'int' [-Werror,-Wshorten-64-to-32]
  163 |                 color_range_start = QString::fromUtf8(str.data(), msg.color_range_start).size();
      |                                   ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~
include/spdlog/sinks/qt_sinks.h:164:86: error: implicit conversion loses integer precision: 'qsizetype' (aka 'long long') to 'int' [-Werror,-Wshorten-64-to-32]
  164 |                 color_range_end = QString::fromUtf8(str.data(), msg.color_range_end).size();
      |                                 ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~~

@tt4g
Copy link
Contributor

tt4g commented Jan 17, 2025

This is due to the design of the qsizetype used for the size of the QString being a signed integer.
However, the Qt documentation states that “This type is guaranteed to be the same size as a size_t on all platforms supported by Qt.”: (https://doc.qt.io/qt-6/qttypes.html#qsizetype-typedef)

Integral type providing Posix' ssize_t for all platforms.

This type is guaranteed to be the same size as a size_t on all platforms supported by Qt.

Note that qsizetype is signed. Use size_t for unsigned values.

In order to print values of this type by using formatted-output facilities such as printf(), qDebug(), QString::asprintf() and so on, you can use the PRIdQSIZETYPE and PRIiQSIZETYPE macros as format specifiers. They will both print the value as a base 10 number.

@davidebeatrici If you trust Qt, you can avoid the warning by sending a PR that casts two types with static_cast.

@davidebeatrici
Copy link
Author

This is due to the design of the qsizetype used for the size of the QString being a signed integer. However, the Qt documentation states that “This type is guaranteed to be the same size as a size_t on all platforms supported by Qt.”: (doc.qt.io/qt-6/qttypes.html#qsizetype-typedef)

The same size, not the same type. The documentation is correct.

Qt's size type used to be a 32 bit signed integer (int) until Qt 6, now it's a 64 bit signed integer (ssize_t).

If you trust Qt, you can avoid the warning by sending a PR that casts two types with static_cast.

Yeah. Let's wait for @gabime's feedback though.

@gabime
Copy link
Owner

gabime commented Jan 17, 2025

PR is welcome. But please check sure it won't crash if the casted size_t is huge and casts to negative

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants