[Development] C++20 ctor-level [[nodiscard]] (was: Re: C++20 @ Qt)

[Marc Mutz]

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.mutz at 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