diff options
author | Giuseppe D'Angelo <giuseppe.dangelo@kdab.com> | 2024-03-22 19:06:08 +0100 |
---|---|---|
committer | Giuseppe D'Angelo <giuseppe.dangelo@kdab.com> | 2024-04-17 20:07:47 +0200 |
commit | 7466831509fe163f3fd1e3a6bbf38f6f5a32ef00 (patch) | |
tree | c6c84e1f2762ba2d003a1eb017f12b8bdb6c0907 /src/widgets | |
parent | 067e3ec1bf3f18a1ca79f7241241d3cf021c3715 (diff) |
Long live [[nodiscard]] QFile::open
Having already caught some bugs in real code because of unchecked calls
to QFile::open, this commit marks QFile::open (and open() in other
file-I/O classes) as [[nodiscard]].
Since it's going to raise warnings, the plan is to keep the existing
behavior up to and including the next LTS. Then the warnings will switch
on by default. All of this is protected by system of macros to opt-in or
opt-out the behavioral change at any time.
A possible counter-argument for doing this is that QFile::open is also
used for opening files in the the resource system, and that opening
"cannot fail". It clearly can, if the resource is moved away or renamed;
code should at a minimum use a Q_ASSERT in debug builds. Another
counter-argument is the opening of file handles or descriptors; but
again, that opening may fail in case the handle has been closed or if
the flags are incompatible.
---
Why not marking *every* open() override? Because some are not meant to
be called directly -- for instance sockets are supposed to be open via
calls to `connectToHost` or similar.
One notable exception is QIODevice::open() itself. Although rarely
called directly by user code (which just calls open() on a specific
subclass, which likely has an override), it may be called:
1) By code that just takes a `QIODevice *` and does something with it.
That code is arguably more rare than code using QFile directly.
Still, being "generic" code, they have an extra responsibility when
making sure to handle a possible opening failure.
2) By QIODevice subclasses, which are even more rare. However, they
usually ignore the return from QIODevice::open() as it's
unconditionally true. (QIODevice::open() doesn't use the protected
virtual pattern.)
I'll try and tackle QIODevice in a future commit.
[ChangeLog][QtCore][QFileDevice] The open() functions of file-related
I/O classes (such as QFile, QSaveFile, QTemporaryFile) can now be marked
with the "nodiscard" attribute, in order to prevent a category of bugs
where the return value of open() is not checked and the file is then
used. In order to avoid warnings in existing code, the marking can be
opted in or out, by defining QT_USE_NODISCARD_FILE_OPEN or the
QT_NO_USE_NODISCARD_FILE_OPEN macros. By default, Qt will automatically
enable nodiscard on these functions starting from Qt 6.10.
Change-Id: Ied940e1c0a37344f5200b2c51b05cd1afcb2557d
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Edward Welbourne <edward.welbourne@qt.io>
Diffstat (limited to 'src/widgets')
-rw-r--r-- | src/widgets/CMakeLists.txt | 1 |
1 files changed, 1 insertions, 0 deletions
diff --git a/src/widgets/CMakeLists.txt b/src/widgets/CMakeLists.txt index 2ba8e4719a..fdef309a4a 100644 --- a/src/widgets/CMakeLists.txt +++ b/src/widgets/CMakeLists.txt @@ -55,6 +55,7 @@ qt_internal_add_module(Widgets QT_NO_CONTEXTLESS_CONNECT QT_NO_USING_NAMESPACE QT_NO_FOREACH + QT_USE_NODISCARD_FILE_OPEN INCLUDE_DIRECTORIES dialogs LIBRARIES |