Re: Proposed style guide modification: using declarations and nested namespaces
On Mon, Nov 12, 2012 at 8:37 PM, Jeff Walden jwalden+...@mit.edu wrote: We ended up removing the nested |using| above and making all SpiderMonkey headers qualify everything with mozilla::. We use few enough things from mozilla:: so far that we switched to |using mozilla::RangedPtr| and so on in .cpp files and didn't think twice. The patch itself was pretty big and annoying, but at least we won't have these issues in the future. Why is using mozilla::RangedPtr required; is RangedPtr ambiguous? Concerning the DOM nsDocument case, one possible solution might be to refer to those symbols with a still-qualified but not fully-qualified name. Type inference code uses types:: without much pain; Ion code uses ion:: (inside the js namespace) without much pain, either. These seem like fine solutions. For another data point, WebKit's WTF headers include |using WTF::Vector| and such at the bottom of them, and I have seen grumbling and/or regrets about this decision, as it's caused issues with name conflicts with some of Google's gtest framework. I think everyone agrees that all kinds of using declarations in headers are disallowed. Rob -- Jesus called them together and said, “You know that the rulers of the Gentiles lord it over them, and their high officials exercise authority over them. Not so with you. Instead, whoever wants to become great among you must be your servant, and whoever wants to be first must be your slave — just as the Son of Man did not come to be served, but to serve, and to give his life as a ransom for many.” [Matthew 20:25-28] ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
Robert O'Callahan wrote: On Mon, Nov 12, 2012 at 8:37 PM, Jeff Walden jwalden+...@mit.edu wrote: We ended up removing the nested |using| above and making all SpiderMonkey headers qualify everything with mozilla::. We use few enough things from mozilla:: so far that we switched to |using mozilla::RangedPtr| and so on in .cpp files and didn't think twice. The patch itself was pretty big and annoying, but at least we won't have these issues in the future. Why is using mozilla::RangedPtr required; is RangedPtr ambiguous? I would have thought it was undefined until you used one of ::mozilla or ::mozilla::RangedPtr to bring it into scope. -- Warning: May contain traces of nuts. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 2012-11-10 12:58 AM, Robert O'Callahan wrote: What exactly is the benefit here? As far as I know, using namespace A; using namespace B; where both A and B define Foo doesn't actually cause a compile error unless/until the code actually references Foo. The scenario I'm concerned with is when a .cpp file does 'using namespace A;' and then goes on to define a bunch of its *own* symbols; later someone adds a symbol to namespace A, and gets an unexpected break possibly miles from their own code. I see *avoiding* this phenomenon as the primary *benefit* of namespaces, and it's totally lost if you do 'using namespace' even in .cpp files. Sort of the same thing as adding DOM properties that then collide with website expandos. zw ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On Mon, Nov 12, 2012 at 9:37 AM, Zack Weinberg za...@panix.com wrote: The scenario I'm concerned with is when a .cpp file does 'using namespace A;' and then goes on to define a bunch of its *own* symbols; later someone adds a symbol to namespace A, and gets an unexpected break possibly miles from their own code. I see *avoiding* this phenomenon as the primary *benefit* of namespaces, and it's totally lost if you do 'using namespace' even in .cpp files. I see. Thanks for explaining that. I've never ever seen that happen, though. So I'm reluctant to pay an up-front cost to mitigate a problem whose size is unknown but could be smaller than any other problem we have. (And if it does occur in the future, the problem can still be quite easily fixed then.) Rob -- Jesus called them together and said, “You know that the rulers of the Gentiles lord it over them, and their high officials exercise authority over them. Not so with you. Instead, whoever wants to become great among you must be your servant, and whoever wants to be first must be your slave — just as the Son of Man did not come to be served, but to serve, and to give his life as a ransom for many.” [Matthew 20:25-28] ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 2012-11-12 1:44 PM, Robert O'Callahan wrote: On Mon, Nov 12, 2012 at 9:37 AM, Zack Weinberg za...@panix.com wrote: The scenario I'm concerned with is when a .cpp file does 'using namespace A;' and then goes on to define a bunch of its *own* symbols; later someone adds a symbol to namespace A, and gets an unexpected break possibly miles from their own code. I see *avoiding* this phenomenon as the primary *benefit* of namespaces, and it's totally lost if you do 'using namespace' even in .cpp files. I see. Thanks for explaining that. I've never ever seen that happen, though. So I'm reluctant to pay an up-front cost to mitigate a problem whose size is unknown but could be smaller than any other problem we have. (And if it does occur in the future, the problem can still be quite easily fixed then.) OK, fair enough. zw ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 11/12/2012 10:44 AM, Robert O'Callahan wrote:
On Mon, Nov 12, 2012 at 9:37 AM, Zack Weinberg za...@panix.com wrote:
The scenario I'm concerned with is when a .cpp file does 'using namespace
A;' and then goes on to define a bunch of its *own* symbols; later someone
adds a symbol to namespace A, and gets an unexpected break possibly miles
from their own code. I see *avoiding* this phenomenon as the primary
*benefit* of namespaces, and it's totally lost if you do 'using namespace'
even in .cpp files.
I've never ever seen that happen, though.
It's happening. We're trying to add a mozilla::Range struct to mfbt that would
lightly encapsulate a pointer and a length, providing assertions whenever you
index into it, extract start or end pointers, and so on. It would have been
simple to add, but SpiderMonkey has a js::ion::Range struct already. And as a
semi-convenience we had this in one place:
namespace js { using namespace mozilla; }
SpiderMonkey currently does |using namespace js;| all over the place, though,
so in Ion code there was suddenly an ambiguity between mozilla::Range and
js::ion::Range -- even for code written inside the ion namespace, I believe.
Ordinarily the innermost Range would win, but once you start opening
namespaces, that rule goes out the door.
We ended up removing the nested |using| above and making all SpiderMonkey
headers qualify everything with mozilla::. We use few enough things from
mozilla:: so far that we switched to |using mozilla::RangedPtr| and so on in
.cpp files and didn't think twice. The patch itself was pretty big and
annoying, but at least we won't have these issues in the future.
Concerning the DOM nsDocument case, one possible solution might be to refer to
those symbols with a still-qualified but not fully-qualified name. Type
inference code uses types:: without much pain; Ion code uses ion:: (inside the
js namespace) without much pain, either.
For another data point, WebKit's WTF headers include |using WTF::Vector| and
such at the bottom of them, and I have seen grumbling and/or regrets about this
decision, as it's caused issues with name conflicts with some of Google's gtest
framework.
Jeff
___
dev-platform mailing list
dev-platform@lists.mozilla.org
https://lists.mozilla.org/listinfo/dev-platform
Proposed style guide modification: using declarations and nested namespaces
I reviewed a patch today that had: using namespace mozilla; using namespace dom; The intent is to pull in the contents of both the mozilla and mozilla::dom namespaces, but this is only clear if you know that there is no top-level dom namespace. In the review comments I asked the author to write this with fully qualified namespaces. That is, as: using namespace mozilla; using namespace mozilla::dom; If nobody has objections I'd like to add this to the style guide. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 11/9/12 10:28 AM, Kyle Huey wrote: I reviewed a patch today that had: using namespace mozilla; using namespace dom; The intent is to pull in the contents of both the mozilla and mozilla::dom namespaces, but this is only clear if you know that there is no top-level dom namespace. In the review comments I asked the author to write this with fully qualified namespaces. That is, as: using namespace mozilla; using namespace mozilla::dom; If nobody has objections I'd like to add this to the style guide. Why stop there? What about: using namespace ::mozilla; using namespace ::mozilla::dom; ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On Fri, Nov 9, 2012 at 10:46 AM, Gregory Szorc g...@mozilla.com wrote: On 11/9/12 10:28 AM, Kyle Huey wrote: I reviewed a patch today that had: using namespace mozilla; using namespace dom; The intent is to pull in the contents of both the mozilla and mozilla::dom namespaces, but this is only clear if you know that there is no top-level dom namespace. In the review comments I asked the author to write this with fully qualified namespaces. That is, as: using namespace mozilla; using namespace mozilla::dom; If nobody has objections I'd like to add this to the style guide. Why stop there? What about: using namespace ::mozilla; using namespace ::mozilla::dom; In the past people have complained when I've written code that prefixes symbols with '::'. I suspect my proposal will be easier to reach consensus on. ;-) I don't have any objections to :: though. - Kyle ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 2012-11-09 1:28 PM, Kyle Huey wrote: I reviewed a patch today that had: using namespace mozilla; using namespace dom; The intent is to pull in the contents of both the mozilla and mozilla::dom namespaces, but this is only clear if you know that there is no top-level dom namespace. In the review comments I asked the author to write this with fully qualified namespaces. That is, as: using namespace mozilla; using namespace mozilla::dom; The style guidelines recommend against using nested namespaces, so doing what you suggest would make them self-inconsistent. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 11/9/12 11:53 AM, Ehsan Akhgari wrote: using namespace mozilla; using namespace mozilla::dom; The style guidelines recommend against using nested namespaces, so doing what you suggest would make them self-inconsistent. But we have some nested namespaces today, so `using` them like Kyle suggests lets people write code as if the nested namespace did not exist. That should make unnesting the namespaces easier in the future. :) chris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 2012-11-09 3:40 PM, Chris Peterson wrote: On 11/9/12 11:53 AM, Ehsan Akhgari wrote: using namespace mozilla; using namespace mozilla::dom; The style guidelines recommend against using nested namespaces, so doing what you suggest would make them self-inconsistent. But we have some nested namespaces today, so `using` them like Kyle suggests lets people write code as if the nested namespace did not exist. That should make unnesting the namespaces easier in the future. :) I agree with what Kyle suggests. But I think that instead of making the style guidelines more friendly to nested namespaces, we should focus on doing less of that in our code. Cheers, Ehsan ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 2012-11-09 1:28 PM, Kyle Huey wrote: I reviewed a patch today that had: using namespace mozilla; using namespace dom; The style guide should forbid `using namespace` altogether. Use only what you need. ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On Fri, Nov 9, 2012 at 10:14 PM, Zack Weinberg za...@panix.com wrote: The style guide should forbid `using namespace` altogether. Use only what you need. I really don't think it should. I do not want to see source files full of difficult-to-maintain and unnecessary using boilerplate a la Java import. At least with Java imports, there are really good tools for managing the mess; we don't have those tools for C++. Rob -- Jesus called them together and said, “You know that the rulers of the Gentiles lord it over them, and their high officials exercise authority over them. Not so with you. Instead, whoever wants to become great among you must be your servant, and whoever wants to be first must be your slave — just as the Son of Man did not come to be served, but to serve, and to give his life as a ransom for many.” [Matthew 20:25-28] ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 2012-11-09 10:49 PM, Robert O'Callahan wrote: On Fri, Nov 9, 2012 at 10:14 PM, Zack Weinberg za...@panix.com wrote: The style guide should forbid `using namespace` altogether. Use only what you need. I really don't think it should. I do not want to see source files full of difficult-to-maintain and unnecessary using boilerplate a la Java import. At least with Java imports, there are really good tools for managing the mess; we don't have those tools for C++. I challenge your presuppositions. The average file should need no more than one or two `using SYMBOL;` lines per header it includes. Maintaining this will not be significantly more difficult than maintaining the existing lists of header inclusions (which I admit is already too difficult). And I think that it is worth it in order not to lose one of the major benefits of namespaces, viz. that you don't have to worry about symbol conflicts with anything but what you actually use. zw ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
2012/11/9 Zack Weinberg za...@panix.com: On 2012-11-09 1:28 PM, Kyle Huey wrote: I reviewed a patch today that had: using namespace mozilla; using namespace dom; The style guide should forbid `using namespace` altogether. Use only what you need. In a given cpp file (in a single TU), as long as it builds, I don't see the problem with using namespace. Of course I see the theoretical problem, it's never been a real issue IME. If it ever causes problems in a given cpp file it's easy enough to stop doing using namespace there. Likewise (even more so) at function scope I don't see an issue with using namespace. The only issue I see is using namespace at file scope in a _header file_. I would support a coding style rule against that. I filed bug 798033 about removing the occurences that we have of that, and a new contributor is making great progress there. Benoit ___ 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: Proposed style guide modification: using declarations and nested namespaces
On 11/9/12 8:11 PM, Benoit Jacob wrote: The only issue I see is using namespace at file scope in a _header file_. I would support a coding style rule against that. https://developer.mozilla.org/en-US/docs/Developer_Guide/Coding_Style#Namespaces says: No using statements are allowed in header files, except inside class definitions or functions. So whoever is putting using in header files is already breaking the rules... -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
Re: Proposed style guide modification: using declarations and nested namespaces
On 11/9/12 8:03 PM, Zack Weinberg wrote: I challenge your presuppositions. The average file should need no more than one or two `using SYMBOL;` lines per header it includes. That depends on the structure of the code, no? For example, until we finish moving over all of the DOM to live inside the mozilla::dom namespace, something like nsDocument would need to have a using for every single interface type used in the Document interface in IDL... I assure you there are more than two of those. -Boris ___ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
