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

[André Pönitz]

On Thu, Jan 08, 2015 at 02:33:34PM -0800, Thiago Macieira wrote:
> Hello
> 
> I think it's time to institute a policy that we should fix our sources to use 
> the new C++11 keywords.

That sounds good, but is not what is proposed in the text below. Instead,
the proposal is to use non-standard Q_KEYWORDS macros instead of standard
C++11 keywords.

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.

> 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'.

>For example, the new QIODevice::readLine overloads
> 
> 	QByteArray readLine(qint64 maxSize);
> 	bool readLine(QByteArray *line, qint64 maxSize = -1);
> 
> In C++11, you could write
> 	if (dev.readLine(Q_NULLPTR))	// read a line and discard


[In C++11 you could write 
  	if (dev.readLine(nullptr)) // read a line and discard]


>  * Q_DECL_EQ_DEFAULT - really discouraged
> [...]
>  * Q_DECL_EQ_DELETE - strongly encouraged, use with care
> [...]
>  * Q_DECL_CONSTEXPR / Q_DECL_RELAXED_CONSTEXPR - strongly encouraged

3 x Ok. [1]

>  * 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.

>  * 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.
 
>  * Q_CONSTEXPR - use only where required
>  [...]
>  * Q_DECL_NOTHROW / Q_DECL_NOEXCEPT - strongly encouraged 
>  [..]

2 x Ok. [1]

Andre'

[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.