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] paged media update?
On Tuesday 15 May 2012 15:20:14 adam wrote: hi Hey there! Just wanted to know if Milian and Dave Hyatt spoke about the paged media updates? In the next months I might also be able to find some resources to help but just wanting to know what the current plan is... I'm still waiting for feedback from Dave Hyatt, yet I fear he's still on sick leave. Lets hope he gets well soon, I'll keep you posted. PS: Great to hear you are interested in working on this as well - I'd appreciate any kind of help in getting this beast implemented! -- Milian Wolff | milian.wo...@kdab.com | Software Engineer KDAB (Deutschland) GmbHCo KG, a KDAB Group company Tel. Germany +49-30-521325470, Sweden (HQ) +46-563-540090 KDAB - Qt Experts - Platform-independent software solutions signature.asc Description: This is a digitally signed message part. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] paged media update?
Ah, Sorry I didnt know he was sick. Best wishes to him. adam On 16/05/12 11:56, Milian Wolff wrote: On Tuesday 15 May 2012 15:20:14 adam wrote: hi Hey there! Just wanted to know if Milian and Dave Hyatt spoke about the paged media updates? In the next months I might also be able to find some resources to help but just wanting to know what the current plan is... I'm still waiting for feedback from Dave Hyatt, yet I fear he's still on sick leave. Lets hope he gets well soon, I'll keep you posted. PS: Great to hear you are interested in working on this as well - I'd appreciate any kind of help in getting this beast implemented! ___ 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 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] paged media update?
The difficulty with implementing paged media right now is that I'm beginning work on re-architecting pagination to unify columns, pages and regions. You can see the beginnings of this with the new multi-column classes that have been added to the tree. The basic idea is that multi-column layout is being separated from RenderBlock and moved to a subclass, RenderMultiColumnBlock. Similar to how regions work, the content is represented by a flow thread and by regions that the flow thread is placed into. In order to avoid creating thousands of regions for columns, I've introduced the concept of a region set, i.e., RenderMultiColumnSet. Pages ultimately need to be rebuilt to work the same way, i.e., to have a RenderPageFlowThread and a RenderPageSet. Once we have actual renderers that represent these objects, it will be a lot easier to make real boxes for things like margins, headers, footers, etc. The idea is that a set represents a contiguous run of objects that can all be rendered identically. If you're forced to have a different kind of object, e.g., a page with a different width/height or different style, then you break up the RenderPageSet and have multiple sets for each (this is similar to how regions work by default, although I hope to apply the same optimization to regions eventually as well if runs of same width/height regions are detected that are also siblings in the render tree). dave ___ 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
[webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: - Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345respectively - : and = delimiters are no longer needed - All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE *Note*: Just listing a test DOES NOT automatically skip a test even after my patch is applied: test.html will result in a warning, and we still expect it to pass. I'll cite Ojan's comment for the rationale: The bigger problem with implying SKIP, is how do you decide when SKIP isn't applied? That needs some hidden set of rules that's too complicated IMO. We can come back to this problem later if we wanted but let us start from the above cleanups first. For those who are interested, we're also going to rename test_expectations.txt to something more WebKitty on https://bugs.webkit.org/show_bug.cgi?id=86690. Best regards, 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] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE I wonder how this is gonna help making test_expectation.txt less obscure. If anything, I would make the file more verbose. I also think it could be split between regular failure, test expectations that we expect to diverge, and temporary failures that just need a rebaseline. For temporary failure, a system that would avoid conflicts on update/pull would be nice. Cheers, Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
Any concrete proposals? On May 16, 2012 9:23 PM, Benjamin Poulain benja...@webkit.org wrote: On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE I wonder how this is gonna help making test_expectation.txt less obscure. If anything, I would make the file more verbose. I also think it could be split between regular failure, test expectations that we expect to diverge, and temporary failures that just need a rebaseline. For temporary failure, a system that would avoid conflicts on update/pull would be nice. Cheers, Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE As I've commented in the bug, I'm not a fan of the proposed syntax above :( Note: Just listing a test DOES NOT automatically skip a test even after my patch is applied: test.html will result in a warning, and we still expect it to pass. I'll cite Ojan's comment for the rationale: The bigger problem with implying SKIP, is how do you decide when SKIP isn't applied? That needs some hidden set of rules that's too complicated IMO. As I've also noted in the bug, I don't follow this ... how is this any more complicated than if there's no modifier, default to SKIP? Why is defaulting to PASS any easier? We can come back to this problem later if we wanted but let us start from the above cleanups first. For those who are interested, we're also going to rename test_expectations.txt to something more WebKitty on https://bugs.webkit.org/show_bug.cgi?id=86690. I would have preferred it if you'd phrased this as proposed renaming test_expectations.txt :). It's not clear to me that renaming it to any of the other suggestions thus far are improvements, and there are lots of people (and scripts) used to the existing names. I'm sorry for sounding negative, but a lot of this feels like superficial stylistic tweaks, so I would be inclined to leave things the way they are unless there is a clear consensus that people prefer the new syntax? -- Dirk ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively I think I might prefer webkit.org/b/12345 - it's a bit less concise but the meaning is more contextually obvious and you can cut and paste it into a browser address field to follow the link, rather than having to know what site to enter it into. : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE Would it be possible to also change the modifiers to lower case or mixed case? All-caps is hard to type and uncomfortable to read. As for where the modifiers go, was there a logical basis for the previous separation into two groups? Would we be losing something by doing that? Cheers, Maciej ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:59 PM, Dirk Pranke dpra...@chromium.org wrote: On Wed, May 16, 2012 at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: For those who are interested, we're also going to rename test_expectations.txt to something more WebKitty on https://bugs.webkit.org/show_bug.cgi?id=86690. I would have preferred it if you'd phrased this as proposed renaming test_expectations.txt :). It's not clear to me that renaming it to any of the other suggestions thus far are improvements, and there are lots of people (and scripts) used to the existing names. It needs to be renamed to be consistent with the naming convention we use in LayoutTests and the rest of the WebKit directories (e.g. ChangeLog, Skipped, etc...). Besides files in webkitpy, few tests, and maybe few other files, we don't use underscore delimited file names. I even think that we shouldn't use it in webkitpy either but that's an orthogonal issue. I'm signing up to do the work since I've been always annoyed and nobody has signed up to do it so far. - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 9:34 PM, Ryosuke Niwa rn...@webkit.org wrote: Any concrete proposals? I would go for something verbose in a easy to understand format. E.g. { test: 'fast/html/keygen.html', platform: 'mac, linux', bug: 'webkit.org/b/6', expectedResults: ['crash', 'text', 'image'], action: 'skip' } For the tests that need rebaseline or new results, I would leave them out of the file. I would use an second file, with entries sorted by bug number. I don't know all the problems related to test_expectations so that is just some ideas from what I encountered in the past. Cheers, Benjamin ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 10:30 PM, Maciej Stachowiak m...@apple.com wrote: On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: Hi, There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively I think I might prefer webkit.org/b/12345 - it's a bit less concise but the meaning is more contextually obvious and you can cut and paste it into a browser address field to follow the link, rather than having to know what site to enter it into. : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE Would it be possible to also change the modifiers to lower case or mixed case? All-caps is hard to type and uncomfortable to read. As for where the modifiers go, was there a logical basis for the previous separation into two groups? Would we be losing something by doing that? There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). I think it makes sense to syntactically separate at least the platform/configuration keywords from the outcome keywords. It might be nice to separate the other things somehow as well, but I don't have any great ideas for things that are clearly better than the existing left/right convention. ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev
Re: [webkit-dev] Simplifying syntax in test_expectations.txt (bug 86691)
On Wed, May 16, 2012 at 10:39 PM, Dirk Pranke dpra...@chromium.org wrote: On Wed, May 16, 2012 at 10:30 PM, Maciej Stachowiak m...@apple.com wrote: On May 16, 2012, at 9:08 PM, Ryosuke Niwa rn...@webkit.org wrote: There has been some complaints / discussions about how syntax in test_expectations.txt is confusing (and I agree with you) on webkit-dev and at contributors' meeting. So I have a patch on https://bugs.webkit.org/show_bug.cgi?id=86691 to simplify syntax in test_expectation.txt as follows: Bug modififers are bug numebr themselves for WebKit and URLs for non-WebKit bugs e.g. 12345 and crbug.com/12345 instead of BUGWK12345 and BUGCR12345 respectively I think I might prefer webkit.org/b/12345 - it's a bit less concise but the meaning is more contextually obvious and you can cut and paste it into a browser address field to follow the link, rather than having to know what site to enter it into. : and = delimiters are no longer needed All modifiers and expectations show before test name e.g. 12345 WIN MAC TEXT IMAGE test.html instead of BUGWK12345 WIN MAC : test.html = TEXT IMAGE Would it be possible to also change the modifiers to lower case or mixed case? All-caps is hard to type and uncomfortable to read. As for where the modifiers go, was there a logical basis for the previous separation into two groups? Would we be losing something by doing that? There was a semi-logical basis, in that the stuff on the right of the test clearly specified the outcome of the test (PASS, IMAGE, TEXT, etc.). The stuff on the left was less well defined: there's the bug numbers, the platform/configuration info (MAC LINUX RELEASE, etc.), and some other stuff that there is less of a good place for (SLOW, SKIP, WONTFIX). What I find most annoying with the current syntax is that modifiers and expectations appear on the opposite ends of a test name. So if I'm copying pasting a test name to add a new entry to test_expectations.txt, then I would have to type something on both ends instead of typing stuff in before or after pasting the test name. Taking Benjamin's idea to make the meaning of tokens explicit and Maciej's (and my original) proposal to use webkit.org/b/12345, how about something like webkit.org/12356 [Win Mac] [Text] failing-test.html rniwa [Linux] [Skip] temporarily-skipped-test.html Here, platforms and expectations are wrapped around [] and non-URL comments are wrapped in . (I think we should just use IRC nick). - Ryosuke ___ webkit-dev mailing list webkit-dev@lists.webkit.org http://lists.webkit.org/mailman/listinfo.cgi/webkit-dev