Re: [webkit-dev] Using namespace std
On Tue, May 15, 2012 at 11:46 AM, Peter Kasting pkast...@chromium.orgwrote: On Tue, May 15, 2012 at 11:37 AM, Ryosuke Niwa rn...@webkit.org wrote: On May 15, 2012 10:53 AM, Peter Kasting pkast...@chromium.org wrote: Given how little of std:: we actually use (since WTF is used instead for most things), what about just explicitly qualifying usages with std:: directly? Can we do that if and only if we have conflicts? Well, I guess we can do anything we want :). It might be nice to have a consistent rule though (much like the current style rule is consistent, if sometimes problematic). That's a good point. On Wed, May 16, 2012 at 12:04 PM, Anders Carlsson ander...@apple.com wrote: On May 16, 2012, at 9:31 AM, Darin Adler da...@apple.com wrote: On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote: there is another conflict which is entirely our own fault. It is between WTF::bind and the new std::bind from C++11 We should find a good solution for this. I’d suggest talking with Anders Carlsson about it. I've run into this and had to use the fully qualified WTF::bind in those cases. FWIW, I don't think we really need using directives for the std namespace - the fully qualified name is short enough and I like the additional clarity that we're calling something in the standard library. A fair point especially since we don't use std functions / classes that often. It appears to me using fully qualified names (e.g. std::max(~) at call site) is far superior to using directive for individual symbols (e.g. using std::max). - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On Thu, May 17, 2012 at 12:02 PM, Ryosuke Niwa rn...@webkit.org wrote: It appears to me using fully qualified names (e.g. std::max(~) at call site) is far superior to using directive for individual symbols (e.g. using std::max). It sounds in general like a number of people have been in favor of this, and there hasn't been any opposition. Should we say that the rule for things in std:: is to qualify at the callsite rather than use either form of a using directive? Is there any disagreement with doing that? (I assume we'd leave the existing rules unchanged for other namespaces.) PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On May 17, 2012, at 12:33 PM, Peter Kasting pkast...@chromium.org wrote: On Thu, May 17, 2012 at 12:02 PM, Ryosuke Niwa rn...@webkit.org wrote: It appears to me using fully qualified names (e.g. std::max(~) at call site) is far superior to using directive for individual symbols (e.g. using std::max). It sounds in general like a number of people have been in favor of this, and there hasn't been any opposition. Should we say that the rule for things in std:: is to qualify at the callsite rather than use either form of a using directive? Is there any disagreement with doing that? Go for it! - Anders ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On Tuesday 15 May 2012, Darin Adler wrote: On May 15, 2012, at 5:48 AM, Allan Sandfeld Jensen wrote: To me it seems like an odd practice, so I would like to ask what original rationale behind that style guideline is Adding a list of using declarations like using std::min to the top of each source file would give us another ongoing maintenance job. The list in each file would grow, we’d not remember to remove them when they are no longer needed, and so on. Adding using namespace std to the top of a source file deals with 99% of the issue without the trouble of maintaining another list at the top of each file. That is reasonable. I am just generally wary of opening external namespaces, since they can eventually expand and end up causing conflicts. When there is a conflict, there are typically many simple ways to resolve the conflict. Removing using namespace std entirely is not the only solution and we should avoid it if possible. I see in your patch in https://bugs.webkit.org/show_bug.cgi?id=86465 that you have decided to remove it from many files, and before we do that I suggest we first investigate the other solutions. Unfortunately your patch does not say what the conflict is; I can’t make a helpful suggestion for an alternate solution without that information. The conflict is between isinf, isnan and std::isinf and std::isnan, but the conflict only exists in C++11 when constexpr versions are introduced. Since the problem exists in both GCC 4.6 and 4.7, I see no alternative than ensuring any files that uses isinf or isnan does not open the std namespace. This is only around 10 files in total as you can see in my patch. Also as I note in https://bugs.webkit.org/show_bug.cgi?id=59249, the only std functions used in all these cases are std::min, std::max and std::numeric_limits. If these three functions were declared using in MathExtra.h then none of these files would need any using std declarations. Best regards `Allan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On Tuesday 15 May 2012, Darin Adler wrote: On May 15, 2012, at 10:04 AM, Allan Sandfeld Jensen wrote: The conflict is between isinf, isnan and std::isinf and std::isnan, but the conflict only exists in C++11 when constexpr versions are introduced. We should try harder to come up with a solution in MathExtras.h. Even one that uses macros. If I understand correctly, the conflict here is not between some WebKit namespace and the std namespace, it’s in the C++ library itself between the global namespace and the std namespace. I think that’s a bug in the C++ library, and MathExtras.h is the perfect place for a workaround for that bug. True. If we can find a method that would work, that would be the optimal solution, but it is a difficult case to work around. It is not that I haven't tried, but modifying those ~10 files was just a lot easier in the end. Since the problem exists in both GCC 4.6 and 4.7, I see no alternative than ensuring any files that uses isinf or isnan does not open the std namespace. This is only around 10 files in total as you can see in my patch. Sure, but that assumes we won’t use more things from the std namespace in the future, and I am not sure that’s a good assumption. You don't have to open the namespace to use the methods within. I prefer to use the 'std::' prefix. I only kept them in this dcase to keep the changes to a minimum. Best regards `Allan ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On Wednesday 16 May 2012, Allan Sandfeld Jensen wrote: On Tuesday 15 May 2012, Darin Adler wrote: On May 15, 2012, at 10:04 AM, Allan Sandfeld Jensen wrote: The conflict is between isinf, isnan and std::isinf and std::isnan, but the conflict only exists in C++11 when constexpr versions are introduced. We should try harder to come up with a solution in MathExtras.h. Even one that uses macros. If I understand correctly, the conflict here is not between some WebKit namespace and the std namespace, it’s in the C++ library itself between the global namespace and the std namespace. I think that’s a bug in the C++ library, and MathExtras.h is the perfect place for a workaround for that bug. True. If we can find a method that would work, that would be the optimal solution, but it is a difficult case to work around. It is not that I haven't tried, but modifying those ~10 files was just a lot easier in the end. Okay I found a solution and have updated the patch in https://bugs.webkit.org/show_bug.cgi?id=86465. It is not pretty, but it works I should point out though, that there is another conflict which is entirely our own fault. It is between WTF::bind and the new std::bind from C++11, we have even declared 'using WTF::bind' in the global namespace, which makes conflicts much easier to trigger. On top of that several of the files using bind, had declared 'using namespace std' even though they do not using any functions from std. We can work around issues like this, but I would still prefer if we advised against 'using namespace std', as seen even more clearly in the 'bind' case, it can easily cause problems when upgrading standard libraries or compilers. Best regards. `Allan Jensen ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote: there is another conflict which is entirely our own fault. It is between WTF::bind and the new std::bind from C++11 We should find a good solution for this. I’d suggest talking with Anders Carlsson about it. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On May 16, 2012, at 9:31 AM, Darin Adler da...@apple.com wrote: On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote: there is another conflict which is entirely our own fault. It is between WTF::bind and the new std::bind from C++11 We should find a good solution for this. I’d suggest talking with Anders Carlsson about it. I've run into this and had to use the fully qualified WTF::bind in those cases. FWIW, I don't think we really need using directives for the std namespace - the fully qualified name is short enough and I like the additional clarity that we're calling something in the standard library. - Anders ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On Wed, May 16, 2012 at 3:04 PM, Anders Carlsson ander...@apple.com wrote: On May 16, 2012, at 9:31 AM, Darin Adler da...@apple.com wrote: On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote: there is another conflict which is entirely our own fault. It is between WTF::bind and the new std::bind from C++11 We should find a good solution for this. I’d suggest talking with Anders Carlsson about it. I've run into this and had to use the fully qualified WTF::bind in those cases. FWIW, I don't think we really need using directives for the std namespace - the fully qualified name is short enough and I like the additional clarity that we're calling something in the standard library. Ditto. After awhile, one gets used to it and I for one especially appreciate the additional clarity that it brings when reading code in a very large project. - Anders ___ 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] Using namespace std
On Wed, May 16, 2012 at 12:04 PM, Anders Carlsson ander...@apple.comwrote: On May 16, 2012, at 9:31 AM, Darin Adler da...@apple.com wrote: On May 16, 2012, at 9:20 AM, Allan Sandfeld Jensen wrote: there is another conflict which is entirely our own fault. It is between WTF::bind and the new std::bind from C++11 We should find a good solution for this. I’d suggest talking with Anders Carlsson about it. I've run into this and had to use the fully qualified WTF::bind in those cases. FWIW, I don't think we really need using directives for the std namespace - the fully qualified name is short enough and I like the additional clarity that we're calling something in the standard library. I would be strongly in favor of using fully qualified names for std classes (std::bind instead of just bind). This isn't a problem in other large codebases, even ones that use far more types from std:: (like containers) and that have column limits. - James - Anders ___ 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] Using namespace std
On May 16, 2012, at 12:47 PM, James Robinson wrote: On Wed, May 16, 2012 at 12:04 PM, Anders Carlsson ander...@apple.com wrote: FWIW, I don't think we really need using directives for the std namespace - the fully qualified name is short enough and I like the additional clarity that we're calling something in the standard library. I would be strongly in favor of using fully qualified names for std classes (std::bind instead of just bind). I’m OK with that. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On May 15, 2012, at 5:48 AM, Allan Sandfeld Jensen wrote: To me it seems like an odd practice, so I would like to ask what original rationale behind that style guideline is Adding a list of using declarations like using std::min to the top of each source file would give us another ongoing maintenance job. The list in each file would grow, we’d not remember to remove them when they are no longer needed, and so on. Adding using namespace std to the top of a source file deals with 99% of the issue without the trouble of maintaining another list at the top of each file. When there is a conflict, there are typically many simple ways to resolve the conflict. Removing using namespace std entirely is not the only solution and we should avoid it if possible. I see in your patch in https://bugs.webkit.org/show_bug.cgi?id=86465 that you have decided to remove it from many files, and before we do that I suggest we first investigate the other solutions. Unfortunately your patch does not say what the conflict is; I can’t make a helpful suggestion for an alternate solution without that information. -- Darin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On Tue, May 15, 2012 at 9:31 AM, Darin Adler da...@apple.com wrote: On May 15, 2012, at 5:48 AM, Allan Sandfeld Jensen wrote: To me it seems like an odd practice, so I would like to ask what original rationale behind that style guideline is Adding a list of using declarations like using std::min to the top of each source file would give us another ongoing maintenance job. The list in each file would grow, we’d not remember to remove them when they are no longer needed, and so on. Given how little of std:: we actually use (since WTF is used instead for most things), what about just explicitly qualifying usages with std:: directly? I realize you probably feel like these sorts of qualifications make code less readable, but the impact of that seems minor to me -- the Chromium codebase gets away with doing it and uses std:: far more often than WebKit -- and it avoids the sort of maintenance headaches you mention. It also makes it clearer to a reader when we are actually pulling in types or functions from std:: (which might not always be what we wanted), and avoids any naming conflicts from pulling in the whole std:: namespace (which is apparently the reason for this thread). PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On May 15, 2012 10:53 AM, Peter Kasting pkast...@chromium.org wrote: On Tue, May 15, 2012 at 9:31 AM, Darin Adler da...@apple.com wrote: On May 15, 2012, at 5:48 AM, Allan Sandfeld Jensen wrote: To me it seems like an odd practice, so I would like to ask what original rationale behind that style guideline is Adding a list of using declarations like using std::min to the top of each source file would give us another ongoing maintenance job. The list in each file would grow, we’d not remember to remove them when they are no longer needed, and so on. Given how little of std:: we actually use (since WTF is used instead for most things), what about just explicitly qualifying usages with std:: directly? I realize you probably feel like these sorts of qualifications make code less readable, but the impact of that seems minor to me -- the Chromium codebase gets away with doing it and uses std:: far more often than WebKit -- and it avoids the sort of maintenance headaches you mention. It also makes it clearer to a reader when we are actually pulling in types or functions from std:: (which might not always be what we wanted), and avoids any naming conflicts from pulling in the whole std:: namespace (which is apparently the reason for this thread). Can we do that if and only if we have conflicts? An alternative solution is to forward conflicting symbols from std to WTF (i.e. make decisions about namespace in WTF) so that the rest of codebase can simply use the forwarded WTF symbols instead. We could even wrap whatever function we're using with a different name if we wanted. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Using namespace std
On Tue, May 15, 2012 at 11:37 AM, Ryosuke Niwa rn...@webkit.org wrote: On May 15, 2012 10:53 AM, Peter Kasting pkast...@chromium.org wrote: Given how little of std:: we actually use (since WTF is used instead for most things), what about just explicitly qualifying usages with std:: directly? Can we do that if and only if we have conflicts? Well, I guess we can do anything we want :). It might be nice to have a consistent rule though (much like the current style rule is consistent, if sometimes problematic). An alternative solution is to forward conflicting symbols from std to WTF (i.e. make decisions about namespace in WTF) so that the rest of codebase can simply use the forwarded WTF symbols instead. We could even wrap whatever function we're using with a different name if we wanted. From the bugs linked in the root post of this thread, it looks like the common offenders are std::min(), std::max(), and std::numeric_limits, with a guest appearance by std::make_pair(). It seems like either solution above -- explicitly qualifying usages of these, or forwarding them through WTF -- would probably work. One factor that makes me still favor the explicit qualification is that I've been burned enough by various compilers/environments defining min and max that anything that results in pulling those symbols into the global scope makes me anxious. PK ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
