[Development] Upgrading the sources to C++11 keywords (Q_NULLPTR, etc.)

[Thiago Macieira]

On Friday 09 January 2015 20:19:36 André Pönitz wrote:
> In some case these macros even expand to something else than the "obviously
> related" C++ keyword, so potential benefits gained from using the actual
> feature have to be weighed with the costs of obfuscation.

They expand to the correct keyword when the keyword is permitted, so there's 
no mistake. They expand to something else that will not affect the code unduly 
if the keyword is not permitted.

Do you have anything in mind for when they'd expand unexpectedly?

> > I'd like to propose the following.
> > 
> > Policy per keyword:
> >  * Q_NULLPTR - strongly encouraged
> > 
> > Use it whenever your literal zero is a null pointer. You can leave it out
> > when it cannot be mistaken for a zero and it would otherwise make the
> > source code harder to read.
> > 
> > Note that Qt still compiles with C++98, so you can't use Q_NULLPTR for
> > disambiguation.
> 
> No, please, not.
> 
> C++ 'nullptr' only gives a benefit over '0' in the rare cases where it
> helps for disambiguation. I would not really like a policy encouraging
> 'nullptr' when '0' is just fine, but at least that would be tolerable.
> 
> On the other hand, 'Q_NULLPTR' does not even serve the 'disambiguation'
> purpose, so it's really useless annoying line noise.
> 
> I propose to continue to use '0' until Qt can switch over to C++11
> and then re-open the discussion with 'nullptr - mildly encouraged'.

Fair enough.

Marc, what's your opinion?

Note that the disambiguation need not be only by the compiler. If the code is 
simpler to understand because you marked something as a pointer, then I think 
you should use the macro.

> >  * Q_DECL_FINAL - optional, use with care
> > 
> > There's a small benefit in optimisation by devirtualising calls. Use it
> > with care.
> > 
> > Use in public classes is discouraged, since people can compile with a
> > C++98
> > and derive / override what they shouldn't.
> 
> Another reason is that even if "we" think it should be final, the user of
> the class (or rather sub-classes) might disagree.

We make the call because we designed the code. If the class isn't intended to 
be derived from, then let the user know.

> >  * Q_DECL_OVERRIDE - required in new code, don't add to old code
> > 
> > Always add Q_DECL_OVERRIDE for any override in new classes you write. This
> > supersedes the need for the "virtual" keyword -- add it or not, it's up to
> > you.
> > 
> > DO NOT add to existing classes, including for new overrides to existing
> > classes. Clang 3.6 has a new warning about inconsistent use of this
> > keyword, so adding to one place requires adding to all overrides in that
> > class. Therefore, this needs to be done fully in each class and that's
> > not a "whitespace correction" error.
> 
> In my opinion Q_DECL_OVERRIDE brings sufficient benefit to generally
> encourage its use despite of the line noise it brings. However, since
> 'virtual' is not needed in case Q_DECL_OVERRIDE is used the rule should be
> to leave 'virtual' out/remove it in that case.

Indeed.

> [1] - All 'ok' under the assumption that this does not affect code that
> does not need to stay compatible with C++98. Such code should rather use
> C++11 keywords directly.

That's still per compiler, since compilers added support for the keywords at 
different times. Therefore, the use of the Q macro is required, unless you've 
already surrounded the code with the matching #ifdef.

For example:

#if defined(Q_COMPILER_CONSTEXPR) && defined(Q_COMPILER_UNIFORM_INIT)
    constexpr Foo() : array{1, 2, 3} ()
#endif

-- 
Thiago Macieira - thiago.macieira (AT) intel.com
  Software Architect - Intel Open Source Technology Center