1317821 - Run clang-format on ipc/glue

Status: RESOLVED WORKSFORME

(Core :: IPC, defect, P3)

Description

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

I think this makes sense to do. I made a quick pass over the patches and everything looks fine to me. The code in this directory has gone through a mix of styles, so the tool should help a lot.

Comment 1

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

Attachment: Bug 1317821 - Reformat ipc/glue with clang-format ()

Review commit: https://reviewboard.mozilla.org/r/93296/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/93296/

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 8811054 [details]
Bug 1317821 - Reformat ipc/glue with clang-format ()

https://reviewboard.mozilla.org/r/93296/#review93294

Interesting, there are some places where this doesn't match Gecko style - like missing braces on if-statements with single-line bodies. Also, not a fan of stuff like this:

 ~ShutdownObserver() { AssertIsOnMainThread(); }
 
Looks nicer, and is easier to step-through and modify as:

 ~ShutdownObserver() {
   AssertIsOnMainThread();
 }
 
But overall this looks like a big improvement.

Comment 3

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8811054 [details]
Bug 1317821 - Reformat ipc/glue with clang-format ()

https://reviewboard.mozilla.org/r/93296/#review93444

::: ipc/.clang-format:1
(Diff revision 1)
> +BasedOnStyle: Mozilla

Why you don't use .clang-format in the base directory?

We should like the number of different coding style in the base code.

Comment 4

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8811054 [details]
Bug 1317821 - Reformat ipc/glue with clang-format ()

https://reviewboard.mozilla.org/r/93296/#review93444

> Why you don't use .clang-format in the base directory?
> 
> We should like the number of different coding style in the base code.

s/like/limit/

Comment 5

[Bill McCloskey [inactive unless it's an emergency] (:billm)]

(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> Comment on attachment 8811054 [details]
> Bug 1317821 - Reformat ipc/glue with clang-format ()
> 
> https://reviewboard.mozilla.org/r/93296/#review93444
> 
> > Why you don't use .clang-format in the base directory?
> > 
> > We should like the number of different coding style in the base code.
> 
> s/like/limit/

The .clang-format file I'm checking in more closely matches Gecko style. Ideally we would use the ipc/.clang-format file in the rest of Gecko, but some of the directives in the ipc version require clang-format-4.0. I don't know who uses clang-format at the moment, so I'd rather not break anyone's workflow. Changing only ipc/ seems like a good first step.

Once this lands I'll post to dev-platform and see if anyone objects to requiring clang-format-4.0.

Comment 6

[Sylvestre Ledru [:Sylvestre]]

> Once this lands I'll post to dev-platform and see if anyone objects to requiring clang-format-4.0.
Thanks for the great input.

I updated  the root clang-format in bug 1322321 and push the change in clang upstream directly
https://reviews.llvm.org/rL289660

So, you can probably remove your local change in your patch.
Thanks again

Comment 7

[Sylvestre Ledru [:Sylvestre]]

Attachment: ipc.diff

here is what it looks like with the current set of rules

Comment 8

[Andrew McCreight [:mccr8]]

The entire tree has long since been formatted.