This reverts commit 79a11470f3, which
resulted in QTBUG-105735. The new behavior is worse and affects multiple
screen readers, while the old issue is isolated to Windows Narrator and
could be considered a narrator bug.
Task-number: QTBUG-105735
Pick-to: 6.2 6.4
Change-Id: Ic8be1dbd592a3fdf2c3219ec4c5524bc2c7f0f6a
Reviewed-by: Jan Arve Sæther <jan-arve.saether@qt.io>
Pass parameters as const-reference, and declare member functions as
const. They have no reason to modifty the converter itself, and
converters are expected to return the same result for identical input.
Task-number: QTBUG-93632
Change-Id: I19ef799c1aca956f2cd1aaea1d4f7ecb565fc017
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
It's not called anywhere, we always get all convertors and iterate over
them to find the best match for the relevant context. Change those
loops to use ranged for.
Task-number: QTBUG-93632
Change-Id: Icb450ca56310a0acab881899234ac647792ce0c9
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
The function was primarily called to generate debug output. The only
other usage was to identify the Rtf converter so that we can avoid that
we convert outgoing HTML to RTF. This is just as easily (and more
accurately) done by skipping any converter that converts HTML to RTF.
Task-number: QTBUG-93632
Change-Id: Ibe5bdece982399e2d9823f6df67393d1f81501fe
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
The logic for registration of converter implementations is only used
internally through static functions. Move those, and related global
functions, to a QMacMimeRegistry namespace.
Reduce the QMacInternalPasteboardMime to the abstract interface for
converting between native and Qt clipboard data.
Historically, mime converters can have different "types"; make that API
type-safe.
Task-number: QTBUG-93632
Change-Id: I0e16fefa350398b693486199fe10357fd84abcd6
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
The class registers converters and provides access to the converter
able to handle specific formats. But it is not a converter itself.
The converter would then be the implementation of the virtual
interface, e.g. QWindowsMime subclasses.
Task-number: QTBUG-93632
Change-Id: I150ef28e9bcead4291d72e0b0aa660be3fcd4a8a
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
QDialogs can be backed by native dialog helpers, and the choice of
whether to use a particular helper is a runtime decision, made
in QDialog::setVisible() via QDialogPrivate::canBeNativeDialog().
As the native dialogs may not always provide every feature of the
corresponding Qt dialog, or if the user wants a uniform look and
feel for their application (even if that breaks the platform UX),
there is a way to opt out, via the Qt::AA_DontUseNativeDialogs
attribute.
This attribute was checked by subclasses of QDialog/QDialogPrivate,
for example QFileDialog or QColorDialog, but not by the base class
implementation, which is being used by e.g. QMessageBox.
Ideally we'd reduce the amount of code duplication in this area,
by having subclasses such as QFileDialog and QColorDialog call the
base class implementations, but for now we add a similar check for
the attribute to the base class as we have in other leaf classes.
[ChangeLog][Widgets] QMessageBox now respects the Qt::AA_DontUseNativeDialogs
application attribute to opt out of native message boxes on
platforms where this is supported (iOS, Android).
Change-Id: I0080441ea8764e9d5fc62d7f2d7b8914a5844c28
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Optimize the code for non-empty strings, which are the vast majority of
the conversions. Plus, qstrnto(u)ll operate just fine on empty strings.
This saves 4 instrctions per call on my laptop for any non-empty input,
and up to 4 cycles of execution.
Pick-to: 6.4
Change-Id: I07ec23f3cb174fb197c3fffd17220b846a026b55
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
QString inputs are required to be in the C locale, so we can simply use
qt_to_latin1() in qstring.cpp to do the conversion from UTF-16 to US-
ASCII. There's no need to operate on QLocaleData.
Benchmark of QString::toInt() on "42"; before:
152.8176 ns task-clock:u # 0.999 CPUs utilized
425.904 cycles:u # 2.787 GHz
1550.992 instructions:u # 3.64 insn per cycle
363.998 branches:u # 2.382 G/sec
Now, with a slightly optimized qt_to_latin1:
19.3383 ns task-clock
54.005 cycles:u # 2.793 GHz
238.000 instructions:u # 4.407 insn per cycle
55.000 branches:u # 2.844 G/sec
And with AVX512 256-bit qt_to_latin1:
20.6798 ns task-clock
57.748 cycles:u # 2.793 GHz
237.000 instructions:u # 4.104 insn per cycle
50.000 branches:u # 2.418 G/sec
For comparison, a QByteArray::toInt() on "42" produces:
17.2310 ns task-clock
48.081 cycles:u # 2.790 GHz
205.000 instructions:u # 4.264 insn per cycle
49.000 branches:u # 2.844 G/sec
Fixes: QTBUG-107788
Pick-to: 6.4
Change-Id: I07ec23f3cb174fb197c3fffd1722042b9ccbacbf
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
When submitting applications to the iOS and macOS AppStore the
application goes through static analysis, which will trigger on
uses of various privacy protected APIs, unless the application
has a corresponding usage description for the permission in the
Info.plist file. This applies even if the application never
requests the given permission, but just links to a Qt library
that has the offending symbols or library dependencies.
To ensure that the application does not have to add usage
descriptions to their Info.plist for permissions they never
plan to use we split up the various permission implementations
into small static libraries that register with the Qt plugin
mechanism as permission backends. We can then inspect the
application's Info.plist at configure time and only add the
relevant static permission libraries.
Furthermore, since some permissions can be checked without any
usage description, we allow the implementation to be split up
into two separate translation units. By putting the request in
its own translation unit we can selectively include it during
linking by telling the linker to look for a special symbol.
This is useful for libraries such as Qt Multimedia who would
like to check the current permission status, but without
needing to request any permission of its own.
Done-with: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Change-Id: Ic2a43e1a0c45a91df6101020639f473ffd9454cc
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
This is a semantic patch using ClangTidyTransformator as in
qtbase/df9d882d41b741fef7c5beeddb0abe9d904443d8, but extended to
handle typedefs and accesses through pointers, too:
const std::string o = "object";
auto hasTypeIgnoringPointer = [](auto type) { return anyOf(hasType(type), hasType(pointsTo(type))); };
auto derivedFromAnyOfClasses = [&](ArrayRef<StringRef> classes) {
auto exprOfDeclaredType = [&](auto decl) {
return expr(hasTypeIgnoringPointer(hasUnqualifiedDesugaredType(recordType(hasDeclaration(decl))))).bind(o);
};
return exprOfDeclaredType(cxxRecordDecl(isSameOrDerivedFrom(hasAnyName(classes))));
};
auto renameMethod = [&] (ArrayRef<StringRef> classes,
StringRef from, StringRef to) {
return makeRule(cxxMemberCallExpr(on(derivedFromAnyOfClasses(classes)),
callee(cxxMethodDecl(hasName(from), parameterCountIs(0)))),
changeTo(cat(access(o, cat(to)), "()")),
cat("use '", to, "' instead of '", from, "'"));
};
renameMethod(<classes>, "count", "size");
renameMethod(<classes>, "length", "size");
except that the on() matcher has been replaced by one that doesn't
ignoreParens().
a.k.a qt-port-to-std-compatible-api V5 with config Scope: 'Container'.
Added two NOLINTNEXTLINEs in tst_qbitarray and tst_qcontiguouscache,
to avoid porting calls that explicitly test count().
Change-Id: Icfb8808c2ff4a30187e9935a51cad26987451c22
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Emscripten does not support EM_JS from side modules
Change-Id: I9c29d112fc0a5ab7830e1cadd0af77829c69d0a9
Reviewed-by: Lorn Potter <lorn.potter@gmail.com>
Change-Id: I0b35ca09314b8708de9ed811328ad1de8717f50d
Reviewed-by: Mikołaj Boc <Mikolaj.Boc@qt.io>
Reviewed-by: Lorn Potter <lorn.potter@gmail.com>
Reviewed-by: Aleksandr Reviakin <aleksandr.reviakin@qt.io>
- If this string isn't shared, don't call detach, instead use ->erase() as
needed
- If this string is shared, create a new string, and copy all elements
except the ones that would be removed, see task for details
Update unittest to test both code paths.
Task-number: QTBUG-106181
Change-Id: I4c73ff17a6fa89ddcf6966f9c5bf789753f6d39e
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Add support for enabling -fwasm-exceptions at compile and
link time, which enables use of C++ exceptions.
Wasm-exceptions is an in-progress roadmap item (see
https://webassembly.org/roadmap/), but is supported
by the major browsers
Change-Id: I6e2847206a46ed8038320c99725bc09a0344d1b4
Reviewed-by: Aleksandr Reviakin <aleksandr.reviakin@qt.io>
Reviewed-by: Lorn Potter <lorn.potter@gmail.com>
The 'qml-root-path' option is not mandatory to have value when using
CMake, even if Qml/Quick is used in the project. This happens when
the project doesn't use .qml files. In this case the 'qml' directory
from Qt6Quick_<abi>-android-dependencies.xml is treated as a normal
folder for scanning and androiddeployqt deploys all the QML plugins
with their dependencies.
It looks like the 'qml' directory was added to bundled file in times
when qmlimportscanner was not implemented, so the need of its use is
redundant. This removes both adding the 'qml' directory as the bundled
dependency and the special case that avoids its scanning. This fix is
applicable for both CMake and qmake.
Amends 54c959643e
Task-number: QTBUG-106035
Task-number: QTBUG-107589
Change-Id: Idd55617b8ca8ab1d210cce737548ee486ea94986
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
The check doesn't affect the flow, since the same check fails the
androiddeployqt execution later, when resolving all elf dependencies.
Skipping the dependency from xml at earlier stage will allow to continue
deployment procedure, without the missing plugins and their dependencies.
Amends 54c959643e
Task-number: QTBUG-106035
Task-number: QTBUG-107589
Change-Id: Ic3d38e05f8ad283244c87858fee29d1035a0da15
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
When creating the syncqt custom target we only check if the
WARNINGS_ARE_ERRORS variable is set. But the warnings-are-errors
feature can be disabled using the QT_SKIP_WARNINGS_ARE_ERRORS
target property. Add the genex condition to opt out the
'-warningsAreErrors' argument using the QT_SKIP_WARNINGS_ARE_ERRORS
property.
Change-Id: I8e6f89453500335c2b31e6e69c65b351c79dddc2
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
The original comments are misreading Microsoft documents. It seems
Microsoft uses FLIP_SEQUENTIAL to refer to the flip model, not just
FLIP_SEQUENTIAL itself. And according to my experiments, DXGI_SCALING_NONE
can be used with FLIP_DISCARD indeed. The swap chain can be created
successfully and I didn't get any failed HRESULTs. So there's no need
to use FLIP_SEQUENTIAL as the default swap effect anymore. Let's change
it back to FLIP_DISCARD.
Change-Id: I6001e2070d3ff00268299e81f6a310394e62c464
Reviewed-by: Laszlo Agocs <laszlo.agocs@qt.io>
Adding target dependencies instead of file dependencies to the
qrc processing custom command broke regeneration of the qrc
file when translation source files were touched, as well as caused
flaky build failures.
Originally, the target dependencies were added to work around an
issue with the Xcode generator (a custom command needing a
common target).
Limit the usage of target dependencies 'approach' to the Xcode
generator only.
This fixes the regression for non-Xcode generators, but will still
cause issues for iOS + Xcode.
A proper fix for Xcode will need more research.
Amends 5b0e765ab0dddba86662925cb44aeac748a286b7 in qttools
Amends cfd5485d41
Pick-to: 6.4 6.4.1
Fixes: QTBUG-107687
Fixes: QTBUG-108113
Task-number: QTBUG-103470
Change-Id: Ibddd05726deba2103c9c3c85a3fefd6d55798020
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
When configuring a qtbase developer build, WARNINGS_ARE_ERRORS is
set to ON and -Werror flags are added to the PlatformInternal
targets. But the value of WARNINGS_ARE_ERRORS is not exported.
This means that CMake code can't make decisions based on that variable
when building other repos, and we now have such code with the new
syncqt.cpp in qt_internal_target_sync_headers.
Export the value of WARNINGS_ARE_ERRORS in
QtBuildInternalsExtra.cmake.in.
Fixes: QTBUG-108151
Change-Id: I5de2633fcb1f20fead7d436c201852424e726842
Reviewed-by: Alexey Edelev <alexey.edelev@qt.io>
When 'this' is IPv6 and 'other' is Any then there is no point in testing
'other's IPv6 address.
Added extra tests against QHostAddress::Any*.
Pick-to: 6.4 6.2 5.15
Fixes: QTBUG-108103
Change-Id: I09f32b1b147b1ec8380546c91cd89684a6bebe2e
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
qtwasmcompositor expects coordinates relative to the canvas. Provide
that by using pointer events' offsetX/offsetY instead of the viewport
x/y.
Fixes: QTBUG-108128
Fixes: QTBUG-106031
Pick-to: 6.4 6.4.1
Change-Id: I76c553b7b7e350fd873b18c775848217a4905f3d
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
With the introduction of QAnyStringView, overloading based on UTF-8
and Latin-1 is becoming more common. Often, the two overloads can
share the processing backend, because we're only interested in the
US-ASCII subset of each.
But if they can't, we need a faster way to convert L1 into UTF-8 than
going via UTF-16. This is where the new private API comes in.
Eventually, we should have the converse operation, too, to complete
the set of direct conversions between the possible three
QAnyStringView encodings L1/U8/U16, but this direction is easier to
code (there are no error cases) and more immediately useful, so
provide L1->U8 alone for now.
Change-Id: I3f7e1a9c89979d0eb604cb9e42dedf3d514fca2c
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
qdoc does enforce lowercase file names for .html pages, and also
replaces underscore with a dash. Make sure that the original \page name
already is normalized, so that it's easier to search.
This was done by
find . -name "*.qdoc" -exec perl -p -i -E "s/\\\page (.*)/\\\page \L\1/ && s/_/-/g" {} ;
Pick-to: 6.4
Change-Id: Ib50b85af8ffd985edf06856266eefdebf8b328a3
Reviewed-by: Paul Wicking <paul.wicking@qt.io>
Reviewed-by: Luca Di Sera <luca.disera@qt.io>
Reviewed-by: Topi Reiniö <topi.reinio@qt.io>
If a user requests no version tagging by
defining QT_NO_VERSION_TAGGING a lot of
redefine warnings will be output from the
compiler when building corelib.
So only define QT_NO_VERSION_TAGGING if it
is not already defined.
Pick-to: 6.4
Change-Id: I56609b3589184bda7bec52d168d9fd11e2f14a2c
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Safer, this way ensures we don't have undefined behavior if one of those
members is returned from a method before being initialized.
Change-Id: I30da1837cea960d43b55259a3906d1aecbf722f4
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
To simplify matters, this removes the support for signing APKs with
just jarsigner. apksigner is always used for APKs, and jarsigner is
used for AABs. I consider that to be just fine, you _have_ to start
using apksigner eventually, and the time for that is long past.
Pick-to: 6.4 6.2 5.15
Task-number: QTBUG-91255
Change-Id: I211ae796db0f2619265deb1f30ab3cc5d1ecfbc9
Reviewed-by: Assam Boudjelthia <assam.boudjelthia@qt.io>
Since we support 3 as well now we should not always use 1_1.
The suffix will change depending on which OpenSSL version was used when
Qt was built.
This only affects Android.
Pick-to: 6.4 6.2 5.15
Change-Id: I2e443b12daa5e79190f1b3367e21ba0fa6a1dcd4
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
Borrowed from tst_qtemporaryfile with some changes.
Change-Id: I596ddd0ac8dbe10edd63e481198064dcec15d3e6
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
This is apparently not suitable: the MTLDevice's registryID is stable
across processes and allows identifying a device (GPU), but the
value may apparently change when the system is rebooted. We see this
both in the CI's M1s and also locally. There may also be differences
between Intel or discreet graphics vs. Apple Silicon based machines
but that we won't be able to investigate further.
Inspired by MoltenVK, stop using the registryID as our "device id",
and instead store the OS version in the cache header. Combined with
the device name this should be good enough.
To generate a device/vendor ID, that we give up for now, those will
be 0 in the QRhiDriverInfo like they are for OpenGL.
This should prevent the "Metal device ID does not match" warnings
that tend to pop up in CI logs and elsewhere. (it also improves
performance, obviously since there is no reason we could not
reuse the disk cache on the same machine where nothing really
has changed, and now we will not incorrectly throw away the
previously written cache contents)
Change-Id: Ib457f6671aceb51f5a9d241702b418aafc888e51
Reviewed-by: Qt CI Bot <qt_ci_bot@qt-project.org>
Reviewed-by: Andy Nichols <andy.nichols@qt.io>
Lost in commit 2766322de3 (at 6.0) as it
was buried in the midst of a lot of QStringRef documentation. This
commit restores the documentation originally added in commit
3238445b27, but locates it among the
other QString::append() overloads and adds a \overload directive.
Pick-to: 6.4 6.2
Change-Id: If7b98f1ae9ae8144c421a048c8a35be8474558c7
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
Many features of today's devices and operating systems can have
significant privacy, security, and performance implications if
misused. It's therefore increasingly common for platforms to
require explicit consent from the user before accessing these
features.
The Qt permission APIs allow the application to check or request
permission for such features in a cross platform manner.
The check is always synchronous, and can be used in both
library and application code, from any thread.
The request is asynchronous, and should be initiated from
application code on the main thread. The result of the request
can be delivered to lambdas, standalone functions, or
regular member functions such as slots, with an optional
context parameter to manage the lifetime of the request.
Individual permissions are distinct types, not enum values,
and can be added and extended at a later point.
Task-number: QTBUG-90498
Done-with: Timur Pocheptsov <timur.pocheptsov@qt.io>
Done-with: Volker Hilsheimer <volker.hilsheimer@qt.io>
Done-with: Mårten Nordheim <marten.nordheim@qt.io>
Change-Id: I821380bbe56bbc0178cb43e6cabbc99fdbd1235e
Reviewed-by: Timur Pocheptsov <timur.pocheptsov@qt.io>
It had the d_func()s it needed, but wasn't using them.
This prepares the way for QEDSP-ification.
Pick-to: 6.4
Task-number: QTBUG-105753
Change-Id: I8b7ba79818f27ae6a2281b276b95b94673f05648
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
We can easily enough obtain the root of the present source tree using
the value of __file__, so might as well do so.
Change-Id: If14773ac1127278b6018a090c0b376437b9c6eec
Reviewed-by: Ievgenii Meshcheriakov <ievgenii.meshcheriakov@qt.io>
Move i18n example out of qtbase. In qttools, it can use lrelease,
avoiding the need to store .qm files in the repository.
Change-Id: I8ba36a1372c2a743b809e3f7ea95a67825558f41
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
When it was added, Greater's description was the same as
Equivalent's. This commit solves it by a proper description.
Fixes: QTBUG-106983
Pick-to: 6.4
Change-Id: I9d9b71572921edb8c33209a0b37a138fff3cf962
Reviewed-by: Giuseppe D'Angelo <giuseppe.dangelo@kdab.com>
We were getting font metrics for the primary screen and then adjusting
for the screen DPI get a screen-independent font size.
This could fail in edge cases where the screen DPI was changed after
app startup, but before the first window was shown. See QTBUG-105857.
Use SystemParametersInfoForDpi() to query for font info at 96 DPI instead,
which removes the need for tracking primary screen DPI.
LOGFONT_to_QFont() still has one usage (qwizard_win.cpp) which provides
a custom DPI for scaling, so we keep that function as-is.
Fixes: QTBUG-105857
Pick-to: 6.4 6.2 5.15
Change-Id: I1adf0ab3bf2c309e8fcb58093e86214fa11a2da8
Reviewed-by: Tor Arne Vestbø <tor.arne.vestbo@qt.io>
The 'const self = this;' declaration was omitted in QtLoader constructor,
which made all of the self.something = value assignments actually
assign to the scope self variable (window.self, in most cases).
Make the loader always be constructed with 'new', and assign 'this' to
'self' to always assign properties to the QtLoader instance.
Change-Id: I9cf7cc95e7341531a702edc431aa242b39911f66
Reviewed-by: Morten Johan Sørvig <morten.sorvig@qt.io>
Three methods parsing strings as numbers claimed not to fall back on
the C locale. In fact, the parsing is somewhat lenient and has long
accepted the digits and signs of the C locale's representations, along
with the U+2212 minus sign, regardless of locale.
The phrasing also tacitly suggested the matching QString function did
fall back to C locale; when, in fact, it (since 6.0) *exclusively*
uses the C locale. So that was misleading, too.
Change-Id: Ida3f2f23b834ced5150bea20d8716756afd48e91
Reviewed-by: André Hartmann <aha_1980@gmx.de>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
qhashfunctions.h defines a catch-all 2-arguments qHash(T, seed)
in order to support datatypes that implement a 1-argument overload
of qHash (i.e. qHash(Type)). The catch-all calls the 1-argument
overload and XORs the result with the seed.
The catch-all is constrained on the existence of such a 1-argument
overload. This is done in order to make the catch-all SFINAE-friendly;
otherwise merely instantiating the catch-all would trigger a hard error.
Such an error would make it impossible to build a type trait that
detects if one can call qHash(T, size_t) for a given type T.
The constraint itself is called HasQHashSingleArgOverload and lives in a
private namespace.
It has been observed that HasQHashSingleArgOverload misbehaves for
some datatypes. For instance, HasQHashSingleArgOverload<int> is actually
false, despite qHash(123) being perfectly callable. (The second argument
of qHash(int, size_t) is defaulted, so the call *is* possible.)
--
Why is HasQHashSingleArgOverload<int> false?
This has to do with how HasQHashSingleArgOverload<T> is implemented: as
a detection trait that checks if qHash(declval<T>()) is callable.
The detection itself is not a problem. Consider this code:
template <typename T>
constexpr bool HasQHashSingleArgOverload = /* magic */;
class MyClass {};
size_t qHash(MyClass);
static_assert(HasQHashSingleArgOverload<MyClass>); // OK
Here, the static_assert passes, even if qHash(MyClass) (and MyClass
itself) were not defined at all when HasQHashSingleArgOverload was
defined.
This is nothing but 2-phase lookup at work ([temp.dep.res]): the
detection inside HasQHashSingleArgOverload takes into account the qHash
overloads available when HasQHashSingleArgOverload was declared, as well
as any other overload declared before the "point of instantiation". This
means that qHash(MyClass) will be visible and detected.
Let's try something slightly different:
template <typename T>
constexpr bool HasQHashSingleArgOverload = /* magic */;
size_t qHash(int);
static_assert(HasQHashSingleArgOverload<int>); // ERROR
This one *does not work*. How is it possible? The answer is that 2-phase
name lookup combines the names found at definition time with the names
_found at instantiation time using argument-dependent lookup only_.
`int` is a fundamental type and does not participate in ADL. In the
example, HasQHashSingleArgOverload has actually no qHash overloads to
even consider, and therefore its detection fails.
You can restore detection by moving the declaration of the qHash(int)
overload *before* the definition of HasQHashSingleArgOverload, so it's
captured at definition time:
size_t qHash(int);
template <typename T>
constexpr bool HasQHashSingleArgOverload = /* magic */;
static_assert(HasQHashSingleArgOverload<int>); // OK!
This is why HasQHashSingleArgOverload<int> is currently returning
`false`: because HasQHashSingleArgOverload is defined *before* all the
qHash(fundamental_type) overloads in qhashfunctions.h.
--
Now consider this variation of the above, where we keep the qHash(int)
overload after the detector (so, it's not found), but also prepend an
Evil class implicitly convertible from int:
struct Evil { Evil(int); };
size_t qHash(Evil);
template <typename T> constexpr bool HasQHashSingleArgOverload = /* magic */;
size_t qHash(int);
static_assert(HasQHashSingleArgOverload<int>); // OK
Now the static_assert passes. HasQHashSingleArgOverload is still not
considering qHash(int) (it's declared after), but it's considering
qHash(Evil). Can you call *that* one with an int? Yes, after a
conversion to Evil.
This is extremely fragile and likely an ODR violation (if not ODR, then
likely falls into [temp.dep.candidate/1]).
--
Does this "really matter" for a type like `int`? The answer is no. If
HasQHashSingleArgOverload<int> is true, then a call like
qHash(42, 123uz);
will have two overloads in its overloads set:
1) qHash(int, size_t)
2) qHash(T, size_t), i.e. the catch-all template. To be pedantic,
qHash<int>(const int &, size_t), that is, the instantiation of the
catch-all after template type deduction for T (= int)
([over.match.funcs.general/8]).
Although it may look like this is ambiguous as both calls have perfect
matches for the arguments, 1) is actually a better match than 2) because
it is not a template specialization ([over.match.best/2.4]).
In other words: qHash(int, size_t) is *always* called when the argument
is `int`, no matter the value of HasQHashSingleArgOverload<int>. The
catch-all template may be added or not to the overload set, but it's
a worse match anyways.
--
Now, let's consider this code:
enum MyEnum { E1, E2, E3 };
qHash(E1, 42uz);
This code compiles, although we do not define any qHash overload
specifically for enumeration types (nor one is defined by MyEnum's
author).
Which qHash overload gets called? Again there are two possible
overloads available:
1) qHash(int, size_t). E1 can be converted to `int` ([conv.prom/3]),
and this overload selected.
2) qHash(T, size_t), which after instantiation, is qHash<MyEnum>(const
MyEnum &, size_t).
In this case, 2) is a better match than 1), because it does not require
any conversion for the arguments.
Is 2) a viable overload? Unfortunately the answer here is "it depends",
because it's subject to what we've learned before: since the catch-all
is constrained by the HasQHashSingleArgOverload trait, names introduced
before the trait may exclude or include the overload.
This code:
#include <qhashfunctions.h>
enum MyEnum { E1, E2, E3 };
qHash(E1, 42uz);
static_assert(HasQHashSingleArgOverload<MyEnum>); // ERROR
will fail the static_assert. This means that only qHash(int, size_t) is
in the overload set.
However, this code:
struct Evil { Evil(int); };
size_t qHash(Evil);
#include <qhashfunctions.h>
enum MyEnum { E1, E2, E3 };
qHash(E1, 42uz);
static_assert(HasQHashSingleArgOverload<MyEnum>); // OK
will pass the static_assert. qHash(Evil) can be called with an object of
type MyEnum after an user-defined conversion sequence
([over.best.ics.general], [over.ics.user]: a standard conversion
sequence, made of a lvalue-to-rvalue conversion + a integral promotion,
followed by a conversion by constructor [class.conv.ctor]).
Therefore, HasQHashSingleArgOverload<MyEnum> is true here; the catch-all
template is added to the overload set; and it's a best match for the
qHash(E1, 42uz) call.
--
Is this a problem? **Yes**, and a huge one: the catch-all template does
not yield the same value as the qHash(int, size_t) overload. This means
that calculating hash values (e.g. QHash, QSet) will have different
results depending on include ordering!
A translation unit TU1 may have
#include <QSet>
#include <Evil>
QSet<MyEnum> calculateSet { /* ... */ }
And another translation unit TU2 may have
#include <Evil>
#include <QSet> // different order
void use() {
QSet<MyEnum> set = calculateSet();
}
And now the two TUs cannot exchange QHash/QSet objects as they would
hash the contents differently.
--
`Evil` actually exists in Qt. The bug report specifies QKeySequence,
which has an implicit constructor from int, but one can concoct infinite
other examples.
--
Congratulations if you've read so far.
=========================
=== PROPOSED SOLUTION ===
=========================
1) Move the HasQHashSingleArgOverload detection after declaring the
overloads for all the fundamental types (which we already do anyways).
This means that HasQHashSingleArgOverload<fundamental_type> will now
be true. It also means that the catch-all becomes available for all
fundamental types, but as discussed before, for all of them we have
better matches anyways.
2) For unscoped enumeration types, this means however an ABI break: the
catch-all template becomes always the best match. Code compiled before
this change would call qHash(int, size_t), and code compiled after this
change would call the catch-all qHash<Enum>(Enum, size_t); as discussed
before, the two don't yield the same results, so mixing old code and new
code will break.
In order to restore the old behavior, add a qHash overload for
enumeration types that forwards the implementation to the integer
overloads (using qToUnderlying¹).
(Here I'm considering the "old", correct behavior the one that one gets
by simply including QHash/QSet, declaring an enumeration and calling
qHash on it. In other words, without having Evil around before including
QHash.)
This avoids an ABI break for most enumeration types, for which one
does not explicitly define a qHash overload. It however *introduces*
an ABI break for enumeration types for which there is a single-argument
qHash(E) overload. This is because
- before this change, the catch-all template was called, and that
in turn called qHash(E) and XOR'ed the result with the seed;
- after this change, the newly introduced qHash overload for
enumerations gets called. It's very likely that it would not give
the same result as before.
I don't have a solution for this, so we'll have to accept the ABI
break.
Note that if one defines a two-arguments overload for an enum type,
then nothing changes there (the overload is still the best match).
3) Make plans to kill the catch-all template, for Qt 7.0 at the latest.
We've asked users to provide a two-args qHash overload for a very long
time, it's time to stop working around that.
4) Make plans to switch from overloading qHash to specializing std::hash
(or equivalent). Specializations don't overload, and we'd get rid of
all these troubles with implicit conversions.
--
¹ To nitpick, qToUnderlying may select a *different* overload than
the one selected by an implicit conversion.
That's because an unscoped enumeration without a fixed underlying type
is allowed to have an underlying type U, and implicitly convert to V,
with U and V being two different types (!).
U is "an integral type that can represent all the enumerator values"
([dcl.enum/7]). V is selected in a specific list in a specific order
([conv.prom]/3). This means that in theory a compiler can take enum E {
E1, E2 }, give it `unsigned long long` as underlying type, and still
allow for a conversion to `int`.
As far as I know, no compiler we use does something as crazy as that,
but if it's a concern, it needs to be fixed.
[ChangeLog][Deprecation Notice] Support for overloads of qHash with only
one argument is going to be removed in Qt 7. Users are encouraged to
upgrade to the two-arguments overload. Please refer to the QHash
documentation for more information.
[ChangeLog][Potentially Binary-Incompatible Changes] If an enumeration
type for which a single-argument qHash overload has been declared is
being used as a key type in QHash, QMultiHash or QSet, then objects of
these types are no longer binary compatible with code compiled against
an earlier version of Qt. It is very unlikely that such qHash overloads
exist, because enumeration types work out of the box as keys Qt
unordered associative containers; users do not need to define qHash
overloads for their custom enumerations. Note that there is no binary
incompatibity if a *two* arguments qHash overload has been declared
instead.
Fixes: QTBUG-108032
Fixes: QTBUG-107033
Pick-to: 6.2 6.4
Change-Id: I2ebffb2820c553e5fdc3a341019433793a58e3ab
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
I think we can remove the WARNING regarding AAT which was probably there
to avoid using older version. As far as I can tell, AAT is supported
from 2.0, and quite stable by now.
Task-number: QTBUG-107044
Change-Id: I8e6ba3f51b85e2859dd46435eee1220da46831d2
Reviewed-by: Alexandru Croitor <alexandru.croitor@qt.io>
