Too many C++ namespaces being used

[Robert O'Callahan]

Today I decided to figure out how many different C++ namespace names our
code uses (excluding third-party code). The answer is approximately 122. My
list is below. I'm only counting names used in header files.

The recommendation in the style guide (all lower-case) is not being
followed, and the proliferation has led to some errors, like JS vs js,
worker vs workers, layout vs css, and net vs network. Workers went bananas
with nested namespaces to the point of introducing macros like
BEGIN_WORKERS_NAMESPACE. People are having trouble figuring out which
namespace to put their code in.

I find this disturbing but I'm not quite sure what to do about it. I think
defaulting to putting things directly in the "mozilla" namespace is a good
idea when there's any doubt. Other than that, I'm not sure.

a11y
analyze
android
arm_private
ArrayType
AvailableMemoryTracker
base
battery
binding
browser
CanvasUtils
CClosure
CData
CheckedInt_internal
child
chromeworker
ClearOnShutdown_Internal
cli
CocoaFileUtils
CommandLineServiceMac
compositor
crash
CrashReporter
css
CType
ctypes
detail
docshell
dom
Elf
events
exceptions
file
filereadersync
filters
Framebuffer
frontend
FunctionType
gc
gcreason
gcstats
gfx
gl
hal
hal_sandbox
HangMonitor
ic
image
indexedDB
Int64
IPC
ipc
JS
js
jsdebugger
jsinspector
jsipc
jsperf
layers
layout
Library
location
mac_plugin_interposing
MapsMemoryReporter
memory
MemoryReporter
mjit
mozilla
MOZ_HAL_NAMESPACE
navigator
net
network
nsMenuUtilsX
nsStyleTransformMatrix
parent
places
plugins
PluginUtilsOSX
PointerType
power
Probes
probes
psm
qualified
reflect
roles
safebrowsing
scache
scriptloader
services
shadow
Shark
sms
sse_private
startup
states
statistics
storage
StructType
stubs
Telemetry
telephony
themeconst
threads
tl
tls
types
UInt64
unicode
utils
VersionFlags
WebGLTexelConversions
widget
windows
worker
workers
xhr
xpc
XPCNativeWrapper
xpconnect
XPCWrapper
XrayUtils



Rob
--
“You have heard that it was said, ‘Love your neighbor and hate your enemy.’
But I tell you, love your enemies and pray for those who persecute you,
that you may be children of your Father in heaven. ... If you love those
who love you, what reward will you get? Are not even the tax collectors
doing that? And if you greet only your own people, what are you doing more
than others?" [Matthew 5:43-47]

[Steve Fink]

On Wed 15 Feb 2012 03:16:05 AM PST, Robert O'Callahan wrote:
> The recommendation in the style guide (all lower-case) is not being
> followed, and the proliferation has led to some errors, like JS vs js,

That one is intentional, FWIW. JS:: is public, js:: is private. (Let's
not talk about Js:: for now.)

[Gijs Kruitbosch]

Having dealt with JS code in the past, I know that this has always been so
(IIRC, before namespaces it was a prefix in the function name, so JS_Foo and
js_foo differentiated public/private) but would the module owners/peers consider
changing it to be more descriptive, now that we do have 'real' namespaces?
Probably js and jsprivate or something? That might simplify matters for people
looking to use the public parts who are not (that) familiar with the module. If
the set of people that fit those criteria is (almost) empty, please ignore this
suggestion.

~ Gijs

[Justin Wood (Callek)]

Robert O'Callahan wrote:
> Today I decided to figure out how many different C++ namespace names our
> code uses (excluding third-party code). The answer is approximately 122. My
> list is below. I'm only counting names used in header files.
>
> The recommendation in the style guide (all lower-case) is not being
> followed, and the proliferation has led to some errors, like JS vs js,
> worker vs workers, layout vs css, and net vs network. Workers went bananas
> with nested namespaces to the point of introducing macros like
> BEGIN_WORKERS_NAMESPACE. People are having trouble figuring out which
> namespace to put their code in.
>
> I find this disturbing but I'm not quite sure what to do about it. I think
> defaulting to putting things directly in the "mozilla" namespace is a good
> idea when there's any doubt. Other than that, I'm not sure.

...

I would be interested in a tree-like view of the namespaces

mozilla::
| - services

etc.

[Joshua Cranmer]

On 2/15/2012 9:16 AM, Justin Wood (Callek) wrote:
> I would be interested in a tree-like view of the namespaces
>
> mozilla::
> | - services
>
> etc.
>

How does the one below look? Note: this only contains those namespaces
that hae at least one typedef or class in them; it does not include
namespaces with only global functions (I'm looking at you,
js::ctypes::CTypes and friends!). This is because I was lazy in
extracting the data from the SQLite database of DXR. It also doesn't
contain any namespace that wouldn't be compiled if you were on Linux.

android
base
base::subtle
chrome
CrashReporter
dwarf2reader
file_util
filters
gfx
google_breakpad
graphite2
graphite2::TtfUtil
graphite2::TtfUtil::Sfnt
graphite2::vm
IPC
js
JS
js::analyze
JSC
js::cli
js::cli::detail
js::crash
js::ctypes
JSC::X86Registers
JSC::Yarr
js::detail
js::gc
js::gcreason
js::gcstats
js::mjit
js::mjit::ic
js::mjit::stubs
js::Probes
js::shadow
js::tl
js::types
js::unicode
js::workers
mac_plugin_interposing
mozilla
mozilla::a11y
mozilla::a11y::roles
mozilla::browser
mozilla::CheckedInt_internal
mozilla::ClearOnShutdown_Internal
mozilla::css
mozilla::ctypes
mozilla::docshell
mozilla::docshell::POfflineCacheUpdate
mozilla::dom
mozilla::dom::battery
mozilla::dom::binding
mozilla::dom::indexedDB
mozilla::dom::network
mozilla::dom::PAudio
mozilla::dom::PBrowser
mozilla::dom::PContent
mozilla::dom::PContentDialog
mozilla::dom::PContentPermissionRequest
mozilla::dom::PCrashReporter
mozilla::dom::PExternalHelperApp
mozilla::dom::PMemoryReportRequest
mozilla::dom::power
mozilla::dom::PStorage
mozilla::dom::sms
mozilla::dom::sms::PSms
mozilla::dom::workers
mozilla::dom::workers::events
mozilla::dom::workers::xhr
mozilla::gfx
mozilla::gl
mozilla::hal
mozilla::hal_impl
mozilla::hal_sandbox
mozilla::hal_sandbox::PHal
mozilla::image
mozilla::ipc
mozilla::ipc::PDocumentRenderer
mozilla::ipc::PTestShell
mozilla::ipc::PTestShellCommand
mozilla::jsdebugger
mozilla::jsinspector
mozilla::jsipc
mozilla::jsipc::PContextWrapper
mozilla::jsipc::PObjectWrapper
mozilla::jsperf
mozilla::layers
mozilla::layers::PCompositor
mozilla::layers::PLayer
mozilla::layers::PLayers
mozilla::layout
mozilla::layout::PRenderFrame
mozilla::MapsMemoryReporter
mozilla::net
mozilla::net::PCookieService
mozilla::net::PFTPChannel
mozilla::net::PHttpChannel
mozilla::net::PNecko
mozilla::net::PWebSocket
mozilla::net::PWyciwygChannel
mozilla::places
mozilla::plugins
mozilla::plugins::parent
mozilla::plugins::PBrowserStream
mozilla::plugins::PPluginBackgroundDestroyer
mozilla::plugins::PPluginIdentifier
mozilla::plugins::PPluginInstance
mozilla::plugins::PPluginModule
mozilla::plugins::PPluginScriptableObject
mozilla::plugins::PPluginStream
mozilla::plugins::PPluginSurface
mozilla::plugins::PStreamNotify
mozilla::psm
mozilla::reflect
mozilla::safebrowsing
mozilla::scache
mozilla::storage
mozilla::Telemetry
mozilla::threads
mozilla::tls
mozilla::widget
nspr
ots
snappy
snappy::internal
std
tracked_objects
v8::internal
WTF
xpc
XPCWrapper

[Justin Wood (Callek)]

Joshua Cranmer wrote:
> On 2/15/2012 9:16 AM, Justin Wood (Callek) wrote:
>> I would be interested in a tree-like view of the namespaces
>>
>> mozilla::
>> | - services
>>
>> etc.
>>
>
> How does the one below look? Note: this only contains those namespaces
> that hae at least one typedef or class in them; it does not include
> namespaces with only global functions (I'm looking at you,
> js::ctypes::CTypes and friends!). This is because I was lazy in
> extracting the data from the SQLite database of DXR. It also doesn't
> contain any namespace that wouldn't be compiled if you were on Linux.

It looks quite good actually..

At a glance the nesting depth does not look that bad, while some names
are clearly confusing (what is <toplevel>::chrome for example)...

The most obvious fruit I see is |<toplevel>::gfx| and |mozilla::gfx|
should likely be merged, most likely under mozilla::

--
~Justin Wood (Callek)

[Joe Drew]

On 2012-02-15 8:46 PM, Justin Wood (Callek) wrote:
> The most obvious fruit I see is |<toplevel>::gfx| and |mozilla::gfx|
> should likely be merged, most likely under mozilla::

I'd be very surprised if ::gfx actually exists in Mozilla code; I rather
suspect it to be a bug in the namespace-finding code.

(With the obvious caveat that I reserve the right to be proven wrong on
this with a link to mxr.)

joe

[Joshua Cranmer]

Like this one?
<http://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/base/gfx/point.h>

[Bobby Holley]

On Wed, Feb 15, 2012 at 8:21 PM, Joshua Cranmer <Pidg...@verizon.net>wrote:

> On 2/15/2012 9:44 PM, Joe Drew wrote:
>
>>
>> On 2012-02-15 8:46 PM, Justin Wood (Callek) wrote:
>>
>>> The most obvious fruit I see is |<toplevel>::gfx| and |mozilla::gfx|
>>> should likely be merged, most likely under mozilla::
>>>
>>
>> I'd be very surprised if ::gfx actually exists in Mozilla code; I rather
>> suspect it to be a bug in the namespace-finding code.
>>
>

> Like this one? <http://mxr.mozilla.org/**mozilla-central/source/ipc/**
> chromium/src/base/gfx/point.h<http://mxr.mozilla.org/mozilla-central/source/ipc/chromium/src/base/gfx/point.h>
> >


That's not really Mozilla code...

[Jeff Walden]

On 02/15/2012 04:18 AM, Gijs Kruitbosch wrote:
> Having dealt with JS code in the past, I know that this has always been so (IIRC, before namespaces it was a prefix in the function name, so JS_Foo and js_foo differentiated public/private) but would the module owners/peers consider changing it to be more descriptive, now that we do have 'real' namespaces? Probably js and jsprivate or something? That might simplify matters for people looking to use the public parts who are not (that) familiar with the module. If the set of people that fit those criteria is (almost) empty, please ignore this suggestion.

JS and js are short, which for some people matters above all other concerns. If I thought we could tolerate arguments on every single little decision, I'd argue it. But no group of people is capable of that, of course, and there are more important things to argue about, so I've left it alone.

Jeff

[Joshua Cranmer]

On 2/15/2012 5:16 AM, Robert O'Callahan wrote:
> I find this disturbing but I'm not quite sure what to do about it. I think
> defaulting to putting things directly in the "mozilla" namespace is a good
> idea when there's any doubt. Other than that, I'm not sure.

There are a few options, I think:
1. Do not allow the creation of a namespace mozilla::foo unless foo is a
module.
2. Do not allow the creation of a namespace mozilla::foo unless foo is a
top-level directory in mozilla/
3. Place all classes that need to be exported cross-module in the
mozilla namespace.

As some more general guidelines, I don't think there is much point in
creating further subnamespaces. The 8 subnamespaces of mozilla::dom seem
like overkill to me, and mozilla::dom::workers::events just shouldn't
exist. I think another rule of thumb could be "if you're going to write
baz::foo::FooHelper, you've got one namespace too many".

Looking at the other major projects I've been working on--LLVM and
clang--they get buy with just two namespaces for the most part (llvm::
and clang::), with a few others for small portions of code where the
class names themselves are short (e.g., llvm::cl::Opt or
llvm::sys::Path). Functionally, Mozilla breaks down into SpiderMonkey,
Necko, Gecko, XPCOM, Toolkit, NSPR, and NSS (with comm-central adding
mailnews and calendar) as parts which are more or less (in theory, if
not so much in practice :-P) independent from each other; subdividing
much further from that is suboptimal IMO. Given that the ns
pseudo-namespace scheme doesn't seem to have caused too many problems
with naming conflicts for its 10 years of existence, I don't think a
fine-grained naming scheme buys anyone anything apart from headaches
when trying to use code.

[Ehsan Akhgari]

In C++, when you see a name in a source file, it is *very* difficult to
determine which namespace it lives under. My normal workflow is something
like: look for the name declaration, continue looking upward in the source
file for namespace declarations, and figure out the fully qualified name,
and then reference the symbol using the required namespaces. This is very
painful in practice and frequently causes unneeded recompilation cycles for
me when I start to touch parts of the code base which I'm not quite
familiar with.

This is an argument against using too many namespaces. Things get worse if
you use nested namespaces too much (see the workflow above).

So, if we were back at the drawing board today and were trying to figure
out how to use namespaces, here's what I would suggest:

1. Put our own code (sans things like NSS and NSPR) under namespace mozilla
(hmm, well, with the JS engine being the obvious exception :/ ).
2. Do not allow any other namespace at the global scope for our own code.
3. For the few exceptional cases where name clashes can actually occur
conceivably, use namespaces denoting the module name all in lower case
nested *only* one level under ::mozilla.

For example, under this set of rules, mozilla::gfx::Path sounds like a
reasonable name (as the Path name could conceivably be used by other
modules, however, mozilla::gfx::SkiaPath does not make any sense, and
should be changed to mozilla::SkiaPath.

If we were to adhere to these rules, we would gain two important benefits:
very little confusion on where names come from (all of our code would be
under namespace mozilla) and very little confusion on where names should go
(under mozilla::, unless you can justfiy why the name in question is so
common which demands living under mozilla::<module_name>).

One possible objection to this proposal would be different teams wanting
different namespaces for their own code. I personally tend to think that
tendency would lead to duplicating information which is already quite
evident in our code (like, if your file lives under the gfx/ directory,
it's part of the Graphics module.)

I'd be interested to hear people's thoughts on this.

Cheers,
--
Ehsan
<http://ehsanakhgari.org/>

> ______________________________**_________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/**listinfo/dev-platform<https://lists.mozilla.org/listinfo/dev-platform>
>

[Boris Zbarsky]

On 2/22/12 10:59 PM, Ehsan Akhgari wrote:
> I'd be interested to hear people's thoughts on this.

Please make it so.

-Boris

[David Rajchenbach-Teller]

Let me speak as a namespace (ab)user.

One of the things I have been doing a lot recently is defining
JavaScript objects/classes in C++. Each object is typically defined by:
- instance methods;
- instance properties;
- prototype methods;
- prototype properties;
- regular constructors;
- named constructors/factories;
- finalizer, serializer, etc.;
- various utility functions that may or may not implement the same
functionality as the previous methods/properties/constructors/factories.

It is not uncommon to have, for instance, an instance method |Foo|, a
prototype method |Foo| and a utility function |Foo| containing the code
shared by the instance method and the prototype method.

Multiply this by all the JavaScript objects and you can quickly end up
with a large mess of confusingly named symbols. Hence the use of
namespaces. I personally like:
- one namespace for the object/class, including utility functions,
finalizer, serializer;
- one sub-namespace for instance methods/properties;
- one sub-namespace for prototype methods/properties;
- explicit use of namespaces.

Right now, for this specific problem, I do not see any better solution.

Cheers,
David

> I'd be interested to hear people's thoughts on this.
>

> Cheers,
> --
> Ehsan
> <http://ehsanakhgari.org/>
>
>
> On Fri, Feb 17, 2012 at 9:42 PM, Joshua Cranmer <Pidg...@verizon.net>wrote:
>

>> ______________________________**_________________
>> dev-platform mailing list
>> dev-pl...@lists.mozilla.org
>> https://lists.mozilla.org/**listinfo/dev-platform<https://lists.mozilla.org/listinfo/dev-platform>
>>

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform


--
David Rajchenbach-Teller, PhD
Performance Team, Mozilla

[Ehsan Akhgari]

I don't really know what code you're talking about, but this seems very
scary to me. :(

I hope that all of these namespaces are defined in a single translation
unit, wrapped in an anonymous namespace. ;-)

--
Ehsan
<http://ehsanakhgari.org/>

[Nicholas Nethercote]

On Thu, Feb 23, 2012 at 2:04 AM, David Rajchenbach-Teller
<dte...@mozilla.com> wrote:
> It is not uncommon to have, for instance, an instance method |Foo|, a
> prototype method |Foo| and a utility function |Foo| containing the code
> shared by the instance method and the prototype method.
>
> Multiply this by all the JavaScript objects and you can quickly end up
> with a large mess of confusingly named symbols. Hence the use of
> namespaces. I personally like:
> - one namespace for the object/class, including utility functions,
> finalizer, serializer;
> - one sub-namespace for instance methods/properties;
> - one sub-namespace for prototype methods/properties;
> - explicit use of namespaces.
>
> Right now, for this specific problem, I do not see any better solution.

What's wrong with vanilla prefixes or suffixes? Foo_object,
Foo_instanceMethod, Foo_prototypeMethod, something like that?

Nick

[Boris Zbarsky]

On 2/23/12 5:44 PM, Ehsan Akhgari wrote:
> I hope that all of these namespaces are defined in a single translation
> unit, wrapped in an anonymous namespace. ;-)

I think within a translation unit people should be free to do whatever
the reviewer lets them get away with.

For exposed stuff, I don't think we should be forcing API consumers to
deal with deeply nested namespaces (where by "deeply" I mean "more than
just mozilla::").

-Boris

[David Rajchenbach-Teller]

On 2/24/12 12:51 AM, Nicholas Nethercote wrote:

>> Right now, for this specific problem, I do not see any better solution.
>
> What's wrong with vanilla prefixes or suffixes? Foo_object,
> Foo_instanceMethod, Foo_prototypeMethod, something like that?

How is that better than namespaces? This is just encoding the namespace
inside the function name.

Cheers,
David

[David Rajchenbach-Teller]

Ah, yes, of course, I forgot to emphasize that none of these namespaces
is exported.

I fully agree that the situation is different for publicly visible
namespaces.

[Igor Bukanov]

On 24 February 2012 09:59, David Rajchenbach-Teller <dte...@mozilla.com> wrote:
> How is that better than namespaces? This is just encoding the namespace
> inside the function name.

It helps the reader to understand where the name belongs to for the
price of some name clatter and efforts to invent good short prefixes
for the writer. For this reason I would also like to see minimal
namespace usage with unprefixed short names used for class method
names or nested classes.

[Robert O'Callahan]

Me too!

(Although I don't actually encounter Ehsan's problem since Eclipse can
usually find definitions for me.)

[Ehsan Akhgari]

On Fri, Feb 24, 2012 at 6:39 AM, Robert O'Callahan <rob...@ocallahan.org>wrote:

> On Thu, Feb 23, 2012 at 6:34 PM, Boris Zbarsky <bzba...@mit.edu> wrote:
>

> Me too!
>
> (Although I don't actually encounter Ehsan's problem since Eclipse can
> usually find definitions for me.)
>

Well, try using vim!

Is this a good time to derail this thread to a conversation about favorite
text editors? ;-)

But seriously, I hate for these kinds of ideas to get lost in threads.
What is the next thing to do here? Filing bugs?

--
Ehsan
<http://ehsanakhgari.org/>

[Robert O'Callahan]

First, get this guidance officially adopted and into the style guide. I'm
not sure what more we need to do, if anything, to make this "official".

[Ehsan Akhgari]

OK, I have modified the style guidelines to reflect these changes:

<
https://developer.mozilla.org/En/Developer_Guide/Coding_Style#C.2B.2B_namespaces
>

Hope people would start following these guidelines. :-)

Cheers,
--
Ehsan
<http://ehsanakhgari.org/>

On Sat, Feb 25, 2012 at 12:27 PM, Robert O'Callahan <rob...@ocallahan.org>wrote:

> First, get this guidance officially adopted and into the style guide. I'm
> not sure what more we need to do, if anything, to make this "official".
>
>

[Scott Johnson]

On 02/27/2012 01:21 PM, thus spoke Ehsan Akhgari:

> OK, I have modified the style guidelines to reflect these changes:

Perhaps it would be a good idea to make an announcement at one of the
Monday all-hands and/or in a blog post, just so we can call attention to
it?

~Scott

> Hope people would start following these guidelines. :-)
>

> Cheers,
> --
> Ehsan
> <http://ehsanakhgari.org/>
>
>

> On Sat, Feb 25, 2012 at 12:27 PM, Robert O'Callahan<rob...@ocallahan.org>wrote:
>
>> First, get this guidance officially adopted and into the style guide. I'm
>> not sure what more we need to do, if anything, to make this "official".
>>
>>

>> Rob
>> --
>> “You have heard that it was said, ‘Love your neighbor and hate your
>> enemy.’ But I tell you, love your enemies and pray for those who persecute
>> you, that you may be children of your Father in heaven. ... If you love
>> those who love you, what reward will you get? Are not even the tax
>> collectors doing that? And if you greet only your own people, what are you
>> doing more than others?" [Matthew 5:43-47]
>>
>>

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

[Ehsan Akhgari]

I will speak about this at the developers meeting tomorrow.

Cheers,
--
Ehsan
<http://ehsanakhgari.org/>

On Mon, Feb 27, 2012 at 3:57 PM, Scott Johnson <sjoh...@mozilla.com> wrote:

> On 02/27/2012 01:21 PM, thus spoke Ehsan Akhgari:
>
> OK, I have modified the style guidelines to reflect these changes:
>>
> Perhaps it would be a good idea to make an announcement at one of the
> Monday all-hands and/or in a blog post, just so we can call attention to it?
>
> ~Scott
>
>> Hope people would start following these guidelines. :-)
>>

>> Cheers,
>> --
>> Ehsan
>> <http://ehsanakhgari.org/>
>>
>>

>> On Sat, Feb 25, 2012 at 12:27 PM, Robert O'Callahan<robert@ocallahan.**
>> org <rob...@ocallahan.org>>wrote:

>>
>> First, get this guidance officially adopted and into the style guide. I'm
>>> not sure what more we need to do, if anything, to make this "official".
>>>
>>>

>>> Rob
>>> --
>>> “You have heard that it was said, ‘Love your neighbor and hate your
>>> enemy.’ But I tell you, love your enemies and pray for those who
>>> persecute
>>> you, that you may be children of your Father in heaven. ... If you love
>>> those who love you, what reward will you get? Are not even the tax
>>> collectors doing that? And if you greet only your own people, what are
>>> you
>>> doing more than others?" [Matthew 5:43-47]
>>>
>>>

[Robert O'Callahan]

On Tue, Feb 28, 2012 at 8:21 AM, Ehsan Akhgari <ehsan....@gmail.com>wrote:

> OK, I have modified the style guidelines to reflect these changes:
>
> <

> https://developer.mozilla.org/En/Developer_Guide/Coding_Style#C.2B.2B_namespaces

> >
>
> Hope people would start following these guidelines. :-)
>

And we should convert existing code to follow them, otherwise people will
just copy the bad patterns.

[Ehsan Akhgari]

On Tue, Feb 28, 2012 at 6:11 AM, Robert O'Callahan <rob...@ocallahan.org>wrote:

> On Tue, Feb 28, 2012 at 8:21 AM, Ehsan Akhgari <ehsan....@gmail.com>wrote:
>
>> OK, I have modified the style guidelines to reflect these changes:
>>
>> <
>> https://developer.mozilla.org/En/Developer_Guide/Coding_Style#C.2B.2B_namespaces
>> >
>>
>> Hope people would start following these guidelines. :-)
>>
>
> And we should convert existing code to follow them, otherwise people will
> just copy the bad patterns.
>

Yes, I think we should file specific bugs about existing code.

There is one additional point brought up by jlebar on IRC. Some headers
need to declare names which are not meant to be used by code not living in
the same header (
http://mxr.mozilla.org/mozilla-central/source/xpcom/base/ClearOnShutdown.hbeing
an example.) I would like to propose that such code should live in
the mozilla::detail namespace. Does anybody object to this? If not I will
update the coding style guidelines to reflect this as well.

[Justin Lebar]

> There is one additional point brought up by jlebar on IRC.  Some headers
> need to declare names which are not meant to be used by code not living in
> the same header (
> http://mxr.mozilla.org/mozilla-central/source/xpcom/base/ClearOnShutdown.hbeing
> an example.)  I would like to propose that such code should live in
> the mozilla::detail namespace.  Does anybody object to this?  If not I will
> update the coding style guidelines to reflect this as well.

This seems to defeat the purpose of namespaces, for code in
mozilla::detail. Under this scheme, code in mozilla::detail can
collide and see all other code that's theoretically hidden.

If this code were in a .cpp file, it'd be in an anonymous namespace.
If we're trying to model that in a header file, then having a
namespace called "mozilla::headername_internal" is a much closer
approximation than "mozilla::detail".

Creating a separate headername_internal namespace means of course that
we'd have more namespaces in total. I don't think this is a problem
in itself. Because these are header-internal namespaces, nobody else
has to call them, or even be aware of them.

Anyway, I don't want to bikeshed about this; it's not a big deal either way.

-Justin

[Jeff Walden]

On 02/28/2012 11:30 AM, Ehsan Akhgari wrote:
> There is one additional point brought up by jlebar on IRC. Some headers
> need to declare names which are not meant to be used by code not living in
> the same header (
> http://mxr.mozilla.org/mozilla-central/source/xpcom/base/ClearOnShutdown.hbeing
> an example.) I would like to propose that such code should live in
> the mozilla::detail namespace. Does anybody object to this? If not I will
> update the coding style guidelines to reflect this as well.

I think I suggested this on IRC, but I don't think I suggested it particularly clearly. Rather, a detail namespace at any nesting level is reserved for implementation details. So mozilla::detail is reserved, mozilla::dom::detail is reserved, js::detail is reserved, and so on. Code which is already worried about conflicts would be in a nested namespace, and therefore it would have a detail namespace with that many fewer collisions.

Contra jlebar, though, I don't believe the purpose of a namespace for this particular case is to avoid collisions -- rather it's to cordon off interfaces and functionality we'd rather not expose, but C++ requires us to expose. It seems unlikely to me that conflicts will be much of a problem in practice for internal-by-intent stuff.

Jeff

[Justin Lebar]

> Contra jlebar, though, I don't believe the purpose of a namespace for this
> particular case is to avoid collisions -- rather it's to cordon off
> interfaces and functionality we'd rather not expose, but C++ requires us to
> expose. It seems unlikely to me that conflicts will be much of a problem in
> practice for internal-by-intent stuff.

I wasn't so clear either, but I agree that "cordoning off" is an
important consideration:

> Under this scheme, code in mozilla::detail can collide **and see** all other

> code that's theoretically hidden.

That is, anybody in mozilla::detail can call anyone else in
mozilla::detail, when in fact each file's mozilla::detail wants to be
cordoned off from all others.

I agree this isn't a big problem in practice, because the
mozilla::detail sections are likely to be relatively small in
practice.

But I don't see the practical advantage of a global mozilla::detail
over a per-file mozilla::this-file's-detail.

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

[Jeff Walden]

On 02/29/2012 10:02 AM, Justin Lebar wrote:
> That is, anybody in mozilla::detail can call anyone else in
> mozilla::detail, when in fact each file's mozilla::detail wants to be
> cordoned off from all others.

Sure. "Don't do that." :-) Reviewers should catch and reject this sort of bizarre cross-file anti-dependency. I suspect it wouldn't be too hard to create a clang lint or whatever that would check for violations of that requirement, too.

> But I don't see the practical advantage of a global mozilla::detail
> over a per-file mozilla::this-file's-detail.

It's an emerging convention in other projects, seems nice to use it for consistency for people coming from other codebases, to cut down on the things they need to learn. Boost uses it, it's claimed game developers use it, and Luke says he's seen it elsewhere (hence why he started using it):

http://stackoverflow.com/questions/2082156/hiding-private-constants-in-an-inline-namespace-header
http://www.gamedev.net/topic/505432-c-private-namespace-is-this-customary/page__view__findpost__p__4296986

Jeff

[Justin Lebar]

On Wed, Feb 29, 2012 at 2:49 PM, Jeff Walden <jwald...@mit.edu> wrote:
> On 02/29/2012 10:02 AM, Justin Lebar wrote:
>>
>> That is, anybody in mozilla::detail can call anyone else in
>> mozilla::detail, when in fact each file's mozilla::detail wants to be
>> cordoned off from all others.
>
>
> Sure.  "Don't do that."  :-)

If we're relying on "Don't do that" then we don't need mozilla::detail
at all. The whole point is to have an automatic "don't do that"
check, isn't it?

Why do we want an automatic don't-do-that check for preventing mozilla
code from using mozilla::detail code, whereas we don't want a
don't-do-that check for preventing mozilla::detail code in header X
from accessing mozilla::detail code in header Y?

>> But I don't see the practical advantage of a global mozilla::detail
>> over a per-file mozilla::this-file's-detail.
>
> It's an emerging convention in other projects, seems nice to use it for
> consistency for people coming from other codebases, to cut down on the
> things they need to learn.  Boost uses it, it's claimed game developers use
> it, and Luke says he's seen it elsewhere (hence why he started using it):

"Everyone else is doing it" is not particularly convincing to me. Of
the myriad things one has to learn about Mozilla, our header-private
namespace convention is perhaps the most minor imaginable.

Nobody is stepping up to defend per-file namespaces, so I guess I
should concede. I just don't get it...

-Justin

[Jeff Walden]

On 02/29/2012 12:17 PM, Justin Lebar wrote:
> If we're relying on "Don't do that" then we don't need mozilla::detail
> at all. The whole point is to have an automatic "don't do that"
> check, isn't it?

The point is to make clear that those bits are purely implementation details that shouldn't be used directly. Without improvements to C++, there's no way to automatically enforce this; "namespace ClearOnShutdown_internal" makes things no more automatically-private than "namespace detail" does. So if we were to use the former, we'd still need to rely on "don't do that" anyway.

Jeff

[Karl Tomlinson]

If someone used namespace ClearOnShutdown_internal from another
file it would be much more obvious than if someone used it from
namespace detail in another file.