[webkit-reviews] review denied: [Bug 123406] check-webkit-style should support C++11 rvalue references : [Attachment 217418] Patch
bugzilla-daemon at webkit.org
bugzilla-daemon at webkit.org
Wed Nov 20 09:31:17 PST 2013
Brent Fulgham <bfulgham at webkit.org> has denied László Langó
<lango at inf.u-szeged.hu>'s request for review:
Bug 123406: check-webkit-style should support C++11 rvalue references
https://bugs.webkit.org/show_bug.cgi?id=123406
Attachment 217418: Patch
https://bugs.webkit.org/attachment.cgi?id=217418&action=review
------- Additional Comments from Brent Fulgham <bfulgham at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=217418&action=review
A good start, but I think your logic in the "check_style" change is flawed. I
think you allow rvalue references now at the cost of ignoring boolean
expressions crossing multiple lines. Please add a test for the multiple-line
case and fix the logic for identifying rvalue references.
> Tools/Scripts/webkitpy/style/checkers/cpp.py:2626
> + if cleansed_line.strip().endswith('||') or
cleansed_line.strip().endswith(' &&'):
Won't strip() remove all the whitespace? Will we ever match ' &&'?
> Tools/Scripts/webkitpy/style/checkers/cpp_unittest.py:629
> + self.assert_lint('T&&', '')
We also need a test that shows that we catch instances of boolean expressions
spanning multiple lines. I believe that your change will break that style
check. You can prove me wrong by including a test case! :-)
More information about the webkit-reviews
mailing list