[Automated review] We should pick one style of PointerBindsToType
Categories
(Developer Infrastructure :: Source Code Analysis, task)
Tracking
(firefox68 fixed)
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.
Assignee | ||
Updated•6 years ago
|
Assignee | ||
Updated•6 years ago
|
Comment 1•6 years ago
|
||
void* foo
is the Gecko standard, at least de facto.
Assignee | ||
Comment 2•6 years ago
|
||
And what we had in the Mozilla coding style.
https://github.com/llvm-mirror/clang/blob/329eb5c6baeca0a1bd6b1a5bd55e41de786f39fa/lib/Format/Format.cpp#L954
Assignee | ||
Comment 3•6 years ago
|
||
Depends on D28953
Assignee | ||
Comment 4•6 years ago
|
||
ignore-this-changeset
Depends on D28954
Assignee | ||
Comment 5•6 years ago
|
||
Left position: 1032 files touched, patch: 8.3M
Right : 10067 files touched, patch: 95M
Comment 7•6 years ago
|
||
bugherder |
https://hg.mozilla.org/mozilla-central/rev/25e1607e6f1e
https://hg.mozilla.org/mozilla-central/rev/e1993a1f09ac
Comment 8•6 years ago
|
||
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?
Comment 9•6 years ago
|
||
(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.)
Comment 10•6 years ago
|
||
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®exp=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).
Comment 11•6 years ago
|
||
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.
Updated•3 years ago
|
Updated•2 years ago
|
Description
•