Uploaded image for project: 'Qt'
  1. Qt
  2. QTBUG-135627

Clean up QVariant floating point -> integral conversions

    XMLWordPrintable

Details

    • Task
    • Resolution: Unresolved
    • Not Evaluated
    • None
    • None
    • None

    Description

      While fixing QTBUG-135285 it emerged that the current QVariant behavior when converting floating points to integral types is un(der)documented.

      • NaNs will return 0 and fail the conversion since https://codereview.qt-project.org/c/qt/qtbase/+/635411 . Before, it was UB. In practice, before the patch, the conversion reported success , and calculated an intermediate qint64 value of LLONG_MIN (at least on x86-64, assuming e.g. that the conversion was dispatched through https://www.felixcloutier.com/x86/cvttsd2si . I have no idea about other CPU architectures.)
      • Other values get rounded and converted to an intermediate qint64 with saturation; the conversion always succeeds. Before the above patch, this was UB due qRound64 overflowing. In practice again the conversion reported success, but may have produced "interesting" results, like LLONG_MIN even for a big positive double value .

      This intermediate qint64 gets then converted to the target integral type. This has well-defined wraparound behavior. Still, the result is surprising:

      • 2147483648.0 (positive), converted to an int, results in -2147483648 (negative).
      • 0x1.1p63, converted to unsigned long long, results in (LLONG_MAX+1) because we've only saturated as a signed int64, losing range;
      • -1.0 towards an unsigned type results in that type's maximal

      Should we change this behavior and always do saturation rounding towards the target type?

      I'm very scared that this will break a lot of code in subtle ways, so let's discuss this.

      Attachments

        No reviews matched the request. Check your Options in the drop-down menu of this sections header.

        Activity

          People

            thiago Thiago Macieira
            peppe Giuseppe D'Angelo
            Votes:
            0 Vote for this issue
            Watchers:
            5 Start watching this issue

            Dates

              Created:
              Updated:

              Gerrit Reviews

                There are no open Gerrit changes