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

Impact of C++20 comparison on our code



    • Epic
    • Resolution: Unresolved
    • P2: Important
    • None
    • None
    • Core: Other
    • None
    • C++20 comparison @ Qt
    • b89284922 (dev), 9b130f43b (dev), 6650ae433 (6.7), aee925597 (dev)


      C++20 changed the way comparison operators work in several ways

      1. most prominently, it added three-way comparison via the spaceship operator <=> to do strcmp-like comparisions, returning, in one call, whether the LHS is less-than, equal, or greater-than, the RHS.
      2. missing relational operators can now be synthesized based on existing ones:
        1. A @ B can use B @ A (reversed order of arguments)
        2. A != B can use A == B (inverting result)
        3. A <= B and A >= B can use A < B (with the usual transformation (!(A < B) etc)
      3. relational operators can now be {{= default}}ed, and will use existing operators, or even member-wise comparison

      = default'ed Operators

      Looking at the impact of the code, = default is something we can't use as long as we support C++17, too. It cannot be hidden behind a macro to avoid C++17 compilers from seeing it, and it's not acceptable that C++20 code should be able to use comparisons on types that C++17 code cannot, unless, of course, the type is C++20-only in the first place.

      Synthesized Operators

      The synthesizing of relational operators creates two sets of changes:

      1. where Qt implements asymmetric versions of relational operators, a C++20 compiler will flag, either as warning or as a hard error, ambiguities between the user-provided and the C++20-synthesized versions of these operators. We've already had several of these (QJson classes as well as QWaylandBufferRef.
      2. where Qt implements incomplete sets of relational operators, a C++20 compiler will permit more user code to compile than a C++17 one

      The first point is something that we need to fix immediately. We've found and fixed several of these issues already, but these were incidental: we found them, because our code used such comparisons. Since the ambiguity is only detected at call-time, we cannot assume that the absence of further compiler warnings or errors means there aren't more of these broken relational operator left in Qt. Absent corresponding static-analysis tools, we need exhaustive testing of all combinations of supposed-to-be-supported relational operators (incl. const/non-const, lvalue, rvalue!), while including all of potential supplicants of such operators (looking at you, QString) into the test TU (in practice: use include <QtModule> includes in these tests). New QTBUG-98873 would come in handy here.

      The second point is less severe, because it only affects code that was initially written in C++20 mode, but then is compiled for C++17. We might reasonably declare this unsupported, or we might take this opportunity and catalogue and document the sets of comparisons that are supposed to be supported (as opposed to working by chance), and then, much like above, writing (a little less) exhaustive tests for these.

      Spaceship Operator

      We should eventually also define the spaceship operator for all Qt types that support op<, too (we might = delete it for the other Qt types). This requires, in addition to the requirements for dealing with Synthesized Operators (ie. docs and tests), that we have, at least,

      1. QTestLib support for the spaceship operator (QCOMPARE_3WAY) and
      2. for each pair of types that are comparable:
        1. a private compare as a back-end for <=> that can be out-of-line (cf. QTBUG-100276)
        2. a classification (incl. user docs!) into exactly one of
          1. strong
          2. weak
          3. partial


        Issue Links

          For Gerrit Dashboard: QTBUG-103757
          # Subject Branch Project Status CR V



              cnn Qt Core & Network
              mmutz Marc Mutz
              Vladimir Minenko Vladimir Minenko
              Alex Blasche Alex Blasche
              2 Vote for this issue
              4 Start watching this issue