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

[apoenitz]

On Fri, Feb 23, 2024 at 09:49:17AM +0000, Jøger Hansegård via Development wrote:
> >> [...]
> >> 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.

A static function does not have to be in an unnamed namespace to operate on
data structures defined there. So I don't think the 'otherwise would make
sense' condition is met regularly, if at all.

The only reason I can think of where putting a static function into an unnamed
namespace may have /some/ advantage is when this is a lonely function amidst
class/enum definitions in an unnamed namespace and one doesn't want to close
and re-open this just to have the 'static' on top level. This should be
a rare case, and I care little whether in this case that static is inside the
namespace or on top-level, as long as it is there.

> But is the real question whether we should omit the `static` keyword in unnamed
> namespaces or not?

The reasons 1-5 and 7 I gave previously for the top-level case still apply
when the static function is wrapped in an unnamed namespace, too. So my
answer to the question is "It should not be omitted".

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

Proposal 3 sounds ok to me.

Andre'