[Qt6] Get rid of deprecated QVariant::Type
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)
🪟 Windows builds ready!
Windows builds of this PR are available for testing here. Debug symbols for this build are available here.
(Built from commit b0f247370b9159af815cd9abb97f9bfc35540fba)
Everything is green now, anyone willing to review this beast :grimacing: ?
@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 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!!
@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...
@troopa81 , good job!