Namespace usage clarifications & rules

[Brett Wilson]

Namespace usage has been a bit ad-hoc. After discussing what to do
with a number of people, the following is an attempt to reconcile the
style guide, common usage in Chrome and at Google, what would be most
clear, and what's practical to do:

- Use namespaces based on the path, and not invent special namespaces
for a single file. This matches with the style guide "choose the name
based on the project's path" and also the dominant usage at Google.
This means that in general we shouldn't have random "file_util" and
"download_drag_util" namespaces. In some cases, classes with static
functions may make sense. In other cases, if you find yourself needing
to group functions like this, it probably means your function names
aren't good enough or your directory structure (and the namespaces
derived from it) don't make sense. As an example, file_util::Foo()
isn't super helpful. One approach might be to move everything
file-related into src/base/filesystem and use base::filesystem::Foo().

- Exceptions to the above rule in common usage at Google are
"myfeature::internal" (meaning "even though this is in a header file
you can use, you shouldn't be calling this") and "myfeature::subtle"
(meaning "even though this is in a header file you can use, you're not
smart enough to use it properly"). The "subtle" one normally indicates
there's a safer way to do what you want for most common uses.

- "Library" toplevel projects like base, app, net, ipc, printing, and
webkit (as opposed to "leaf" projects like chrome) should always use a
namespace matching their directory name. This means that everything in
src/base (yes, even string16) should be in the "base" namespace. Since
these projects can be re-used in different projects, making sure
everything is unique and easy to identify is especially important.

- Subdirectories in these library toplevel projects may or may not use
a separate namespace, but if they do it should be of the form
<toplevel_dir>::<nested_dir> as in "base::win::FooBar()". You don't
need the nested namespace if it doesn't make sense: some
subdirectories are made more for organizing files and don't
necessarily justify separate namespaces, and some subdirectories may
have a naming scheme that separates them without a namespace (all
names in src/net/url_request starts with "URLRequest" so a namespace
"net::url_request::URLRequest" would be redundant). You should be
consistent, either using a nested namespace for everything in a
directory, or only using the parent namespace.

- "Leaf" projects like Chrome and test_shell that aren't built into
anything else may or may not use namespaces based on their root,
depending on project style. It would be nice to require consistent
usage, but it's not practical to add the "chrome" namespace to all
3000 .cc files in src/chrome, and doing so wouldn't get us much, since
it's not designed to be built into anything else.

- If your feature in src/chrome has more than a couple of files, it's
good practice to add a namespace corresponding to that feature (which
should be the same as the directory name). Good examples are "history"
and "downloads". In this case, treat your feature like a library
toplevel project above and be consistent.

- In general, you don't need to use the very general "chrome" or
"browser" namespace unless its usage clarifies something. They can be
useful in some cases as a way of calling out that something like a
constant is the "chrome blahblah" when it might appear overly general
or unclear otherwise, but doesn't help anything for most normal code.
Another example is the code in browser/net which would be better as
"browser::net" so as to not be confused with the "net" toplevel
module's namespace.

[William Chan (陈智昌)]

I agree with everything above.  The stuff below I weakly disagree with.  I don't intend to fight hard for it, since it really doesn't matter that much, but I just wanted to raise a counterpoint and see if anyone else agrees.  I don't want to start a bike shedding discussion, so hopefully this won't degrade into one.  Let me reiterate that I agree with the majority of the guidelines set here and am willing to adopt them as is, without any changes, if that's what everyone else wants.

That said, it seems to me that although we agree that it would be nice to require consistent usage, we are giving up on it because it's too much effort for too little gain.  While I agree that it's probably not worth the effort to actively spend developers' time moving code into the chrome namespace, I think that it's reasonable to say that all new code under chrome/ should go into the chrome namespace.  This is what is done in google3.  Existing code is fine as is, but newly written code always needs to be in a namespace.  The readability review template that some readability reviewers use includes a section to remind people of this if necessary.  I think that there's enough active development and new code written and rewritten that using the chrome toplevel namespace can become a reasonably strong convention in not too long.

Also, while Chrome is currently considered "leaf", I can see some of the code under chrome/ being used outside of chrome/.  There are these weird forks of Chromium that may well be writing their own "leaf" projects that use code from chrome/.  I know of at least one project that uses some chrome/browser/net/ code, which somehow manages to work since its dependencies are pretty thin.

So, I think it's a good rule to follow that all new code goes into a toplevel namespace.  But that's just my two cents. Thanks for sending this out and finally bringing some consistency to the namespace madness!

 

- "Leaf" projects like Chrome and test_shell that aren't built into
anything else may or may not use namespaces based on their root,
depending on project style. It would be nice to require consistent
usage, but it's not practical to add the "chrome" namespace to all
3000 .cc files in src/chrome, and doing so wouldn't get us much, since
it's not designed to be built into anything else.

- If your feature in src/chrome has more than a couple of files, it's
good practice to add a namespace corresponding to that feature (which
should be the same as the directory name). Good examples are "history"
and "downloads". In this case, treat your feature like a library
toplevel project above and be consistent.

- In general, you don't need to use the very general "chrome" or
"browser" namespace unless its usage clarifies something. They can be
useful in some cases as a way of calling out that something like a
constant is the "chrome blahblah" when it might appear overly general
or unclear otherwise, but doesn't help anything for most normal code.
Another example is the code in browser/net which would be better as
"browser::net" so as to not be confused with the "net" toplevel
module's namespace.

--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
   http://groups.google.com/a/chromium.org/group/chromium-dev

[Peter Kasting]

On Wed, Jan 5, 2011 at 5:46 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

That said, it seems to me that although we agree that it would be nice to require consistent usage, we are giving up on it because it's too much effort for too little gain.  While I agree that it's probably not worth the effort to actively spend developers' time moving code into the chrome namespace, I think that it's reasonable to say that all new code under chrome/ should go into the chrome namespace.

Is your only motivation for this this next bit?:

 

while Chrome is currently considered "leaf", I can see some of the code under chrome/ being used outside of chrome/.

Or are there other gains you also see this giving us?

PK

[William Chan (陈智昌)]

Elimination of exceptions => simpler rules.  Instead of having to think about library vs leaf projects, just have the simple rule that all new code goes into the toplevel namespace based on path.

Rather than asking what the gain is of being consistent, I'd rather ask, what's the gain of being inconsistent?  I guess I'm mainly questioning the rationale that because we have a lot of chrome code that isn't in the chrome namespace, new code doesn't have to go into a namespace.

Again, I don't feel too strongly about it, so hopefully you won't engage me in too much debate over this detail, and if you disagree, will just say you disagree, which I'm more than willing to accept :)

PK

[Peter Kasting]

On Wed, Jan 5, 2011 at 6:02 PM, William Chan (陈智昌) <will...@chromium.org> wrote:

Elimination of exceptions => simpler rules.  Instead of having to think about library vs leaf projects, just have the simple rule that all new code goes into the toplevel namespace based on path.

Rather than asking what the gain is of being consistent, I'd rather ask, what's the gain of being inconsistent?  I guess I'm mainly questioning the rationale that because we have a lot of chrome code that isn't in the chrome namespace, new code doesn't have to go into a namespace.

OK.  I mainly wanted to understand your point of view better.

Again, I don't feel too strongly about it, so hopefully you won't engage me in too much debate over this detail, and if you disagree, will just say you disagree, which I'm more than willing to accept :)

I don't feel terribly strongly either (really, about almost any of this stuff).  I am willing to believe that there's very little gain but much cost to namespacing everything in chrome and I'm also willing to believe that it's not a big cost and would be nice.  I leave it to Brett and anybody else who looked deeply at this to make the rules.

PK 

[Brett Wilson]

Frankly the "leaf" project rule was reverse-engineered from the
"realistically nobody is going to fix chrome" position. We are on a
path to get everything in base, app, net, and gfx basically using the
right namespaces in the foreseeable, if not too-close future. I'd
rather have no-namespace for Chrome than a forever "random bits of the
code use the Chrome namespace so you never know whether to type
chrome:: or not" situation.

Brett

[Darin Fisher]

Yeah, I find it really annoying when coding in net/url_request that I need to

use net:: everywhere.  However, at least in that case it is fairly predictable

as everything that is not URLRequest* is already in the net namespace.

Hmm, but perhaps we could mechanically add everything under chrome/

to the chrome:: namespace using a script?

(One advantage to using the chrome:: namespace is that it would cause

symbol names, when unmangled, to be more meaningful.)

-Darin

[danakj]

Hello,

Don't mind me while I raise this 5 year necro thread.

On Wednesday, January 5, 2011 at 12:46:52 PM UTC-8, Brett Wilson wrote:

Namespace usage has been a bit ad-hoc. After discussing what to do
with a number of people, the following is an attempt to reconcile the
style guide, common usage in Chrome and at Google, what would be most
clear, and what's practical to do:

 

- Use namespaces based on the path, and not invent special namespaces
for a single file. This matches with the style guide "choose the name
based on the project's path" and also the dominant usage at Google.
This means that in general we shouldn't have random "file_util" and
"download_drag_util" namespaces. In some cases, classes with static
functions may make sense. In other cases, if you find yourself needing
to group functions like this, it probably means your function names
aren't good enough or your directory structure (and the namespaces
derived from it) don't make sense. As an example, file_util::Foo()
isn't super helpful. One approach might be to move everything
file-related into src/base/filesystem and use base::filesystem::Foo().

The "not invent special namespaces for a single file" clause here is counter to some pretty clearly worded text in the Google C++ Style Guide, which says "Rather than creating classes only to group static member functions which do not share static data, use namespaces instead."

https://google.github.io/styleguide/cppguide.html#Nonmember,_Static_Member,_and_Global_Functions

Is this something that has changed, because the Chromium style guide doesn't mention anything on the subject. Is this still not a dominant practice at Google? We should probably add something to our style guide if we're going to differ on something worded so clearly there. But I thought I'd ask if this rule still makes sense 5 years later.

[Darin Fisher]

I think the advice to avoid dumping ground files (like file_util.h) is still relevant and is different than the issue you are raising?

-Darin

--

[dan...@chromium.org]

On Wed, Jun 29, 2016 at 5:40 PM, Darin Fisher <da...@chromium.org> wrote:

I think the advice to avoid dumping ground files (like file_util.h) is still relevant and is different than the issue you are raising?

Ya, I'm asking about the advice which says to not make a namespace for a single file, the other option is to leave them in a more general namespace (which makes things less clear), or make a class of only static methods (which the google styleguide explicitly calls out as a no).

Concrete examples:

https://cs.chromium.org/chromium/src/cc/base/math_util.h - A class of only-static methods to group them under a scope.

vs 

https://cs.chromium.org/chromium/src/content/common/gpu/client/command_buffer_metrics.h - A namespace for the file to group methods/enums under a scope.

I did the latter one recently after reading this part of the Google style guide, but I've seen reviewers asking people not to make a namespace and citing this thread more recently.

[Jeremy Roman]

On Wed, Jun 29, 2016 at 8:45 PM, <dan...@chromium.org> wrote:

On Wed, Jun 29, 2016 at 5:40 PM, Darin Fisher <da...@chromium.org> wrote:

I think the advice to avoid dumping ground files (like file_util.h) is still relevant and is different than the issue you are raising?

Ya, I'm asking about the advice which says to not make a namespace for a single file, the other option is to leave them in a more general namespace (which makes things less clear), or make a class of only static methods (which the google styleguide explicitly calls out as a no).

Concrete examples:

https://cs.chromium.org/chromium/src/cc/base/math_util.h - A class of only-static methods to group them under a scope.

vs 

https://cs.chromium.org/chromium/src/content/common/gpu/client/command_buffer_metrics.h - A namespace for the file to group methods/enums under a scope.

I did the latter one recently after reading this part of the Google style guide, but I've seen reviewers asking people not to make a namespace and citing this thread more recently.

I wasn't here 5 years ago, but my two cents: I prefer to avoid the class-with-only-static-members pattern (and the upstream style guide agrees), but even the namespace pattern should probably be uncommon, and should also correspond to the directory structure where reasonable, even though the last bit might be a basename rather than a directory name.

[Darin Fisher]

On Wed, Jun 29, 2016 at 5:45 PM, <dan...@chromium.org> wrote:

On Wed, Jun 29, 2016 at 5:40 PM, Darin Fisher <da...@chromium.org> wrote:

I think the advice to avoid dumping ground files (like file_util.h) is still relevant and is different than the issue you are raising?

Ya, I'm asking about the advice which says to not make a namespace for a single file, the other option is to leave them in a more general namespace (which makes things less clear), or make a class of only static methods (which the google styleguide explicitly calls out as a no).

Concrete examples:

https://cs.chromium.org/chromium/src/cc/base/math_util.h - A class of only-static methods to group them under a scope.

vs 

https://cs.chromium.org/chromium/src/content/common/gpu/client/command_buffer_metrics.h - A namespace for the file to group methods/enums under a scope.

I did the latter one recently after reading this part of the Google style guide, but I've seen reviewers asking people not to make a namespace and citing this thread more recently.

I see. My take is that we should follow the style guide as it helps us have less debate about style. We can have exceptions, but they should be rare.

I think there are two parts here. Frequently (at least back in 2011 :-)) it seems like we end up with "dumping ground" files with a namespace derived from the file name. A suffix of "_util.h" is usually a dead give away. What seems better is splitting the file up into several smaller files that are more scoped. This could mean extracting some classes into separate files and finding finer-grained groupings for functions that make sense. When this extraction occurs maybe we end up not needing another namespace.

The second part is the issue you raise. I have certainly written my fair share of classes that just have static methods. I don't recall there being a rule against this. I remember it being optional, but I don't think that matters. My take is that we should just conform with the style guide going forward so we do less debating :)

Here's a link to the relevant section of the style guide:

https://google.github.io/styleguide/cppguide.html#Nonmember,_Static_Member,_and_Global_Functions

-Darin

[Peter Kasting]

On Wed, Jun 29, 2016 at 5:45 PM, <dan...@chromium.org> wrote:

On Wed, Jun 29, 2016 at 5:40 PM, Darin Fisher <da...@chromium.org> wrote:

I think the advice to avoid dumping ground files (like file_util.h) is still relevant and is different than the issue you are raising?

Ya, I'm asking about the advice which says to not make a namespace for a single file, the other option is to leave them in a more general namespace (which makes things less clear), or make a class of only static methods (which the google styleguide explicitly calls out as a no).

Concrete examples:

https://cs.chromium.org/chromium/src/cc/base/math_util.h - A class of only-static methods to group them under a scope.

vs 

https://cs.chromium.org/chromium/src/content/common/gpu/client/command_buffer_metrics.h - A namespace for the file to group methods/enums under a scope.

I did the latter one recently after reading this part of the Google style guide, but I've seen reviewers asking people not to make a namespace and citing this thread more recently.

My recollection is that Brett was trying to advise people not to go around creating a namespace for every single file, or trying to ensure that every file's contents were in some kind of namespace, or similar excessive/silly uses of namespaces.  It was not an attempt to explicitly contradict this section of the style guide.

In cases where you would either create a statics-only class or a namespace in order to group a set of related functions, follow the Google style guide and use a namespace.

PK 

[Brett Wilson]

Basically what Peter said. We went through a time where people were getting a bit namespace happy and everybody started creating namespace file_util in file_util.h for file utilities named like ReadFileToString. The namespace here isn't helping scope or disambiguate anything, especially since there were multiple file headers using this namespace across the codebase.

I haven't seen this nearly so much now, I think people's expectations have been reset enough.

In general, I like to encourage new things to be done in a nice subdirectory somewhere with its own build file. If you think you need a namespace, you use the name of your directory. Beyond this, I have realized the wholeness of the universe and that beauty can only exist in the presence of ugliness and please don't ask me about namespace style. :)

Brett