[Qt6] Get rid of deprecated QVariant::Type

[troopa81]

This PR mainly focus on replacing deprecated QVariant::Type with QMetaType::Type, but I plan also to replace then all QVariant/QMetaType deprecated methods.

There are still things that need to be fixed (Hana provider for starter) but I would like to start discussions before going any further.

• I choose to replace QVariant::Type with QMetaType::Type to stay consistent with our API but Qt has adopted a different way and choose to use integer when it comes to type id because user values are not represented by the enum. We could do the same if we think it's better.
• type() is deprecated, we use userType() instead and cast it to QMetaType::Type only when needed, i.e. when we pass it to a method taking now a QMetaType::Type, but not when we compare QMetaType::Type and integer id.
• new QgsVariantUtils::createVariant has been introduced because QVariant creation from metatype id differs between Qt5 and Qt6
• In order to keep Python backward compatiblity, methods having a QVariant::Type argument have 2 version, a deprecated one with QVariant::Type and a new one with QMetaType::Type. Methods which return QVariant::Type or exposed struct variable (here for instance) are now returning QMetaType::Type only. Even if the returned object type has changed, it behaves the same way than before (there are integer in the end), and didn't break any tests. So I would say, this is ok.
• Occurrences of QVariant LastType have been removed

These modifications have been partially generated with the following python script.

#!/usr/bin/env python
""" Usage: call with <build_dir>
build_dir must contain compile_commands.json file
"""

import os
import sys
import pickle
import clang.cindex
from collections import defaultdict
import traceback
from colorama import Fore, Style

debugfile = None
debugline = None
debugAndReturn = False

# module = "src/3d"
module = "tests/src/providers/grass"
# module = "src/app"

# debugfile = "/home/julien/work/QGIS/tests/src/providers/grass/testqgsgrassprovider.cpp"
# debugline = 191
# debugAndReturn = False

def node_string(node, full=False, print_children=False, indent=0):

    if not node:
        return "None"
    
    tokens = node.get_tokens()
    tokens = list(tokens) if tokens else []
    fileName = node.location.file.name if node.location.file else None
    str_tokens = "".join([token.spelling for token in tokens])
    ret = f"file={os.path.basename(fileName) if fileName else None} spelling={node.spelling}, kind={node.kind}, start={node.extent.start.line},{node.extent.start.column} end={node.extent.end.line},{node.extent.end.column} tokens={Fore.BLUE}\"{str_tokens}\"{Style.RESET_ALL}"

    if full:
        ret += f" type={node.type.spelling}"
        ret += f" args={len(list(node.get_arguments()))}"
        ret += f" definition={node.get_definition().type.spelling if node.get_definition() else None}"

        ret += " others= "
        if node.is_converting_constructor():
            ret += "conv_constructor,"
        
    if print_children:
        str_indent = indent * " "
        children_str = "\n".join([f"{str_indent}* {node_string(child, full, True, indent+2)}" for child in node.get_children()]) 
        ret += ("\n" if children_str else "") + children_str
    
    return ret

# search the associated type according to given tokens
def searchType(node, tokens):
    
    def searchTypeTokens(node, parent_str_tokens):
        for child in node.get_children():
            str_tokens = [token.spelling for token in child.get_tokens()]
            if str_tokens == parent_str_tokens:
                return child.type.spelling

            type_ = searchTypeTokens(child, parent_str_tokens)
            if type_:
                return type_

        return None

    return searchTypeTokens(node, [token.spelling for token in tokens])


def find_qvariant_deprecated(to_replace, node, parents, already_parsed):
    """
    Find all deprecated call to QVariant and add them to to_replace dict
    """

    def add_to_replace(start, end, new_str):
        # if node.location.line == 87:
        #     print((start.line, start.column, end.line, end.column, new_str))
        #     print("---")
        to_replace[node.location.file.name].add((start.line, start.column, end.line, end.column, new_str))
    
    if (node.location.file and ( not node.location.file.name.startswith("/home/julien/work/QGIS/") or node.location.file.name in already_parsed )
        or node.spelling == "variantTypeToMetaType" or node.spelling == "metaTypeToVariantType" ):
        return {}

    tokens = list(node.get_tokens())
    parent = parents[0] if len(parents) else None
    
    if (debugAndReturn and debugfile and debugline and node.location.file and node.location.file.name == debugfile and node.location.line == debugline):
          print(f"-- node={node_string(node,True,True,4)}")
          return
      
    if node.kind == clang.cindex.CursorKind.MACRO_INSTANTIATION and node.spelling == "Q_PROPERTY":
        assert(tokens[0].spelling == "Q_PROPERTY")
        assert(tokens[1].spelling == "(")

        if tokens[2].spelling == "QVariant" and tokens[3].spelling == "::" and tokens[4].spelling == "Type":
            add_to_replace(tokens[2].location, tokens[4].extent.end, "QMetaType::Type")

    # replace static_cast<QVariant::Type>() with static_cast<QMetaType::Type>()
    # would have been better to mutualize with the one above but ... I don't want to break the rest
    elif ( node.kind == clang.cindex.CursorKind.CXX_STATIC_CAST_EXPR and node.type and node.type.spelling == "QVariant::Type"
           and [token.spelling for token in tokens[:7]] == ["static_cast", "<", "QVariant", "::", "Type", ">", "("] and tokens[-1].spelling == ")"):

        add_to_replace(tokens[2].extent.start, tokens[4].extent.end, "QMetaType::Type")

    # replace QList<QVariant::Type>() with QList<QMetaType::Type>()
    elif ( node.kind == clang.cindex.CursorKind.UNEXPOSED_EXPR and node.type and node.type.spelling == "QList<QVariant::Type>"
           and [token.spelling for token in tokens[:6]] == ["QList", "<", "QVariant", "::", "Type", ">"] and tokens[-1].spelling == ")"):

        add_to_replace(tokens[2].extent.start, tokens[4].extent.end, "QMetaType::Type")
        
    # replace QVariant::Type::XXX Enum with appropriate QMetaType::Type::YYY enum
    elif (tokens and node.kind == clang.cindex.CursorKind.DECL_REF_EXPR
          and tokens[0].spelling == "QVariant" and tokens[1].spelling == "::"
          and node.get_definition() and node.get_definition().kind == clang.cindex.CursorKind.ENUM_CONSTANT_DECL ):

        enum_defn = node.get_definition()
        enum_tokens = list(enum_defn.get_tokens())
        
        i=2
        if tokens[i].spelling == "Type":
            i+=1
            assert(tokens[i].spelling == "::")
            i+=1

        assert(enum_tokens[0].spelling == tokens[i].spelling)
        assert(enum_tokens[1].spelling == "=")
        assert(enum_tokens[2].spelling == "QMetaType")
        assert(enum_tokens[3].spelling == "::")

        add_to_replace(tokens[0].extent.start, tokens[i].extent.end, "QMetaType::Type::" + enum_tokens[4].spelling)
        return

    # replace QVariant::Type variable declaration with QMetaType::Type one
    elif ( node.kind == clang.cindex.CursorKind.TYPE_REF and node.type and node.type.spelling == "QVariant::Type"
           and len(tokens) == 1 and tokens[0].spelling == "Type" ):

        to_replace[node.location.file.name].add((tokens[0].extent.start.line, tokens[0].extent.start.column-len("QVariant::"),
                                                 tokens[0].extent.end.line, tokens[0].extent.end.column, "QMetaType::Type"))
        return

    # replace QVariant::type() with QVariant::userType()
    elif ( node.kind == clang.cindex.CursorKind.MEMBER_REF_EXPR and len(tokens) > 2 and tokens[-1].spelling == "type"
           and "QVariant" in list(node.get_children())[0].type.spelling
           and parent.kind == clang.cindex.CursorKind.CALL_EXPR ):

        # we need to static_cast if
        # - methodCall
        # - variable assignment
        # - unary "?:" operator
        # but no in of comparaison clause (not needed)
        needCast = False
        for p in parents[1:]:
            if (p.kind in [clang.cindex.CursorKind.CONDITIONAL_OPERATOR,
                          clang.cindex.CursorKind.VAR_DECL,
                          clang.cindex.CursorKind.CALL_EXPR] or
                # assignment is a a binary operator that has the type of the assigned value
                p.kind == clang.cindex.CursorKind.BINARY_OPERATOR and p.type and p.type.spelling == "QVariant::Type"):
                needCast = True
                break;
            elif p.kind in [ clang.cindex.CursorKind.BINARY_OPERATOR,
                             clang.cindex.CursorKind.BINARY_OPERATOR.IF_STMT,
                             clang.cindex.CursorKind.BINARY_OPERATOR.SWITCH_STMT]:
                break;

        if needCast: 
            parent_tokens = list(parent.get_tokens())
            add_to_replace(parent_tokens[-1].extent.end, parent_tokens[-1].extent.end, " )")
            add_to_replace(tokens[-1].extent.start, tokens[-1].extent.end, "userType")
            add_to_replace(tokens[0].extent.start, tokens[0].extent.start, "static_cast<QMetaType::Type>( ")
        else:
            add_to_replace(tokens[-1].extent.start, tokens[-1].extent.end, "userType")

    # force QVariant(typeid) call to QgsVariantUtils::createVariant(typeid) because behavior is different between
    # Qt 5 and Qt6
    elif ( node.kind == clang.cindex.CursorKind.UNEXPOSED_EXPR and node.type.spelling == "QVariant"
           and len(tokens) > 3
           and tokens[0].spelling == "QVariant" and tokens[1].spelling == "(" and tokens[-1].spelling == ")"
           and searchType(node, tokens[2:-1]) in ["QMetaType::Type", "QVariant::Type", "const QMetaType::Type", "const QVariant::Type"] ):
        
        add_to_replace(tokens[0].extent.start, tokens[0].extent.end, "QgsVariantUtils::createVariant")
        
    # Same as above but with the form QVariant var( metatypeid )
    elif ( node.kind == clang.cindex.CursorKind.VAR_DECL and node.type.spelling == "QVariant"
           and len(tokens) > 4
           and tokens[0].spelling == "QVariant" and tokens[2].spelling == "(" and tokens[-1].spelling == ")"
           and searchType(node, tokens[3:-1]) == "QVariant::Type" ):
        
        add_to_replace(tokens[2].extent.start, tokens[2].extent.end, f" = QgsVariantUtils::createVariant(")

    # Recurse for children of this node
    for c in node.get_children():
        to_replace_children = find_qvariant_deprecated(to_replace, c, [node] + parents, already_parsed)
                    
compileDB = clang.cindex.CompilationDatabase.fromDirectory(sys.argv[1])

# try to load some cache to speed up things
all_to_replaces = defaultdict(set)
cache_filename = "/tmp/remove_deprecated_qvariant.pkl"
if os.path.exists(cache_filename):
    with open(cache_filename, 'rb') as f:
        all_to_replaces = defaultdict(set, pickle.load(f))
        
already_parsed=[]
index = clang.cindex.Index.create()

debugCppFile = debugfile.replace(".h",".cpp") if debugfile else None
# clear cache for debug file
if debugfile and debugfile in all_to_replaces:
    del all_to_replaces[debugfile]
if debugCppFile and debugCppFile in all_to_replaces:
    del all_to_replaces[debugCppFile]

for compileCommand in compileDB.getAllCompileCommands():

    basename = os.path.basename( compileCommand.filename )
    if (basename.startswith( "sip_" ) or basename.startswith( "mocs_" )
        or compileCommand.filename.startswith("/home/julien/work/QGIS/external/")
        or compileCommand.filename in all_to_replaces ):
        continue

    if not compileCommand.filename.startswith(f"/home/julien/work/QGIS/{module}"):
        continue

    if debugfile and debugline and compileCommand.filename != debugCppFile:
        continue

    tu = index.parse(compileCommand.filename, args=list(compileCommand.arguments)[1:-2], options=clang.cindex.TranslationUnit.PARSE_DETAILED_PROCESSING_RECORD)
    print('Translation unit:{}'.format(tu.spelling))

    # to be sure we add it in cache to avoid reparsing it again
    all_to_replaces[compileCommand.filename] = set()
    
    find_qvariant_deprecated(all_to_replaces, tu.cursor, [], already_parsed)
    already_parsed=list(all_to_replaces.keys())

    # update cache
    with open(cache_filename, 'wb') as f:
        pickle.dump(dict(all_to_replaces), f)


if debugAndReturn:
    print("-- DebugAndReturn")
    exit(0)

for filename, to_replaces_set in all_to_replaces.items():

    if ( (debugfile and debugline and filename != debugfile)
         or not filename.startswith(f"/home/julien/work/QGIS/{module}") ):
        continue
    
    with open(filename, 'r') as f:
        data = f.readlines()

    to_replaces = list(to_replaces_set)
    to_replaces.sort(key=lambda item: (item[0], item[1]), reverse=True)
    for to_replace in to_replaces:

        start_line, start_col, end_line, end_col, new_str = to_replace

        # lines start at 1, array index start at 0
        rowstart = start_line-1
        rowend = end_line-1
        colstart = start_col-1
        colend = end_col-1

        #print(f"-- start={rowstart}/{colstart} end={rowend}/{colend}")
        if rowstart != rowend:
            # keep only the first line
            del data[rowstart+1:rowend+1]
            colend = None

        newline = data[rowstart][:colstart] + new_str
        if colend:
            newline += data[rowstart][colend:]

        data[rowstart] = newline

    with open(filename, 'w') as f:
        f.writelines(data)

[github-actions]

🪟 Windows builds ready!

Windows builds of this PR are available for testing here. Debug symbols for this build are available here.

(Built from commit b0f2473)

[troopa81]

Everything is green now, anyone willing to review this beast 😬 ?

[nyalldawson]

@troopa81 I'm part way through a review. Nothing scary so far, but I want to be extra cautious here! It's looking great so far

[troopa81]

@troopa81 I'm part way through a review. Nothing scary so far, but I want to be extra cautious here! It's looking great so far

Great to hear, thanks!!

[nyalldawson]

Something has gone wrong here -- the original code wasn't creating a variant, it was using a variant type as the default value from the map

[troopa81]

QVariant value() takes the default value, meaning a QVariant object in the case of a QVariantMap, so we need a QVariant object. QVariant object was automatically constructed when passing a QVariant::Invalid using this ctor deprecated in Qt 6.

I don't see any other way of fixing this

[nyalldawson]

But in this case we're not trying to retrieve a variant from the map, we are trying to retrieve an int/enum value. So I think the better version would be something like:

static_cast< QMetaType::Type >( map.value( QStringLiteral( "type" ), static_cast< int >( QMetaType::Type::UnknownType ) ).toInt() )

Otherwise the default value will be an invalid QVariant(), we'll call .toInt() on that and get 0 (since QVariant() can't be converted to int, but we aren't checking whether the conversion succeeded), and then static cast it to QMetaType::Type... which still results in QMetaType::Type::UnknownType, but just via a convulated and tricky to understand path :rofl:

[nyalldawson]

I'll address this in a follow up, let's get this monster merged before more merge conflicts are introduced!

[nyalldawson]

See #57620

[nyalldawson]

Something has gone wrong here too -- there should be no createVariant call here.

[troopa81]

same comment as above

[nyalldawson]

@troopa81 ok, just a couple of small changes. Which is amazing given the size of this changeset! Fantastic work here, I can only imagine how painful this was...

[nirvn]

@troopa81 , good job!