C++17 feature proposal: Nested namespaces

[Avi Drissman]

It's pure syntactic sugar, but nested namespaces are something that simplify things just a little bit. Any of the base platform teams, who are familiar with the base::mac style of platform scoping, will welcome this small bit of respite, as will those who forward-declare double-namespaced things.

The only argument I can think of against it is that for the transformation

namespace A {

namespace B {

to

namespace A::B {

directly grepping for the uses of namespace B will be more difficult. However, I think that the need to grep for namespaces in declarations and definitions (as opposed to usages) is exceptionally rare and this doesn't matter. 

Therefore, I nominate nested namespaces from C++17 to be allowed.

Avi

[Peter Kasting]

+1

PK

[Daniel Hosseinian]

I'm not necessarily against nested namespaces, but I have a couple of concerns:

1) I'm worried about the stylistic inconsistency that will arise in the codebase. Do we plan to migrate everything to the new style?

2) You mention the added difficulty of grepping. I would prioritize the needs of the code readers/searchers over those of writers.

On Fri, Dec 24, 2021, 8:33 AM 'Peter Kasting' via cxx <c...@chromium.org> wrote:

+1

PK

--
You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFC3QS_EJu5yxruvz-H4jCiEGOe2Jj_bTTF_pYfKNjcA-g%40mail.gmail.com.

[Avi Drissman]

1) I'm happy to do a bulk cleanup. It's pretty find-and-replace friendly.

2) I've worked on Chromium since 2008 and honestly I don't think I've ever had to search for a second-level nested namespace in the way I described. This would only be for definitions and declarations, not uses, and given that the second-level namespace is only ever used as a child namespace of the first, this would actually help people searching for, say, base::mac, as now definitions and declarations would look just like uses. My rephrasing would be that this is a theoretical downside but literally no one will be affected, and the new feature will help searchability.

Avi

[Daniel Hosseinian]

+1 if this comes with a bulk cleanup and documentation of preferred style. Although I would also like guidance of how to handle multiple nested namespaces:

namespace A {

namespace B { ... }

namespace C { ... }

}  // namespace A

Should we write it as the following?

namespace A::B { ... }

namespace A::C { ... }

[Avi Drissman]

Re your example, I'd think it'd be cleaner to leave it be. Nested namespaces are sugar to be used when it makes things more readable, not a requirement for use whenever possible. I would recommend its use in the case of

namespace A {

namespace B {

// ...

}  // namespace B

}  // namespace A

and right now I can't think of any other situations where it would lead to more readable code. However, even that set of cases is large enough to make it worth it imo.

As for a bulk cleanup, I'm looking at clang-tidy's modernize-concat-nested-namespaces which seems perfect, except for the fact that it doesn't rewrite the comment on the trailing brace. So, for example, if you were to run it on the above example, you'd get

namespace A::B {

// ...

}  // namespace A

which isn't what we want. Is there a clang-tidy plugin to enforce those comments at the end? I can and will do a bulk cleanup with a find/replace by hand if that's what it takes, but help from clang tooling would be nice.

Avi

[Daniel Cheng]

I think clang-format adds those comments.

That being said, I'm not sure of the utility of a bulk cleanup here. The style has never been fixed in stone and is expected to continue changing over time. I'd favor eventual consistency.

Daniel

2021年12月27日(月) 12:17 'Avi Drissman' via cxx <c...@chromium.org>:

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACWgwAaN_LQzYnVsBkJsS1nefHMu133oEfHzmEkCs%2BuXR2HM2Q%40mail.gmail.com.

[Avi Drissman]

clang-format sometimes is adding that trailing comment, and sometimes isn't. I'm not sure why.

In any case, my proposal CL is https://chromium-review.googlesource.com/c/chromium/src/+/3359337. Happy to wait until next week for people to get back to commit it, but if you have a comment for it, please let me know.

[K. Moon]

clang-format doesn't add the trailing comment if there's only one line in the namespace block. (I discovered that this was a special rule in the logic some time ago.)

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACWgwAaujAEg2_Vcwqj%3D%3DtT%3D6j%3DUUWK%2BuM2kwhgtSXor02kHbA%40mail.gmail.com.

[Avi Drissman]

These were large blocks. I know `git cl format` tries to not touch unchanged code, but I did formatting in VSCode. In any case I'm not super worried.

[Avi Drissman]

I think I have an answer; spinning out to a new thread and saving this thread for the nested namespace proposal.

[Peter Kasting]

On Mon, Dec 27, 2021 at 2:00 PM Daniel Cheng <dch...@chromium.org> wrote:

That being said, I'm not sure of the utility of a bulk cleanup here. The style has never been fixed in stone and is expected to continue changing over time. I'd favor eventual consistency.

+1 to not needing a bulk cleanup here, though I'm also fine with running the clang-tidy pass even if it leaves trailing comments in a slightly odd state.  Also +1 to "can't recall having ever searched for namespace usage in a way this would negatively impact".

Avi, 

[Jeremy Roman]

+1 to both of these

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CAAHOzFBHNcThtxPnrPLx7MSmXyton7wEQ6VG%3D0va25FwZ0J%3D0g%40mail.gmail.com.

[Avi Drissman]

Would adding modernize-concat-nested-namespaces to the .clang-tidy files as I do in https://crrev.com/c/3359337 be sufficient?

[Peter Kasting]

On Wed, Dec 29, 2021 at 11:54 AM Avi Drissman <a...@google.com> wrote:

Would adding modernize-concat-nested-namespaces to the .clang-tidy files as I do in https://crrev.com/c/3359337 be sufficient?

I believe that will cause Tricium to start suggesting these to people, but it won't automatically clean up things.

PK

[Avi Drissman]

I know; what I meant was, given that you're inclined to eventual consistency, would adding that line to the clang tidy files be sufficient for you?

[Peter Kasting]

I'm less inclined to add it than to do nothing; adding it is something I'd only do after running clang tidy to clean it up.

PK

[Avi Drissman]

I'm still happy to do a cleanup. Note that we have a lot of clang tidy options listed that we never did a cleanup on; I constantly get cleanup alerts on lots of files.

[dan...@chromium.org]

I think it's worth adding to the tricium. It's a lost cost fyi signal to help teach modern best practices and/or avoid bugs.

And +1 to allowing this and to not being worried about needing slightly better grep patterns.

--

You received this message because you are subscribed to the Google Groups "cxx" group.
To unsubscribe from this group and stop receiving emails from it, send an email to cxx+uns...@chromium.org.

To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/cxx/CACWgwAbLNo-pUG8oUQLkOcCqRmv1XK%2Bd2D3V-RciNsCxfKVnnA%40mail.gmail.com.

[Avi Drissman]

Now that everyone's back, PTAL at https://crrev.com/c/3359337

[Avi Drissman]

This seems to have general consensus, and I have approval on the CL, so if there are no objections by the end of day I'll land it.

Thank you!