Re: [C++] Intent to eliminate: `using namespace std; ` at global scope

[Eric Rescorla]

+1. This::sounds::like::a::great::change.

-Ekr


On Thu, Aug 29, 2019 at 12:13 PM Nathan Froyd  wrote:

> Hi all,
>
> In working on upgrading our C++ support to C++17 [1], we've run into
> some issues [2] surrounding the newly-introduced `std::byte` [3],
> various Microsoft headers that pull in definitions of `byte`, and
> conflicts between the two when one has done `using namespace std;`,
> particularly at global scope.  Any use of `using namespace $NAME` is
> not permitted by the style guide [4].
>
> A quick perusal of our code shows that we have, uh, "many" violations
> of the prohibition of `using namespace $NAME` [5].  I do not intend to
> boil the ocean and completely rewrite our codebase to eliminate all
> such violations.  However, since the use of `using namespace std;` is
> relatively less common (~100 files) and is blocking useful work,
> eliminating that pattern seems like a reasonable thing to do.
>
> Thus far, it appears that the problematic `using namespace std;`
> instances all appear at global scope.  We have a handful of
> function-scoped ones that do not appear to be causing problems; if
> those are easy to remove in passing, we'll go ahead and remove
> function-scoped ones as well.  The intent is to not apply this change
> to third-party code unless absolutely necessary; we have various ways
> of dealing with the aforementioned issues--if they even come up--in
> third-party code.
>
> The work is being tracked in [2].  Please do not add new instances of
> `using namespace std;` at global scope, or approve new instances in
> patches that you review; when this work is complete, we will ideally
> have a lint that checks for this sort of thing automatically.  If you
> would like to help with this project, please file blocking bugs
> against [2].
>
> Thanks,
> -Nathan
>
> [1] https://bugzilla.mozilla.org/show_bug.cgi?id=1560664
> [2] https://bugzilla.mozilla.org/show_bug.cgi?id=1577319
> [3] http://eel.is/c++draft/cstddef.syn#lib:byte
> [4] https://google.github.io/styleguide/cppguide.html#Namespaces
> [5]
> https://searchfox.org/mozilla-central/search?q=using+namespace+.%2B%3B&case=false®exp=true&path=
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

[C++] Intent to eliminate: `using namespace std;` at global scope

[Nathan Froyd]

Hi all,

In working on upgrading our C++ support to C++17 [1], we've run into
some issues [2] surrounding the newly-introduced `std::byte` [3],
various Microsoft headers that pull in definitions of `byte`, and
conflicts between the two when one has done `using namespace std;`,
particularly at global scope.  Any use of `using namespace $NAME` is
not permitted by the style guide [4].

A quick perusal of our code shows that we have, uh, "many" violations
of the prohibition of `using namespace $NAME` [5].  I do not intend to
boil the ocean and completely rewrite our codebase to eliminate all
such violations.  However, since the use of `using namespace std;` is
relatively less common (~100 files) and is blocking useful work,
eliminating that pattern seems like a reasonable thing to do.

Thus far, it appears that the problematic `using namespace std;`
instances all appear at global scope.  We have a handful of
function-scoped ones that do not appear to be causing problems; if
those are easy to remove in passing, we'll go ahead and remove
function-scoped ones as well.  The intent is to not apply this change
to third-party code unless absolutely necessary; we have various ways
of dealing with the aforementioned issues--if they even come up--in
third-party code.

The work is being tracked in [2].  Please do not add new instances of
`using namespace std;` at global scope, or approve new instances in
patches that you review; when this work is complete, we will ideally
have a lint that checks for this sort of thing automatically.  If you
would like to help with this project, please file blocking bugs
against [2].

Thanks,
-Nathan

[1] https://bugzilla.mozilla.org/show_bug.cgi?id=1560664
[2] https://bugzilla.mozilla.org/show_bug.cgi?id=1577319
[3] http://eel.is/c++draft/cstddef.syn#lib:byte
[4] https://google.github.io/styleguide/cppguide.html#Namespaces
[5] 
https://searchfox.org/mozilla-central/search?q=using+namespace+.%2B%3B&case=false®exp=true&path=
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Ehsan Akhgari]

On 2014-05-21, 7:40 AM, Nicolas Silva wrote:

Sorry, my example was not clear enough. The issue with using namespace +
unifoed builds is that the using namespace declaration applies to all
(or most) of the headers included in the unified translation unit. So
using namespace mozilla at the top of each .cpp is harmless until we
include third party libraries, which we do.

This build failure is a conflict between our gfx:: classes and some
defined by 3rd party libraries we use on Mac.

This is really not a style problem. It breaks builds.


Yes, I understand this problem.  It's basically why our style guide says 
"using namespace ...; is only allowed in .cpp files after all 
#includes.", it tries to protect you from this problem.  The only thing 
that changes in the context of unified builds, is that adding a using 
namespace statement after the #includes in one .cpp file may be putting 
it before the #includes in the next file.


For the concrete case at hand, I think the simplest fix is to put the 
code inside those .cpp files in the mozilla::gfx namespace if they 
belong there, and avoid |using namespace mozilla::gfx;| elsewhere.


___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Nicolas Silva]

Sorry, my example was not clear enough. The issue with using namespace +
unifoed builds is that the using namespace declaration applies to all (or
most) of the headers included in the unified translation unit. So using
namespace mozilla at the top of each .cpp is harmless until we include
third party libraries, which we do.

This build failure is a conflict between our gfx:: classes and some defined
by 3rd party libraries we use on Mac.

This is really not a style problem. It breaks builds.

Cheers,

Nical




On Wed, May 21, 2014 at 4:36 AM, Ehsan Akhgari wrote:

> On 2014-05-20, 10:00 PM, Joshua Cranmer 🐧 wrote:
>
>> On 5/20/2014 8:37 PM, Ehsan Akhgari wrote:
>>
>>> FWIW, I argued against nested namespaces a few years ago (couldn't
>>> find a link to it through Google unfortunately) and people let me
>>> "win" that battle by allowing me to edit the coding style to prohibit
>>> nested namespoaces in most cases
>>> <https://developer.mozilla.org/en-US/docs/Mozilla/
>>> Developer_guide/Coding_Style#Namespaces>,
>>> but unfortunately we never adhered to this rule in practice, and these
>>> days three level nested namespaces are pretty common in the code base.
>>> We're just bending C++ in a way that it's not quite comfortable with
>>> here.
>>>
>>
>> How about adding the rule "all new namespaces must be approved by
>> ", just like all new
>> top-level directories need to be explicitly approved? If we could get
>> reviewers to enforce that, it would hopefully cut down on people using
>> outlandishly long namespaces.
>>
>
> I don't think that will work.  IIRC I approached some module owners about
> this back in the day and they explicitly told me that they don't care and
> asked me to fix the rest of the places where we use nested namespaces, and
> I ended up giving up on the whole idea.
>
>
>  I think there are valid reasons to have a two-level namespace (e.g.,
>> mozilla::mailnews [1]), but I find it deathly hard to justify going any
>> deeper.
>>
>> [1] Actually, I would probably prefer mozilla::comm had
>> mozilla::mailnews not already had precedent when I started using C++
>> namespaces. :-)
>>
>
> See, this is the problem!  Everyone thinks that it's reasonable to add
> their own nested namespaces, and they're right as long as they don't look
> at the big picture.  As long as we allow any code to #include any other
> code, this is a slippery slope.
>
> Honestly, speaking as the person who did the majority of the work to give
> us unified builds, and presumably dealt with the most of this issue while
> doing so, this is not a very important problem, and it's uncommon enough
> that I don't think it warrants an effort to get a handle on our namespace
> usage or add stricter review requirements.
>
> Cheers,
> Ehsan
>
>
> ___
> dev-platform mailing list
> dev-platform@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform
>
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Ehsan Akhgari]

On 2014-05-20, 10:00 PM, Joshua Cranmer 🐧 wrote:

On 5/20/2014 8:37 PM, Ehsan Akhgari wrote:

FWIW, I argued against nested namespaces a few years ago (couldn't
find a link to it through Google unfortunately) and people let me
"win" that battle by allowing me to edit the coding style to prohibit
nested namespoaces in most cases
,
but unfortunately we never adhered to this rule in practice, and these
days three level nested namespaces are pretty common in the code base.
We're just bending C++ in a way that it's not quite comfortable with
here.


How about adding the rule "all new namespaces must be approved by
", just like all new
top-level directories need to be explicitly approved? If we could get
reviewers to enforce that, it would hopefully cut down on people using
outlandishly long namespaces.


I don't think that will work.  IIRC I approached some module owners 
about this back in the day and they explicitly told me that they don't 
care and asked me to fix the rest of the places where we use nested 
namespaces, and I ended up giving up on the whole idea.



I think there are valid reasons to have a two-level namespace (e.g.,
mozilla::mailnews [1]), but I find it deathly hard to justify going any
deeper.

[1] Actually, I would probably prefer mozilla::comm had
mozilla::mailnews not already had precedent when I started using C++
namespaces. :-)


See, this is the problem!  Everyone thinks that it's reasonable to add 
their own nested namespaces, and they're right as long as they don't 
look at the big picture.  As long as we allow any code to #include any 
other code, this is a slippery slope.


Honestly, speaking as the person who did the majority of the work to 
give us unified builds, and presumably dealt with the most of this issue 
while doing so, this is not a very important problem, and it's uncommon 
enough that I don't think it warrants an effort to get a handle on our 
namespace usage or add stricter review requirements.


Cheers,
Ehsan

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Joshua Cranmer 🐧]

On 5/20/2014 8:37 PM, Ehsan Akhgari wrote:
FWIW, I argued against nested namespaces a few years ago (couldn't 
find a link to it through Google unfortunately) and people let me 
"win" that battle by allowing me to edit the coding style to prohibit 
nested namespoaces in most cases 
, 
but unfortunately we never adhered to this rule in practice, and these 
days three level nested namespaces are pretty common in the code base. 
We're just bending C++ in a way that it's not quite comfortable with 
here.


How about adding the rule "all new namespaces must be approved by 
", just like all new 
top-level directories need to be explicitly approved? If we could get 
reviewers to enforce that, it would hopefully cut down on people using 
outlandishly long namespaces.


I think there are valid reasons to have a two-level namespace (e.g., 
mozilla::mailnews [1]), but I find it deathly hard to justify going any 
deeper.


[1] Actually, I would probably prefer mozilla::comm had 
mozilla::mailnews not already had precedent when I started using C++ 
namespaces. :-)


--
Joshua Cranmer
Thunderbird and DXR developer
Source code archæologist

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Robert O'Callahan]

On Wed, May 21, 2014 at 1:11 PM, L. David Baron  wrote:

> I wonder if the problem is that we're overusing namespaces (i.e., we
> have too many of them or put many classes at places too deeply
> nested in the namespace hierarchy)?  Perhaps it makes sense to have
> a guideline that names shouldn't feel overly verbose with only a
> "using namespace mozilla"?
>

As Ehsan says, we already have that guideline.

Rob
-- 
Jtehsauts  tshaei dS,o n" Wohfy  Mdaon  yhoaus  eanuttehrotraiitny  eovni
le atrhtohu gthot sf oirng iyvoeu rs ihnesa.r"t sS?o  Whhei csha iids  teoa
stiheer :p atroa lsyazye,d  'mYaonu,r  "sGients  uapr,e  tfaokreg iyvoeunr,
'm aotr  atnod  sgaoy ,h o'mGee.t"  uTph eann dt hwea lmka'n?  gBoutt  uIp
waanndt  wyeonut  thoo mken.o w
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Ehsan Akhgari]

On 2014-05-20, 9:11 PM, L. David Baron wrote:

On Wednesday 2014-05-21 11:51 +1200, Robert O'Callahan wrote:

On Wed, May 21, 2014 at 4:36 AM, Nicolas Silva wrote:


Honestly, I don't mean to start a bikeshed about whether we should enforce
strict rules about this project-wise, because I know that people have
different tastes as to whether writing "Size" is vastly better than
"gfx::Size". But I'd like to encourage people to carefully make use of this
c++ feature, especially when the namespace in question is used a handful of
times in the file. I personally will be strict about it the reviews I give.



I don't think we should have rules that some reviewers enforce and others
don't. That makes life hard for everyone.

It's not clear to me how moving the "using namespace" declaration inside a
namespace helps. Won't most files in a unified build translation unit put
their declarations in the same namespace, negating any benefit?

Personally I find unified-build-related using-namespace errors are rare ---
I've not encountered one yet. I'm not sure they're worth attempting to
ameliorate.


I wonder if the problem is that we're overusing namespaces (i.e., we
have too many of them or put many classes at places too deeply
nested in the namespace hierarchy)?  Perhaps it makes sense to have
a guideline that names shouldn't feel overly verbose with only a
"using namespace mozilla"?  For example, that would suggest that
layers::Layer feeling redundant is a sign that Layer should instead
be directly in the mozilla namespace.

I think following such a guideline (and avoiding using declarations
of the inner namespaces) might help avoiding the various collision
problems we've hit.


The problem is that we're using nested namespaces with same names under 
them.  C++ doesn't really provide a good mechanism to fix this problem, 
which means we have to create arbitrary restrictions on things such as 
where using namespaces go.


FWIW, I argued against nested namespaces a few years ago (couldn't find 
a link to it through Google unfortunately) and people let me "win" that 
battle by allowing me to edit the coding style to prohibit nested 
namespoaces in most cases 
<https://developer.mozilla.org/en-US/docs/Mozilla/Developer_guide/Coding_Style#Namespaces>, 
but unfortunately we never adhered to this rule in practice, and these 
days three level nested namespaces are pretty common in the code base. 
We're just bending C++ in a way that it's not quite comfortable with here.


Cheers,
Ehsan
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[L. David Baron]

On Wednesday 2014-05-21 11:51 +1200, Robert O'Callahan wrote:
> On Wed, May 21, 2014 at 4:36 AM, Nicolas Silva wrote:
> 
> > Honestly, I don't mean to start a bikeshed about whether we should enforce
> > strict rules about this project-wise, because I know that people have
> > different tastes as to whether writing "Size" is vastly better than
> > "gfx::Size". But I'd like to encourage people to carefully make use of this
> > c++ feature, especially when the namespace in question is used a handful of
> > times in the file. I personally will be strict about it the reviews I give.
> >
> 
> I don't think we should have rules that some reviewers enforce and others
> don't. That makes life hard for everyone.
> 
> It's not clear to me how moving the "using namespace" declaration inside a
> namespace helps. Won't most files in a unified build translation unit put
> their declarations in the same namespace, negating any benefit?
> 
> Personally I find unified-build-related using-namespace errors are rare ---
> I've not encountered one yet. I'm not sure they're worth attempting to
> ameliorate.

I wonder if the problem is that we're overusing namespaces (i.e., we
have too many of them or put many classes at places too deeply
nested in the namespace hierarchy)?  Perhaps it makes sense to have
a guideline that names shouldn't feel overly verbose with only a
"using namespace mozilla"?  For example, that would suggest that
layers::Layer feeling redundant is a sign that Layer should instead
be directly in the mozilla namespace.

I think following such a guideline (and avoiding using declarations
of the inner namespaces) might help avoiding the various collision
problems we've hit.

-David

-- 
𝄞   L. David Baron http://dbaron.org/   𝄂
𝄢   Mozilla  https://www.mozilla.org/   𝄂
 Before I built a wall I'd ask to know
 What I was walling in or walling out,
 And to whom I was like to give offense.
   - Robert Frost, Mending Wall (1914)


signature.asc
Description: Digital signature
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Brian Birtles]

(2014/05/21 8:51), Robert O'Callahan wrote:

Personally I find unified-build-related using-namespace errors are rare ---
I've not encountered one yet. I'm not sure they're worth attempting to
ameliorate.


I wasted half a day recently due to a "using namespace mozilla::layers" 
in an unrelated file (/layout/style/AnimationCommon.cpp) causing 
clashes. (I replaced it with "using mozilla::layers::Layer" which I 
guess still has problems.)


Brian
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Robert O'Callahan]

On Wed, May 21, 2014 at 4:36 AM, Nicolas Silva wrote:

> Honestly, I don't mean to start a bikeshed about whether we should enforce
> strict rules about this project-wise, because I know that people have
> different tastes as to whether writing "Size" is vastly better than
> "gfx::Size". But I'd like to encourage people to carefully make use of this
> c++ feature, especially when the namespace in question is used a handful of
> times in the file. I personally will be strict about it the reviews I give.
>

I don't think we should have rules that some reviewers enforce and others
don't. That makes life hard for everyone.

It's not clear to me how moving the "using namespace" declaration inside a
namespace helps. Won't most files in a unified build translation unit put
their declarations in the same namespace, negating any benefit?

Personally I find unified-build-related using-namespace errors are rare ---
I've not encountered one yet. I'm not sure they're worth attempting to
ameliorate.

Rob
-- 
Jtehsauts  tshaei dS,o n" Wohfy  Mdaon  yhoaus  eanuttehrotraiitny  eovni
le atrhtohu gthot sf oirng iyvoeu rs ihnesa.r"t sS?o  Whhei csha iids  teoa
stiheer :p atroa lsyazye,d  'mYaonu,r  "sGients  uapr,e  tfaokreg iyvoeunr,
'm aotr  atnod  sgaoy ,h o'mGee.t"  uTph eann dt hwea lmka'n?  gBoutt  uIp
waanndt  wyeonut  thoo mken.o w
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Benjamin Smedberg]

On 5/20/2014 12:36 PM, Nicolas Silva wrote:


So, I strongly encourage everyone to stop using "using namespace".


I think I disagree about the "mozilla" namespace specifically. Since 
basically all of gecko does or will end up being in the mozilla 
namespace, I'd like to counter-propose that we should write `using 
namespace mozilla;` at the head of all gecko .cpp files in the tree.


Otherwise, I think you're right about other namespaces. I do wonder 
though, if you would accept/encourage these at the toplevel:


using mozilla::dom::ipc::Blob; // or whatever

That pattern still pollutes the global namespace in a way that 
potentially affects unified builds, but in general it seems like the C++ 
pattern we want.


--BDS

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Boris Zbarsky]

On 5/20/14, 2:23 PM, Bobby Holley wrote:

In XPConnect, we have "using namespace JS" everywhere. This is pretty
important, because the JS rooting/handle API is very cumbersome to use
otherwise.


Note that if every file in a directory has the same "using namespace" 
declarations, then there is no problem with unified builds: the leaking 
"using namespace" bits only leak across to code that was using that 
namespace anyway.


So I don't think this is an issue for XPConnect.  Just like I don't 
think it's an issue to have "using namespace mozilla::dom" in DOM code.


-Boris

___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Jonas Sicking]

On Tue, May 20, 2014 at 9:36 AM, Nicolas Silva  wrote:
> Now that we have unified builds, writing "using namespace" in the global
> scope of a .cpp file is almost as bad as writing it in a header. Regularly
> build errors show up like this one:
> https://tbpl.mozilla.org/php/getParsedLog.php?id=40010766&tree=Try

I thought the plan was to only enable unified builds in directories
that didn't see a lot of active development. Sounds like the solution
is to disable unified builds in these directories while you are doing
very active development there.

/ Jonas
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

Re: using namespace

[Bobby Holley]

In XPConnect, we have "using namespace JS" everywhere. This is pretty
important, because the JS rooting/handle API is very cumbersome to use
otherwise.
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform

using namespace

[Nicolas Silva]

Now that we have unified builds, writing "using namespace" in the global
scope of a .cpp file is almost as bad as writing it in a header. Regularly
build errors show up like this one:
https://tbpl.mozilla.org/php/getParsedLog.php?id=40010766&tree=Try

What's awesome about these errors is that they typically show up on some
platform but not the others and they come and go as we add files or do
things that can alter the generation of the unified .cpp files.

So, I strongly encourage everyone to stop using "using namespace".
If you really really want to use them, please put them inside the scope of
a namespace. For instance a .cpp file in the gfx/layers directory typically
looks like this:

// using namespace gfx; here means nical: r-
namespace mozilla {
namespace layers {

// using namespace gfx; here means nical is grumpy but he'll probably not
block your super important path for this.

// ...

}
}

Beware that once using namespace is placed in an enclosing namespace foo,
it will affect the namespace foo the next time you open it:

namespace foo {
using namespace unicorns;
// ...
}

namespace foo {
// here it's just like you wrote using namespace unicorns; again
}

Which leads to problems with namespaces like mozilla::ipc and
mozilla::dom::ipc being ambiguous in the mozilla namespace. it is probably
just not possible to write using namespace mozilla::dom; in most of the
layers code for this reason.

Honestly, I don't mean to start a bikeshed about whether we should enforce
strict rules about this project-wise, because I know that people have
different tastes as to whether writing "Size" is vastly better than
"gfx::Size". But I'd like to encourage people to carefully make use of this
c++ feature, especially when the namespace in question is used a handful of
times in the file. I personally will be strict about it the reviews I give.

Cheers,

Nical
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform