//third_party/WebKit clang-format preparation patch

//third_party/WebKit clang-format preparation patch

This CL contains the manual changes required for the
Blink whitespace reformatting:
- Remove portions of check-webkit-style that complain
  about Chromium style formatting.
- Remove nested .clang-formats to ensure a globally
  consistent style.
- Enforce clang-format formatting to avoid losing
  formatting style coverage.

The subsequent CL will contain only automated
reformatting changes.

BUG=563793
R=danakj@chromium.org, dpranke@chromium.org, thakis@chromium.org

Committed: https://crrev.com/7055702f4e456753783d341e73043d311fcbfa85
Cr-Commit-Position: refs/heads/master@{#422255}

[dcheng, 2016-09-25 03:48:27]

dcheng@chromium.org changed reviewers:
+ thakis@chromium.org

[dcheng, 2016-09-25 03:48:27]

I haven't attempted a full cleanup of cpp.py. Basically, my methodology was:

Look at the output, see what things are firing after a clang-format, and remove.
If convenient, remove nearby things that are also pointless with clang-format
now.

I'm using the following command to try to filter out errors (and already
existing errors):
$ cat presubmit_output | grep -v "ASSERT is deprecated" | grep -v
"ASSERT_NOT_REACHED is deprecated" | grep -v "ASSERT_WITH_SECURITY_IMPLICATION"
| grep -v "A conditional or loop body must use braces if the statement is more
than one line long." | grep -v "ASSERT_UNUSED is deprecated" | grep -v "An else
if statement should be written as an if statement when the prior "if" concludes
with a return, break, continue or goto statement." | grep -v "adds no
information, so it should be removed." | grep -v "If one part of an if-else
statement uses curly braces, the other part must too." | grep -v "enum members
should use InterCaps with an initial capital letter." | grep -v "Don't use
underscores in your identifier names." | grep -v "concludes with a return,
break, continue or goto statement" | grep -v "Local variables should never be
PassRefPtr" | grep -v "is redundant since function is already declared as " |
grep -v "defined as a bitfield of type " | grep -v "Tests for true/false and
null/non-null should be done without equality comparisons." | less

Whoever ends up landing will presumably patch this CL in before running
clang-format and submitting it one giant chunk of fun.

[dcheng, 2016-09-29 18:23:08]

+dpranke, mind looking over this staging CL? The idea is that tomorrow, whoever
is doing the clang-format commit will git cl patch this in, run clang-format -i
--style=file $(git ls-files third_party/WebKit | grep -E "\.cpp$|\.h$"), and
then commit the result.

[Dirk Pranke, 2016-09-29 20:36:26]

dpranke@chromium.org changed reviewers:
+ danakj@chromium.org, dpranke@chromium.org

[Dirk Pranke, 2016-09-29 20:36:27]

lgtm, assuming thakis@ or danakj@ also approves, since I'm not close to the
details of which things can be deleted and which need to stay.

[cbiesinger, 2016-09-29 20:39:08]

cbiesinger@chromium.org changed reviewers:
+ cbiesinger@chromium.org

[cbiesinger, 2016-09-29 20:39:08]

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/.clang-format (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/.clang-format:8: Standard: Cpp11
As one of the layout/ng engineers: Can we keep ReflowComments as true for this
directory?

[Nico, 2016-09-29 20:46:09]

dcheng: will look once home (traveling, should be home in 2-3h)

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/.clang-format (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/.clang-format:8: Standard: Cpp11
On 2016/09/29 20:39:08, cbiesinger wrote:
> As one of the layout/ng engineers: Can we keep ReflowComments as true for this
> directory?

Sure. I think the way we're going to roll out comment formatting is to manually
reflow comments per directory and add a . clang-format in each dir that's done,
so we can just put that here if the comments here are already short. (Note that
clang-format's comment reflowing really is unusably bad, you'll always want to
manually do this)

For organizational reasons we'll probably delete this file and add it back right
after the reformatting if that's cool.

[cbiesinger, 2016-09-29 20:47:17]

On 2016/09/29 20:46:09, Nico (mostly away until Oct 3) wrote:
> dcheng: will look once home (traveling, should be home in 2-3h)
> 
>
https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/layout/ng/.clang-format (left):
> 
>
https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/layout/ng/.clang-format:8: Standard: Cpp11
> On 2016/09/29 20:39:08, cbiesinger wrote:
> > As one of the layout/ng engineers: Can we keep ReflowComments as true for
this
> > directory?
> 
> Sure. I think the way we're going to roll out comment formatting is to
manually
> reflow comments per directory and add a . clang-format in each dir that's
done,
> so we can just put that here if the comments here are already short. (Note
that
> clang-format's comment reflowing really is unusably bad, you'll always want to
> manually do this)
> 
> For organizational reasons we'll probably delete this file and add it back
right
> after the reformatting if that's cool.

That's fine, just want to make sure we don't accidentally reintroduce longer
comments.

[danakj, 2016-09-29 20:54:05]

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/.clang-format (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/.clang-format:8: Standard: Cpp11
On 2016/09/29 20:39:08, cbiesinger wrote:
> As one of the layout/ng engineers: Can we keep ReflowComments as true for this
> directory?

If BasedOnStyle: Chromium means all the comments in here are 80 cols then
keeping ReflowComments makes sense. Same for other cases.

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py:1540: elif
search(r'\([ \t]+(?!(\s*\\)|\()', function_call):
All these whitespace things look redundant with the "is the patch
clang-formatted?" check

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py:1879: 'Blank
line at the start of a code block.  Is this needed?')
Doesn't clang-format kill these?

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py:1937: 'Missing
spaces around =')
..and fix these.. 

I'm inclined to suggest removing all "whitespace/xyz" checks.

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py:2318: for
current_line in clean_lines.elided[line_number + 1:]:
This whole loop has no errors left so it could all go away?

[dcheng, 2016-09-29 21:10:58]

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/.clang-format (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/.clang-format:8: Standard: Cpp11
On 2016/09/29 20:54:04, danakj wrote:
> On 2016/09/29 20:39:08, cbiesinger wrote:
> > As one of the layout/ng engineers: Can we keep ReflowComments as true for
this
> > directory?
> 
> If BasedOnStyle: Chromium means all the comments in here are 80 cols then
> keeping ReflowComments makes sense. Same for other cases.

We'll reland this afterwards, but for the giant commit, it's a bit safer to just
delete any nested .clang-formats and reland any that are still desired after the
reformat. The idea is to prevent any interesting surprises in the giant commit
(which we've been testing).

Does this sound reasonable to you?

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py:1879: 'Blank
line at the start of a code block.  Is this needed?')
On 2016/09/29 20:54:04, danakj wrote:
> Doesn't clang-format kill these?

(This also applies to the previous comment)
It does, but I'm trying to minimize the number of manual changes we land
alongside the giant reformat. The main goal of these deletions is to make
check-webkit-style not complain about clang-formatted code, not to eliminate all
things that would be redundant with clang-format.

I do plan on cleaning this up after clang-format lands and sticks, I'm just
being very conservative with what's removed for now (since each
check-webkit-style run takes several minutes)

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py:2318: for
current_line in clean_lines.elided[line_number + 1:]:
On 2016/09/29 20:54:04, danakj wrote:
> This whole loop has no errors left so it could all go away?

Done.

[danakj, 2016-09-29 21:19:57]

LGTM

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/layout/ng/.clang-format (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/layout/ng/.clang-format:8: Standard: Cpp11
On 2016/09/29 21:10:58, dcheng wrote:
> On 2016/09/29 20:54:04, danakj wrote:
> > On 2016/09/29 20:39:08, cbiesinger wrote:
> > > As one of the layout/ng engineers: Can we keep ReflowComments as true for
> this
> > > directory?
> > 
> > If BasedOnStyle: Chromium means all the comments in here are 80 cols then
> > keeping ReflowComments makes sense. Same for other cases.
> 
> We'll reland this afterwards, but for the giant commit, it's a bit safer to
just
> delete any nested .clang-formats and reland any that are still desired after
the
> reformat. The idea is to prevent any interesting surprises in the giant commit
> (which we've been testing).
> 
> Does this sound reasonable to you?

Yes

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
File third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py (left):

https://codereview.chromium.org/2366153002/diff/1/third_party/WebKit/Tools/Sc...
third_party/WebKit/Tools/Scripts/webkitpy/style/checkers/cpp.py:1879: 'Blank
line at the start of a code block.  Is this needed?')
On 2016/09/29 21:10:58, dcheng wrote:
> On 2016/09/29 20:54:04, danakj wrote:
> > Doesn't clang-format kill these?
> 
> (This also applies to the previous comment)
> It does, but I'm trying to minimize the number of manual changes we land
> alongside the giant reformat. The main goal of these deletions is to make
> check-webkit-style not complain about clang-formatted code, not to eliminate
all
> things that would be redundant with clang-format.
> 
> I do plan on cleaning this up after clang-format lands and sticks, I'm just
> being very conservative with what's removed for now (since each
> check-webkit-style run takes several minutes)

okay. Given the complexity here I was not sure if it makes your life easier or
harder to delete a lot more code.

[Nico, 2016-09-30 00:37:15]

lgtm also. Thanks!

https://codereview.chromium.org/2366153002/diff/20001/third_party/WebKit/.cla...
File third_party/WebKit/.clang-format (right):

https://codereview.chromium.org/2366153002/diff/20001/third_party/WebKit/.cla...
third_party/WebKit/.clang-format:10: SortIncludes: false
(we can remove this, it's implicitly off by default in clang-format for style
Chromium for now)

[dcheng, 2016-09-30 16:43:58]

https://codereview.chromium.org/2366153002/diff/20001/third_party/WebKit/.cla...
File third_party/WebKit/.clang-format (right):

https://codereview.chromium.org/2366153002/diff/20001/third_party/WebKit/.cla...
third_party/WebKit/.clang-format:10: SortIncludes: false
On 2016/09/30 00:37:15, Nico (mostly away until Oct 3) wrote:
> (we can remove this, it's implicitly off by default in clang-format for style
> Chromium for now)

Done.

[dcheng, 2016-09-30 22:49:31]

Description was changed from

==========
DO NOT COMMIT

Pre clang-format prep patch.

BUG=563793
==========

to

==========
//third_party/WebKit clang-format preparation patch

Pre clang-format prep patch.

BUG=563793
==========

[dcheng, 2016-09-30 22:52:47]

Description was changed from

==========
//third_party/WebKit clang-format preparation patch

Pre clang-format prep patch.

BUG=563793
==========

to

==========
//third_party/WebKit clang-format preparation patch

This CL contains the manual changes required for the
Blink whitespace reformatting:
- Remove portions of check-webkit-style that complain
  about Chromium style formatting.
- Remove nested .clang-formats to ensure a globally
  consistent style.
- Enforce clang-format formatting to avoid losing
  formatting style coverage.

The subsequent CL will contain only automated
reformatting changes.

BUG=563793
==========

[commit-bot: I haz the power, 2016-09-30 23:11:13]

Description was changed from

==========
//third_party/WebKit clang-format preparation patch

This CL contains the manual changes required for the
Blink whitespace reformatting:
- Remove portions of check-webkit-style that complain
  about Chromium style formatting.
- Remove nested .clang-formats to ensure a globally
  consistent style.
- Enforce clang-format formatting to avoid losing
  formatting style coverage.

The subsequent CL will contain only automated
reformatting changes.

BUG=563793
==========

to

==========
//third_party/WebKit clang-format preparation patch

This CL contains the manual changes required for the
Blink whitespace reformatting:
- Remove portions of check-webkit-style that complain
  about Chromium style formatting.
- Remove nested .clang-formats to ensure a globally
  consistent style.
- Enforce clang-format formatting to avoid losing
  formatting style coverage.

The subsequent CL will contain only automated
reformatting changes.

BUG=563793
R=danakj@chromium.org, dpranke@chromium.org, thakis@chromium.org

Committed: https://crrev.com/7055702f4e456753783d341e73043d311fcbfa85
Cr-Commit-Position: refs/heads/master@{#422255}
==========

[commit-bot: I haz the power, 2016-09-30 23:11:14]

Patchset 3 (id:??) landed as
https://crrev.com/7055702f4e456753783d341e73043d311fcbfa85
Cr-Commit-Position: refs/heads/master@{#422255}