Hi,
TL;DR:
- we should add Q_DISCARD_CTOR to all ctors of classes with class-level
[[nodiscard]], esp. RAII objects
- class-level [[nodiscard]] seems to be abused atm to get ctor-level
[[nodiscard]] semantics in C++!7
- do we want to mark (almost) all ctors as [[nodiscard]], in the future?
Long form:
Ivan implemented Q_NODISCARD_CTOR for 6.6 and I've started to roll use
of it our over the QtBase. I have some questions.
First, some terms:
I call this a class-level [[nodicard]]:
class [[nodiscard]] QSignalBlocker { ~~~~ };
This is a C++17 feature and we can (and do) use it unconditionally.
I call this a ctor-level [[nodiscard]]:
class QSignalBlocker {
public:
[[nodiscard]]
explicit QSignalBlocker(QObject *o) ~~~~;
~~~~
};
This is a C++20 feature (technically, it's a clarification, potentially
retroactively applied to C++!7, but that still means we can't rely on
it), which is why we need a macro.
I won't lecture about what the two so, you can read up on that on
cppreference.com:
- https://en.cppreference.com/w/cpp/language/attributes/nodiscard
- Paper: wg21.link/p1771
The obvious first scenario is to cause warnings for code such as
// Listing 1
QMutexLocker(&mutex);
Or any other RAII class. Clang warns about this with a class-level
[[nodiscard]]. GCC does not.
So the first rule should be to make all ctors [[nodiscard]] in classes
that are class-level [[nodiscard]].
So far so good.
Enter QScopedPointer. It currently has a class-level [[nodiscard]], but
does that actually make sense? C++!7 allows us to return QScopedPointer
from a function (guaranteed copy elision), so users can write functions
that return QScopedPointer. But it's not like use of that return value
is required to be required.
So I've come to think that this use of class-level [[nodiscard]] is wrong.
It's the only way to get the desired warning for Listing 1 in C++17
(absent P1771), on some compilers, but the semantics are wrong. Even for
pure RAII types: I might return a QMutexLocker from a function, but that
shouldn't mean that function's return value is [[nodiscard]]. It might
be perfectly ok to discard it. We don't know, yet we enforce that policy
for everyone by making QMutexLocker class-level [[nodiscard]].
In fact, what we actually want is ctor-level [[nodiscard]] in these
cases. For the time being, given we support C++17, I'm ok with having
class-level [[nodiscard]] on pure RAII classes (ie. not on smart
pointers), but come C++20, we should remove these in favour of
ctor-level [[nodiscard]]s.
Here comes the can of worms: If you accept that Listing 1 is worth
warning about, what about
// Listing 2
QSize(12, 32);
// Listing 3
QString("Hello, useless");
// Listing 4
QSlider(0, 100, this);
Does this mean that, like for most const member functions that have
since gotten [[nodiscard]] because their only side-effect is to produce
a return value, we should mark _all_ ctors [[nodiscard]], because their
only side-effect is to produce an object?
Thanks,
Marc
--
Marc Mutz <marc.m...@qt.io>
Principal Software Engineer
The Qt Company
Erich-Thilo-Str. 10 12489
Berlin, Germany
www.qt.io
Geschäftsführer: Mika Pälsi, Juha Varelius, Jouni Lintunen
Sitz der Gesellschaft: Berlin,
Registergericht: Amtsgericht Charlottenburg,
HRB 144331 B
--
Development mailing list
Development@qt-project.org
https://lists.qt-project.org/listinfo/development
