[Development] Can we remove recommendation against unnamed namespaces from Qt coding conventions?

[Jøger Hansegård]

>> >>    Our Qt coding conventions ([1]https://wiki.qt.io/Coding_Conventions)
>> >>    has a statement on the use of unnamed (anonymous) namespaces. As far as
>> >>    I understand, this statement is now outdated.
>>
>> > [I'll assume we are talking about functions here.]
>>
>> Anonymous namespaces can be quite helpful to avoid name clashes of non-function types.
>
>Non-function types are practically not covered by the current rule, as it has
>an 'static [...] if possible' and there's no possibility to have 'static' on enums
>and types, and it has a different meaning for e.g. class members.
>
>> Quite often I see both local functions and other local entities in the same cpp
>> file. I don't want to recommend against moving local functions into the unnamed
>> namespace unless there is a technical reason why we shouldn't.
>
>I gave some numbers in the previous mail that I considered at the time of its
>writing a technical reason.
>
>What would be an acceptable 'technical reason' here?
>
>> > It would be a change to the worse in my book which I personally would not like.
>> > Anonymous namespaces are harder to set up and to maintain and produce at best
>> > identical results compared to static'
>>
>> Would an option be to change from:
>>
>>    Avoid the use of anonymous namespaces in favor of the static keyword
>>    if possible. A name localized to the compilation unit with static is
>>    guaranteed to have internal linkage. For names declared in anonymous
>>    namespaces the C++ standard unfortunately mandates external linkage.
>>    (7.1.1/6, or see various discussions about this on the gcc mailing
>>    lists)
>>
>> to
>>
>>    Use unnamed namespaces for all internal/non-exported entities. Functions can
>>    alternatively be marked `static` in the global namespace. Marking functions
>>    `static` also in the unnamed namespace can help readability, particularly in reviews.
>
>That's technically an option, but again not a good one in my book. There is no
>advantage /known to me/ /for functions/ to be in the anonymous namespace instead
>of being 'static', and I listed a few cases where there are disadvantages.
>
>This change also does not solve the "problem" of having /different/ rules than
>the (self-proclaimed...) "Core Guidelines" which was - to me - the main reason
>for the proposed change.
>
>Andre'

The main reason for changing the current convention on unnamed namespaces is
that the rationale is no longer correct, which makes it confusing to use in code
reviews. I only mention Cpp Core Guidelines because if we delete our item, Cpp
Core Guidelines would be the remaining guideline mentioning unnamed namespaces.
However, since it appears clear that the community disagree with the Cpp Core
Guidelines on this topic, we should keep a Qt specific convention on unnamed
namespaces.

Reading the Qt convention again, I read it as "do not replace `static` with
unnamed namespace", but does not forbid moving `static`
functions into unnamed namespaces if this otherwise would make sense, for
example if the function operates on data structures defined inside that
namespace. I can live with that.

But is the real question whether we should omit the `static` keyword in unnamed
namespaces or not? I added a couple of proposals. Which would you prefer?

Proposal 1

    Use unnamed namespaces for all internal/non-exported entities. Functions can
    alternatively be marked `static` in the global namespace. Marking functions
    `static` also in the unnamed namespace can help readability, particularly in
    reviews.

Proposal 2 (same as current convention, but with new rationale):

    Avoid the use of anonymous namespaces in favor of the static keyword if
    possible. Rationale: `static` is attached to individual functions, not a
    scope of uncertain extent. When working on unfamiliar code it helps to
    understand the context.

Proposal 3:

    Use the `static` keyword with local functions even if it is inside anonymous
    namespaces. Rationale: `static` is attached to individual functions, not a
    scope of uncertain extent. When working on unfamiliar code it helps to
    understand the context.

--
Jøger Hansegård