[normative] QUIP-0019: [[nodiscard]] Policy

Fixes: QTBUG-104167
Change-Id: I4b6b079f4a1c0ee39035aa424f9447732424975c
Reviewed-by: Volker Hilsheimer <volker.hilsheimer@qt.io>
Reviewed-by: Allan Sandfeld Jensen <allan.jensen@qt.io>
Reviewed-by: Mårten Nordheim <marten.nordheim@qt.io>

1 files changed, 155 insertions, 0 deletions

diff --git a/quip-0019-nodiscard-policy.rst b/quip-0019-nodiscard-policy.rst
new file mode 100644
index 0000000..2c4adff
--- /dev/null
+++ b/quip-0019-nodiscard-policy.rst
@@ -0,0 +1,155 @@
+QUIP: 19
+Title: [[nodiscard]] Policy
+Author: Marc Mutz
+Status: Active
+Type: Implementation
+Created: 2023-06-19
+Post-History: https://lists.qt-project.org/pipermail/development/2023-June/044044.html
+
+``[[nodicard]]`` Policy
+===================
+
+This QUIP aims to document how we use the ``[[nodiscard]]`` attribute in Qt.
+
+Motivation
+----------
+
+The ``[nodiscard]]`` attribute can be applied to functions to cause a
+warning if the function's return value is ignored (discarded). This
+can be used to alert the user of the function to non-sensical
+(``list.empty();``) or dangerous code (ignoring error codes).
+
+It can also be applied to constructors (but see below) to warn about
+the object being ignored (``QMutexLocker(&mutex)`` instead of
+``QMutexLocker locker(&mutex)``).
+
+Finally, it can be applied to classes whole-sale, in which case it
+applies to all functions returning that type, Qt or user functions,
+and, depending on compiler, to all constructors of the type, too.
+
+Version History
+---------------
+
+- Since ``[[nodiscard]]`` is a C++17 addition, Qt 5 has the
+  ``Q_REQUIRED_RESULT`` macro, defined to platform-dependent
+  equivalents, if any. This macro can only be used on
+  (non-constructor) functions.
+
+- Since Qt 6.0, we can use ``[nodiscard]`` unconditionally on
+  (non-constructor) functions and at class level (non-exported classes
+  only). The use of ``Q_REQUIRED_RESULT`` is discouraged in Qt 6.
+
+- Since Qt 6.6, we can use ``Q_NODISCARD_CTOR`` to apply
+  ``[[nodiscard]]`` to contructors, too.
+
+Policy
+------
+
+The key words "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT",
+"SHOULD", "SHOULD NOT", "RECOMMENDED", "MAY", and "OPTIONAL" in this
+document are to be interpreted as described in `RFC 2119
+<https://datatracker.ietf.org/doc/html/rfc2119>`_.
+
+Unless mentioned otherwise, the following applies to both public and
+private Qt C++ APIs.
+
+Constructors
+````````````
+
+
+1. From Qt 6.6 onwards, all constructors of RAII and smart pointer
+   classes SHALL be marked as ``Q_NODISCARD_CTOR``. Constructors of
+   other classes MAY be marked as ``Q_NODISCARD_CTOR``, at author's
+   and reviewer's discretion.
+
+2. Qt versions prior to 6.6 lack the macro and therefore can't use
+   ``[[nodiscard]]`` on constructors.
+
+Other Functions
+```````````````
+
+1. Functions where calls that ignore the return type are most likely
+   wrong (e.g. ``QList::empty()``, which sounds like a modifying
+   function, but in fact is ``const``) SHALL be marked as
+   ``[[nodiscard]]`` (or ``Q_REQUIRED_RESULT``, in Qt 5).
+
+2. Other functions whose only side-effect is to produce the
+   return value, esp. `const` member functions, MAY be marked as
+   ``[[nodiscard]]``, at author's and reviewer's discretion.
+
+3. Functions that have side-effects SHALL NOT be marked
+   ``[[nodiscard]]``, unless they fall into Case 1.
+
+Whole Classes
+`````````````
+
+1. We currently do not use ``[[nodiscard]]`` on whole classes. Qt
+   still contains such uses, but they are historic and scheduled to be
+   replaced with ``Q_NODISCARD_CTOR`` on all the class' constructors.
+
+2. ``[[nodiscard]]`` MAY be used on classes, though, provided the user
+   documentation (in case of public API) or the commit message (in
+   case of private API) give rationale for doing so.
+
+3. Due to a bug in GCC, ``[[nodiscard]]`` *classes* cannot be exported
+   wholesale at the moment. If there is enough demand, this may be
+   worked around in the future.
+
+C++20 ``[[nodiscard("reason")]]``
+`````````````````````````````````
+
+Qt currently does not require C++20, and does not offer a macro
+implementing this feature, so we cannot use it in the code base,
+unless guarded by a feature check macro.
+
+If and when such a macro is added, it SHALL be named
+``Q_NODISCARD_X``, not ``Q_REQUIRED_RESULT_X``.
+
+
+Formatting
+----------
+
+1. Syntactically, ``[[nodiscard]]`` MUST come first in the
+   declaration, esp. before a ``Q_*_EXPORT`` macro, if any. If the
+   function is a template, the *template-initalizer* MUST come
+   first. C++ enforces both, even though some compilers are known to
+   accept some forms of non-standard formatting, too (e.g. GCC).
+
+   Examples:
+
+   ::
+
+      template <typname T>
+      [[nodiscard]] int foo(const T&);
+
+      [[nodiscard]] Q_CORE_EXPORT int foo(const QString&);
+
+      [[nodiscard]] Q_CORE_EXPORT QVeryLongReturnTypeClassName
+      bar(const QString&);
+
+      Q_NODISCARD_CTOR explicit QFoo(const QBar &);
+
+   Definitions that are not declarations SHALL NOT repeat the
+   attribute.
+
+2. New code SHOULD add the ``[[nodiscard]]`` or ``Q_NODISCARD_CTOR``
+   as part of the line that also contains the (leading) return type.
+
+   See point 1 for examples.
+
+3. When adding ``[[nodiscard]]`` or ``Q_NODISCARD_CTOR`` to
+   declarations that pertain to already-released API, the attribute
+   SHOULD be added on a separate line, or in a line with other
+   attributes newly-added for the current release. This makes the
+   header diff easier to read in API diffs at the cost of somewhat
+   more vertical space needed for the declaration going forward.
+
+
+References
+----------
+
+.. `[[nodiscard]] documentation: https://en.cppreference.com/w/cpp/language/attributes/nodiscard`_
+
+.. `"[[nodiscard]] for ctors", Peter Sommerlad: https://wg21.link/p1771`_
+
+.. `GCC bug preventing class-level ``[[nodiscard]]`` on exported classes: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=96117`_