Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Oct 19, 2011, at 9:56 PM, Ryosuke Niwa wrote: On Wed, Oct 19, 2011 at 6:32 PM, Adam Barth aba...@webkit.org wrote: On Wed, Oct 19, 2011 at 6:24 PM, Ryosuke Niwa rn...@webkit.org wrote: Also... VC++'s debugger happens to have some issues around unnamed namespaces: http://msdn.microsoft.com/en-us/library/0888kc6a%28VS.80%29.aspx Don't recall if this affected classes inside unnamed namespace though. I think VC++ 2005's support for unnamed namespace is better than nested classes (cl.exe has some serious bugs in resolving nested class names) so if our only alternative to unnamed namespace is nested classes, then I'd much prefer unnamed namespace. Adam, do you recall any compiler/debugger issues with classes inside unnamed namespace on VC++ 2005? Nope, but I haven't used VC 2005 in many years. Oops, sorry I meant to cc A. Roben. Adam (R), do you remember any issues with unnamed namespace? I don't, but that's mostly because we don't use them much so I don't have much experience with them! -Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
[webkit-dev] Style guide should mention that we don't use anonymous namespace
Hi, This came up again in one of my reviews :( I hate to add a bunch of new rules to style guide but apparently there are 86 instances of anonymous namespaces in WebKit already so it's probably a good idea to mention that we don't normally do this. Best, Ryosuke Niwa Software Engineer Google Inc. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
I'm not an advocate for or against, but it would be nice if we're going to disallow this, that there be a rationale. regards, --Tom Hi, This came up again in one of my reviews :( I hate to add a bunch of new rules to style guide but apparently there are 86 instances of anonymous namespaces in WebKit already so it's probably a good idea to mention that we don't normally do this. Best, Ryosuke Niwa Software Engineer Google Inc. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
The guideline is not to disallow anonymous namespaces. It’s to prefer “static” over anonymous namespaces when either one would work. Debugging tools on at least some of the platforms work better with functions that are given internal linkage by using the “static” keyword rather than functions that are inside anonymous namespaces. On the other hand, anonymous namespaces are a more powerful tool that can do more than the “static” keyword can. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? - Ryosuke On Wed, Oct 19, 2011 at 3:45 PM, Darin Adler da...@apple.com wrote: The guideline is not to disallow anonymous namespaces. It’s to prefer “static” over anonymous namespaces when either one would work. Debugging tools on at least some of the platforms work better with functions that are given internal linkage by using the “static” keyword rather than functions that are inside anonymous namespaces. On the other hand, anonymous namespaces are a more powerful tool that can do more than the “static” keyword can. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
I'm interested in learning the existing convention, not reasons to prefer one or another. - Ryosuke On Wed, Oct 19, 2011 at 3:50 PM, John Knottenbelt jknot...@chromium.orgwrote: I would recommend wrapping such classes in an anonymous namespace to avoid surprising link errors due to unintentional name collision. Such problems can also be difficult to spot at first as sometimes the linker just works and then you get a seg fault sometime later. On Wed, Oct 19, 2011 at 3:47 PM, Ryosuke Niwa rn...@webkit.org wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? - Ryosuke On Wed, Oct 19, 2011 at 3:45 PM, Darin Adler da...@apple.com wrote: The guideline is not to disallow anonymous namespaces. It’s to prefer “static” over anonymous namespaces when either one would work. Debugging tools on at least some of the platforms work better with functions that are given internal linkage by using the “static” keyword rather than functions that are inside anonymous namespaces. On the other hand, anonymous namespaces are a more powerful tool that can do more than the “static” keyword can. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Oct 19, 2011, at 3:47 PM, Ryosuke Niwa wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? I’d suggest not wrapping them in an anonymous namespace. Debugging tools work better when classes have unique names. Can anyone name a single time in the entire history of the WebKit project where we had a problem with class name collision? -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
I’m open to changing our style. The compiler many of us used in the past, gcc, had problems generating correct debug information for code in anonymous namespaces. Given that most uses didn’t accomplish anything more than putting static in front of functions did, we decided to stick with that. But that old gcc problem is not an active problem for most WebKit developers, since we’re almost all using either clang or Visual C++. As far as classes are concerned, I am not sure what kinds of difficulties qw might have debugging if there was no simple way to type a class’s name unambiguously. I’m sure that at least some things that are easier if each class has a unique name. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Wed, Oct 19, 2011 at 3:51 PM, Darin Adler da...@apple.com wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? I’d suggest not wrapping them in an anonymous namespace. Debugging tools work better when classes have unique names. Has this been a convention we use? - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Oct 19, 2011, at 3:58 PM, Ryosuke Niwa wrote: On Wed, Oct 19, 2011 at 3:51 PM, Darin Adler da...@apple.com wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? I’d suggest not wrapping them in an anonymous namespace. Debugging tools work better when classes have unique names. Has this been a convention we use? I’m not sure how to answer that. I have not used anonymous namespaces in any WebKit code I contributed, and the first case of doing so that I recall was when Adam Barth used some in the HTML parser work. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Wed, Oct 19, 2011 at 4:00 PM, Darin Adler da...@apple.com wrote: On Oct 19, 2011, at 3:58 PM, Ryosuke Niwa wrote: On Wed, Oct 19, 2011 at 3:51 PM, Darin Adler da...@apple.com wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? I’d suggest not wrapping them in an anonymous namespace. Debugging tools work better when classes have unique names. Has this been a convention we use? I’m not sure how to answer that. I have not used anonymous namespaces in any WebKit code I contributed, and the first case of doing so that I recall was when Adam Barth used some in the HTML parser work. Okay. Sounds like it's left to reviewers' and committers' discretions. It'll be still nice to give some guidance on when we should and should not use anonymous namespaces so that I don't have to engage in a debate on every code review. I personally don't like anonymous namespaces because VS.net's class view separates classes within anonymous namespace from the rest: http://goo.gl/2IkzQ (screen shot) But I think we've established that we prefer having unique class/function names over using anonymous namespaces. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Wed, Oct 19, 2011 at 4:29 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Oct 19, 2011 at 4:00 PM, Darin Adler da...@apple.com wrote: On Oct 19, 2011, at 3:58 PM, Ryosuke Niwa wrote: On Wed, Oct 19, 2011 at 3:51 PM, Darin Adler da...@apple.com wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? I’d suggest not wrapping them in an anonymous namespace. Debugging tools work better when classes have unique names. Has this been a convention we use? I’m not sure how to answer that. I have not used anonymous namespaces in any WebKit code I contributed, and the first case of doing so that I recall was when Adam Barth used some in the HTML parser work. Okay. Sounds like it's left to reviewers' and committers' discretions. It'll be still nice to give some guidance on when we should and should not use anonymous namespaces so that I don't have to engage in a debate on every code review. I personally don't like anonymous namespaces because VS.net's class view separates classes within anonymous namespace from the rest: http://goo.gl/2IkzQ (screen shot) But I think we've established that we prefer having unique class/function names over using anonymous namespaces. Note that unless we're running some extra analysis over the codebase beyond the standard compilation, uniqueness won't be enforced. The compiler isn't required to warn if the same name is used in different translation units. While this might be only a theoretical danger (Darin asked above for cases where we've actually had name collisions, and I haven't been around long enough to know the answer), the symptoms of such a collision can be quite subtle. - Adam ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
Another benefit to using the static keyword is that you can tell at a glance that the function is internal to the translation unit, while you may not notice the anonymous namespace. -Sam On Oct 19, 2011, at 3:45 PM, Darin Adler wrote: The guideline is not to disallow anonymous namespaces. It’s to prefer “static” over anonymous namespaces when either one would work. Debugging tools on at least some of the platforms work better with functions that are given internal linkage by using the “static” keyword rather than functions that are inside anonymous namespaces. On the other hand, anonymous namespaces are a more powerful tool that can do more than the “static” keyword can. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
Also... VC++'s debugger happens to have some issues around unnamed namespaces: http://msdn.microsoft.com/en-us/library/0888kc6a%28VS.80%29.aspx Don't recall if this affected classes inside unnamed namespace though. I think VC++ 2005's support for unnamed namespace is better than nested classes (cl.exe has some serious bugs in resolving nested class names) so if our only alternative to unnamed namespace is nested classes, then I'd much prefer unnamed namespace. Adam, do you recall any compiler/debugger issues with classes inside unnamed namespace on VC++ 2005? - Ryosuke On Wed, Oct 19, 2011 at 4:29 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Oct 19, 2011 at 4:00 PM, Darin Adler da...@apple.com wrote: On Oct 19, 2011, at 3:58 PM, Ryosuke Niwa wrote: On Wed, Oct 19, 2011 at 3:51 PM, Darin Adler da...@apple.com wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? I’d suggest not wrapping them in an anonymous namespace. Debugging tools work better when classes have unique names. Has this been a convention we use? I’m not sure how to answer that. I have not used anonymous namespaces in any WebKit code I contributed, and the first case of doing so that I recall was when Adam Barth used some in the HTML parser work. Okay. Sounds like it's left to reviewers' and committers' discretions. It'll be still nice to give some guidance on when we should and should not use anonymous namespaces so that I don't have to engage in a debate on every code review. I personally don't like anonymous namespaces because VS.net's class view separates classes within anonymous namespace from the rest: http://goo.gl/2IkzQ (screen shot) But I think we've established that we prefer having unique class/function names over using anonymous namespaces. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Wed, Oct 19, 2011 at 6:24 PM, Ryosuke Niwa rn...@webkit.org wrote: Also... VC++'s debugger happens to have some issues around unnamed namespaces: http://msdn.microsoft.com/en-us/library/0888kc6a%28VS.80%29.aspx Don't recall if this affected classes inside unnamed namespace though. I think VC++ 2005's support for unnamed namespace is better than nested classes (cl.exe has some serious bugs in resolving nested class names) so if our only alternative to unnamed namespace is nested classes, then I'd much prefer unnamed namespace. Adam, do you recall any compiler/debugger issues with classes inside unnamed namespace on VC++ 2005? Nope, but I haven't used VC 2005 in many years. Adam On Wed, Oct 19, 2011 at 4:29 PM, Ryosuke Niwa rn...@webkit.org wrote: On Wed, Oct 19, 2011 at 4:00 PM, Darin Adler da...@apple.com wrote: On Oct 19, 2011, at 3:58 PM, Ryosuke Niwa wrote: On Wed, Oct 19, 2011 at 3:51 PM, Darin Adler da...@apple.com wrote: How about classes that are only used in one cpp file? Should we be wrapping those in an anonymous namespace? I’d suggest not wrapping them in an anonymous namespace. Debugging tools work better when classes have unique names. Has this been a convention we use? I’m not sure how to answer that. I have not used anonymous namespaces in any WebKit code I contributed, and the first case of doing so that I recall was when Adam Barth used some in the HTML parser work. Okay. Sounds like it's left to reviewers' and committers' discretions. It'll be still nice to give some guidance on when we should and should not use anonymous namespaces so that I don't have to engage in a debate on every code review. I personally don't like anonymous namespaces because VS.net's class view separates classes within anonymous namespace from the rest: http://goo.gl/2IkzQ (screen shot) But I think we've established that we prefer having unique class/function names over using anonymous namespaces. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Style guide should mention that we don't use anonymous namespace
On Wed, Oct 19, 2011 at 6:32 PM, Adam Barth aba...@webkit.org wrote: On Wed, Oct 19, 2011 at 6:24 PM, Ryosuke Niwa rn...@webkit.org wrote: Also... VC++'s debugger happens to have some issues around unnamed namespaces: http://msdn.microsoft.com/en-us/library/0888kc6a%28VS.80%29.aspx Don't recall if this affected classes inside unnamed namespace though. I think VC++ 2005's support for unnamed namespace is better than nested classes (cl.exe has some serious bugs in resolving nested class names) so if our only alternative to unnamed namespace is nested classes, then I'd much prefer unnamed namespace. Adam, do you recall any compiler/debugger issues with classes inside unnamed namespace on VC++ 2005? Nope, but I haven't used VC 2005 in many years. Oops, sorry I meant to cc A. Roben. Adam (R), do you remember any issues with unnamed namespace? - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
