[Development] RFF: nullptr rules

[Marc Mutz]

On Wednesday 09 December 2015 16:14:00 Marc Mutz wrote:
> Hi,
> 
> in 5.7, nullptr (Q_C_NULLPTR) is required to be supported by the compiler,
> but there are no guidelines as to its use in the coding conventions (to
> the extent they need to be in there).
> 
> I propose the following, based on Thiago's proposal from January this year,
> considering the new situation that we now require nullptr support in the
> compiler:
> 
> - 0 as a nullptr constant is banned except in tests testing APIs so
>   we don't accidentally require nullptr (ie. all tests should use 0, not
>   nullptr, as far as it makes sense)
> - clang-modernize is used to convert all uses of the null pointer constant
> to nullptr, incl. examples, excl. tests and 3rdparty
> - compilers that have it, will have -Wzero-as-nullptr-constant added during
>   headersclean[0]
> - APIs that require the use of nullptr for disambiguation are discouraged,
>   but may be acceptable to be decided on a case-by-case basis.
> - Accidental (ie. non-apidox'ed) reliance on nullptr for disambiguation is
>   always an error. To this end, tests should continue to use 0, not
> nullptr. - if (!foo) vs. if (foo == nullptr), if (foo) vs. if (foo !=
> nullptr): author's prerogative[1]
> 
> [0] I'd prefer "when building Qt", but realise that we'll have problems
> with upstream libs
> [1] I prefer the short form, but I don't think we'll gain a consensus here,
> so let's not even try
> 
> Arguments in favour:
> - it's the C++ way of writing the null pointer constant these days
> - we need to use it in headers, anyway, to allow people to use
> -Wzero-as..., and it makes no sense to have two sets of rules for headers
> and impl - it can disambiguate code and prevent accidents
> - in some situations, it makes code easier to understand (:
> m_foo(nullptr)).
> 
> Arguments against:
> - it's uglier than "0", and more to type

Because it's coming up again to have nullptr only in some places and leave 0 
in others:

The predictable "let's use it only in X" reply is a rearguard battle trying to 
keep the _inevitable_ change from 0 to nullptr contained to a certain subset 
of cases, thereby complicating the ruleset, subsequently the reviews based on 
that ruleset and thwards automatic migration.

Converting _all_ null pointer constants to nullptr is a simple run of clang-
modernize. It is reasonable to trust the tool and not review every single 
instance by multiple people on Gerrit, but wave it though into CI quickly.

But if we have a complicated ruleset for when to use nullptr and when to use 
0, the person submitting the results of clang-modernize will have to wade 
through the changes one-by-one, separating the wanted from the unwanted 
changes. Then a few Gerrit reviewers will have to review all the changes 
_again_, matching against the ruleset, discussing unclear situations....

All this just because some people have an aesthetic problem with "nullptr". 
Guess what, you can always s/nullptr/0/ for display, but the converse is _not_ 
possible.

Whatever happened to KISS?

Thanks,
Marc

-- 
Marc Mutz <marc.mutz at kdab.com> | Senior Software Engineer
KDAB (Deutschland) GmbH & Co.KG, a KDAB Group Company
Tel: +49-30-521325470
KDAB - The Qt Experts