[style] Sorting class forward decls

[Ben Goodger (Google)]

Since we're having a few discussions about style, how about this one:

I've been moving some code into a namespace, and have come across a diversity of styles for forward decls. Here's the one I think I like the most so far:

class A;

namespace N1 {

class B;

class C;

}

namespace N2 {

class D;

class E;

}

All classes in global namespace go first, alphabetically sorted, followed by namespaces, alphabetically sorted by namespace, with NLs between each section, and classes alphabetically sorted inside. Also the trailing // namespace N1 is omitted because it seems redundant for such a small enclosing block.

The only situation that causes turmoil here is when you have:

#ifndef C_H_

...

namespace N1 {

class A;

class B;

}

namespace N0 {

class D;

class C {

};

}  // namespace N0

...

Note that N0 is sorted after N1, and D before C, but that's because N0 is the "main" namespace in the file. This doesn't bother me though.

Thoughts?

-Ben

[Mark Mentovai]

Ben Goodger wrote:
> Since we're having a few discussions about style, how about this one:
> I've been moving some code into a namespace, and have come across a
> diversity of styles for forward decls. Here's the one I think I like the
> most so far:

[snip]

> All classes in global namespace go first, alphabetically sorted, followed by
> namespaces, alphabetically sorted by namespace, with NLs between each
> section, and classes alphabetically sorted inside.

I like all of this.

> Also the trailing //
> namespace N1 is omitted because it seems redundant for such a small
> enclosing block.

I’d probably leave it in anyway. I’d certainly leave it in if there
were multiple levels of namespacing.

> The only situation that causes turmoil here is when you have:

[snip]

> Note that N0 is sorted after N1, and D before C, but that's because N0 is
> the "main" namespace in the file. This doesn't bother me though.
> Thoughts?

This doesn’t bother me either. In fact, you presumably included
forward declarations in N1 because you needed to use them in this
file, and it’s likely that you needed them somewhere in N0, so they
necessarily must precede N0. For example, you might have had a
constructor N0::C::C(N1::A* a).

When I’m working with forward declarations, I always try to put all of
them at the top of the file, immediately following the header
#includes. Whether or not namespaces are in play doesn’t seem
relevant. Forward declarations first, then “meat.” If all of your
meat’s in N0, so be it.

With that in mind, I might revise your example slightly:

namespace N0 {
class D;
} // namespace N0

namespace N1 {
class A;
class B;

} // namespace N1

namespace N0 {

class C {
// ...
private:
N1::A* a_;
N1::B* b_;
D* d_;
};

} // namespace N0

Nothing wrong with opening, then closing, and then reopening the same
namespace multiple times, even with other stuff in between.

[Robert Sesek]

On Thu, Jan 6, 2011 at 11:13 PM, Ben Goodger (Google) <b...@chromium.org> wrote:

Thoughts?

What about forward decls of platform-specific classes that need to be #ifdef'ed?

class N;

#if defined(OS_MACOSX)

class NSWindow;

#endif

class O;

or

class N;

class O;

#if defined(OS_MACOSX)

class NSWindow;

#endif

Or some other permutation? I think the latter is easier to follow and more common. In which case, should we specify ordering rules there, too?

rsesek / @chromium.org

[Mark Mentovai]

Robert Sesek wrote:
> What about forward decls of platform-specific classes that need to be
> #ifdef'ed?

We just went through this with header #includes too in the past month.
We agreed on “put all of the platform #includes after the non-platform
ones.” I think we can do the same for forward declarations.

[Elliot Glaysher (Chromium)]

+1

I'm not sure I can automatically enforce the order in a clang plugin though.

-- Elliot

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

Agreed (I've already been doing that).

And agreed to everything Ben and Mark previously said.

PK

[Ben Goodger (Google)]

On Thu, Jan 6, 2011 at 8:49 PM, Mark Mentovai <ma...@chromium.org> wrote:

> Also the trailing //

> namespace N1 is omitted because it seems redundant for such a small
> enclosing block.

I’d probably leave it in anyway. I’d certainly leave it in if there
were multiple levels of namespacing.

OK. Perhaps this should be a soft recommendation though, since most of our forward decls seem to omit the trailing // namespace Foo

-Ben

[Mark Mentovai]

That’s fine with me.

[Lei Zhang]

Can we update the style guide [1] to reflect this decision? There's no
point in commenting the namespace's end brace when the content of the
namespace is only 1 or 2 lines long.

[1] https://sites.google.com/a/chromium.org/dev/developers/coding-style

[Ben Goodger (Google)]

Feel free to :-)

-Ben