Closed Bug 1547143 Opened 6 years ago Closed 6 years ago

[Automated review] We should pick one style of PointerBindsToType

Categories

(Developer Infrastructure :: Source Code Analysis, task)

task
Not set
normal

Tracking

(firefox68 fixed)

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: glandium, Assigned: Sylvestre)

Details

Attachments

(2 files)

Phabricator URL: https://phabricator.services.mozilla.com/D28757

Currently, clang-format will apparently pick the most used kind in the file, of void* foo vs void *foo.

We should pick one and apply it consistently everywhere.

Type: defect → task
Regressions: clang-format
Assignee: nobody → sledru

void* foo is the Gecko standard, at least de facto.

Depends on D28953

ignore-this-changeset

Depends on D28954

Left position: 1032 files touched, patch: 8.3M
Right : 10067 files touched, patch: 95M

Pushed by sledru@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/25e1607e6f1e Force the pointer style declaration r=Ehsan https://hg.mozilla.org/integration/autoland/rev/e1993a1f09ac Format the tree: Be prescriptive with the pointer style (left) r=Ehsan
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68

Hmm, we've been reformatting comm-central in bug 1546364 and it looks like the the stars went mostly to the right. With this change, we'd have to reformat everything again. Is there a way to maintain what we have so far?

EDIT: See bug 1546364 comment #13. Could we put PointerAlignment: Right for comm-central code?

(In reply to Jorg K (GMT+2) from comment #8)

Hmm, we've been reformatting comm-central in bug 1546364 and it looks like the the stars went mostly to the right. With this change, we'd have to reformat everything again. Is there a way to maintain what we have so far?

EDIT: See bug 1546364 comment #13. Could we put PointerAlignment: Right for comm-central code?

.clang-format files are a very simple format which doesn't allow for any kind of "configuration", so we can't express "if c-c do this else do that". If you would like to follow a different style in c-c the way to do that is through using a separately maintained .clang-format file. (Obviously the downside to that would be that you'd need to pick up upstream changes to that file manually if you'd like to keep up with m-c coding style changes.)

Thanks for the comment. We actually followed M-C style until you decided to change that style (and reformatted parts of M-C with an 8.4MB patch). I think, reformatting is a pretty disruptive action, so we're doing it in C-C before the branch to 68 beta.

Following the pointer style change from "prevalent" to "left" would mean reformatting 100% of C-C code again. I'm actually not looking forward to ever doing any reformatting again. What coding style changes do you have in mind? We might wish to follow some of them if some sporadic "ugliness" were removed. For example I noticed

  rv =
      Foo(x, y, ...

so some lonely LHS, and also

  rv = FooBarBaz(
      a, b, c, ...

where I really don't understand why why not more is placed onto the first line. Like:
https://searchfox.org/comm-central/rev/31ebd78f17e1cd8d486323130d7d939a0046fedf/mailnews/base/search/src/nsMsgFilterService.cpp#433
or just look for clang-format off in general:
https://searchfox.org/comm-central/search?q=clang-format+off&case=false&regexp=false&path=

I scanned hundreds of thousands of lines of code to fix some of the worst ugliness, especially around inline comments (obviously unrelated to the lonely LHS).

I am explicitly not going to enter a discussion on coding style details here. I hope you'll forgive me for refraining to enter such a discussion, but m-c has moved on from caring about whitespace explicitly to avoid having these discussions.

The answer to all questions around the theme of "what coding style decisions will m-c make in the future?" will be: "it will continue running clang-format and not treat whitespace formatting as something humans should worry about."

If that answer works for c-c, great, I suggest adopting the coding style of m-c and reaping the productivity benefits. If it doesn't, then I suggest figuring out what it is that you all would like to do and do that instead. I have no position on this choice whatsoever so it is entirely up to you all how you'd like to handle it since either way it won't impact m-c code.

No longer regressions: clang-format
Product: Firefox Build System → Developer Infrastructure
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: