Closed Bug 1274087 Opened 8 years ago Closed 3 months ago

prepare a repository that shows what clang-format on the entire tree looks like

Categories

(Developer Infrastructure :: Lint and Formatting, defect)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: dbaron, Unassigned)

References

Details

Attachments

(1 file)

One of the things that we should do in advance of bug 1188202 (essentially running clang-format on all of the non-imported code in the repository) is make sure that we're happy with it's output.

To do this, I'm going to suggest that somebody:

 (1) figure out the list of all code that should be formatted according to Mozilla style (to be used eventually in bug 1188202).  (Maybe we already have this list?)

 (2) Make a commit against mozilla-central (or maybe a series of commits) that uses clang-format to reformat all of that code

 (3) Put the repository somewhere public so that people can comment on whether the current clang-format options are the ones we want.

I'd like to avoid excessive bikeshedding on this, but at the same time I do want people to have a chance to check *before* we do mass changes for things that would lead to complaints afterwards, in particular:
 * to avoid changing style in ways that we actually didn't want, and
 * to avoid messing with blame where we actually didn't want to change style
https://github.com/staktrace/gecko-dev/tree/formatted is approximately what it might look like. I obtained this by running:

$ mv intl/unicharutil/util/nsUnicodePropertyData.{cpp,cppbk}
$ (for i in $(find . -type f -name "*.h" -o -name "*.cpp"); do echo $i; ~/.mozbuild/clang-format-3.5 -style='{BasedOnStyle: Mozilla, CommentPragmas: "^"}' -i $i; done) 2>&1 | tee clang-format.out
$ mv intl/unicharutil/util/nsUnicodePropertyData.{cppbk,cpp}

Notes:
- I had to move nsUnicodePropertyData file because clang-format goes bananas on it, eating up all the CPU for a long time.
- clang-format does crash/segfault on a few of the files
- The style I used is what is in the .clang-format file at the top of the tree. For some reason I couldn't get clang-format to actually read it from that file.
- The version of clang-format in my ~/.mozbuild folder might be out-of-date, it has a timestamp of Oct 15, 2014 and was built by anthony jones sometime before that.
Neat.

I think this shows that the clang-format configuration representing mozilla style still needs a good bit of work to represent mozilla style.  (Probably iterating on that work should involve seeing how the fixes influence the size of this diff.)

I'm attaching a list of a few issues I noticed.
(In reply to Kartikaya Gupta (email:kats@mozilla.com) from comment #1)
> - The version of clang-format in my ~/.mozbuild folder might be out-of-date,
> it has a timestamp of Oct 15, 2014 and was built by anthony jones sometime
> before that.

Indeed, the version of clang-format installed by mach is very old. I'm pretty short on time right now, but I'll try to get it updated over in bug 1226938 soonish.

(In reply to David Baron :dbaron: ⌚️UTC-7 (review requests must explain patch) from comment #2)
> I think this shows that the clang-format configuration representing mozilla
> style still needs a good bit of work to represent mozilla style.  (Probably
> iterating on that work should involve seeing how the fixes influence the
> size of this diff.)
> 
> I'm attaching a list of a few issues I noticed.

I actually already contributed patches for most of these issues (see bug 961541). The latest version of Clang-Format is much closer to Mozilla style when used together with the appropriate .clang-format options.

Space after `template` remains an issue, but unfortunately the Clang-Format maintainer doesn't consider that to be worthy of an option. We could fork Clang-Format, but probably not worth it for that alone.
(In reply to Birunthan Mohanathas [:poiru] from comment #3)Format
> maintainer doesn't consider that to be worthy of an option. We could fork
> Clang-Format, but probably not worth it for that alone.
Do you have a pointer to this statement? Thanks
(In reply to Sylvestre Ledru [:sylvestre] from comment #4)
> (In reply to Birunthan Mohanathas [:poiru] from comment #3)Format
> > maintainer doesn't consider that to be worthy of an option. We could fork
> > Clang-Format, but probably not worth it for that alone.
> Do you have a pointer to this statement? Thanks

Yes, see e.g. http://reviews.llvm.org/D10807 and http://reviews.llvm.org/D12921#247649
(In reply to Birunthan Mohanathas [:poiru] from comment #5)
> Yes, see e.g. http://reviews.llvm.org/D10807 and
> http://reviews.llvm.org/D12921#247649
Good news, Daniel accepted the change. As I wanted this to move quickly, I rebased your patch (and credited you in the svn commit comment), this already landed and should be in clang 4.0 (eta end of the year).
I pushed an updated version of the repo:
https://github.com/sylvestre/gecko-dev
This is using clang head with the patch mentioned in comment #6.

I am also ignoring thirdparty code & tests:

Here is the (crappy) script used:
CLANG_FORMAT=~/dev/debian/pkg-llvm/llvm-toolchain/branches/llvm-toolchain-snapshot_4.0~svn278786/build-llvm/bin/clang-format
mv intl/unicharutil/util/nsUnicodePropertyData.{cpp,cppbk}
(for i in $(find . -type f -name "*.h" -o -name "*.cpp"|grep -v -E '(media/webrtc/trunk/|media/ffvpx/|media/gmp-clearkey/|media/libvorbis/|media/gmp-clearkey/|media/openmax_il/|media/libopus/|media/libsoundtouch/|media/libcubeb/|media/mtransport/third_party|media/libstagefright/|media/omx-plugin/|intl/icu/|ipc/chromium/src/|gfx/angle/|gfx/thebes/|gfx/cairo/|gfx/graphite2|gfx/skia/|xpcom/tests/|dom/base/test/|dom/bindings/test/|dom/media/platforms/ffmpeg/ffmpeg57/|dom/media/platforms/ffmpeg/libav5|gfx/harfbuzz/|image/test/gtest/|ipc/ipdl/test/cxx/|ipc/testshell/|js/src/jsapi-tests/|media/mtransport/test/|mfbt/tests/|modules/brotli/|modules/fdlibm/|modules/freetype2/|modules/woff2/|modules/zlib/|other-licenses/|nsprpub/|security/nss/|security/sandbox/chromium/|testing/gtest/|storage/test/|toolkit/crashreporter/google-breakpad/|xpcom/tests/|xpcom/glue/tests/|widget/tests/|uriloader/exthandler/tests/tools/profiler/tests/|/obj-|testing/gtest/|mfbt/tests/|memory/jemalloc/|media/mtransport/|ipc/ipdl/test/|image/test|gfx/testsd|dom/media/gtest/|build/clang-plugin/tests/)'); do
	echo $i
	$CLANG_FORMAT -style='{BasedOnStyle: Mozilla, CommentPragmas: "^"}' -i $i
done) 2>&1 | tee clang-format.out
mv intl/unicharutil/util/nsUnicodePropertyData.{cppbk,cpp}



Still the same issue with intl/unicharutil/util/nsUnicodePropertyData.cpp
> The biggest thing I noticed is that it's reformatting "nsIFrame*
> aFrame" to "nsIFrame *aFrame", when we've been explicitly doing the
> opposite.
This is fixed and forced in my patch

> It's unindenting case: inside of switch, which we also don't want.
Fixed too


>  It's putting { on their own line for namespace, which we don't do.
Fixed thanks to Poiru's patch

>  It's turning empty functions at toplevel into {} at the end of a line,
>  which we don't want.  (But for functions not at toplevel, it's doing
>  the opposite in some places, which I think is good, and pushes things
>  towards the dominant style.)
Do you have an example?

> And it needs to handle indentation of NS_INTERFACE_MAP_ENTRY macros,
> since it's making those unreadable.
This is going to be hard to implement I guess.

> It's adding spaces after "template", which I think we don't do.
This is fixed


About nsUnicodePropertyData.cpp, this is just super slow, I reported a bug upstream:
https://llvm.org/bugs/show_bug.cgi?id=29014
(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> > And it needs to handle indentation of NS_INTERFACE_MAP_ENTRY macros,
> > since it's making those unreadable.
> This is going to be hard to implement I guess.

I already implemented this (see MacroBlockBegin and MacroBlockEnd).
(In reply to Birunthan Mohanathas [:poiru] from comment #9)
> (In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> > > And it needs to handle indentation of NS_INTERFACE_MAP_ENTRY macros,
> > > since it's making those unreadable.
> > This is going to be hard to implement I guess.
> 
> I already implemented this (see MacroBlockBegin and MacroBlockEnd).

Oh, great, thanks. I have to update the clang-format configuration to list these macros now, correct?
I did it and I pushed the update version

I only found these macros:
MacroBlockBegin: NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION
MacroBlockEnd: NS_INTERFACE_MAP_END

https://github.com/sylvestre/gecko-dev/commit/6a47f1d0538a6a1076809569b9feb7c178b21d63
(In reply to Sylvestre Ledru [:sylvestre] from comment #11)
> I did it and I pushed the update version
> 
> I only found these macros:
> MacroBlockBegin: NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION
> MacroBlockEnd: NS_INTERFACE_MAP_END

I had this in my .clang-format, but there are likely more:

  MacroBlockBegin: "^(NS_INTERFACE_MAP_BEGIN|NS_INTERFACE_TABLE_HEAD|NS_INTERFACE_MAP_BEGIN_CYCLE_COLLECTION|NS_IMPL_CYCLE_COLLECTION_.*_BEGIN)$"
  MacroBlockEnd: "^(NS_INTERFACE_MAP_END|NS_IMPL_CYCLE_COLLECTION_.*_END)"
(In reply to Sylvestre Ledru [:sylvestre] from comment #8)
> >  It's turning empty functions at toplevel into {} at the end of a line,
> >  which we don't want.  (But for functions not at toplevel, it's doing
> >  the opposite in some places, which I think is good, and pushes things
> >  towards the dominant style.)
> Do you have an example?

Not in the current version.

In the version from comment 1, the first example was DocAccessibleWrap::~DocAccessibleWrap() in accessible/atk/DocAccessibleWrap.cpp.


There are still some further examples of macro indent problems.  Some are because these macros:
NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED
NS_IMPL_CYCLE_COLLECTION_TRAVERSE_BEGIN_INHERITED
NS_IMPL_CYCLE_COLLECTION_UNLINK_BEGIN_INHERITED
aren't recognized as starting indentation.

or from having an extra level of indentation between:
NS_INTERFACE_TABLE_BEGIN
NS_INTERFACE_TABLE_END
(in dom/base/nsDocument.cpp)


I still tend to think that the opening { on its own line when a function or type is *not* at toplevel (i.e., when the opening { wouldn't be in column 1) isn't the dominant style.  This patch moves towards that, which is at least unusual for layout code.

It's also moving the terminating \ for multiline macros from column 79 to column 80, which makes diffs hard to read in an 80 character terminal.

I'm not happy about what it does to #defines that are lined up (e.g., in layout/style/nsStyleConsts.h), although maybe there's no good answer there.

This indentation also seems not to match our local style:
> nsStyleContext::CreateEmptyStyleData(const nsStyleStructID& aSID)
> {
>   MOZ_ASSERT(!mChild && !mEmptyChild &&
>-             !(mBits & nsCachedStyleData::GetBitForSID(aSID)) &&
>-             !GetCachedStyleData(aSID),
>+               !(mBits & nsCachedStyleData::GetBitForSID(aSID)) &&
>+               !GetCachedStyleData(aSID),
(and it isn't something clang-format does in if (), only in assertion macros).


Validating whether many of these complaints are valid should probably be done by seeing if fixing them (if easy) would reduce the size of the diff.


Overall I think this version does look more like what I recognize as Mozilla style than the version from comment 1.
Good, thanks for the feedback. I will have a look.
By the way, LLDB did a massive reformat of C++ & Python in this commit:
https://reviews.llvm.org/rL280751
> There are still some further examples of macro indent problems.  Some are because these macros:
> NS_INTERFACE_TABLE_HEAD_CYCLE_COLLECTION_INHERITED
[...]
This is done:
https://github.com/sylvestre/gecko-dev/commit/d2ad447624b99dc9dfadfc97ec7557db7c85e385

> It's also moving the terminating \ for multiline macros from column 79 to column 80, which makes 
> diffs hard to read in an 80 character terminal.
This is an upstream bug:
https://llvm.org/bugs/show_bug.cgi?id=28065

> This indentation also seems not to match our local style
reported upstream:
https://llvm.org/bugs/show_bug.cgi?id=30582
Andi is working on this bug: https://llvm.org/bugs/show_bug.cgi?id=28065
I tried to relaunch clang-format on the update source code with a fix for bug 28065
here is the result:
https://github.com/sylvestre/gecko-dev/commit/265611bbee6d5a6baaa1d5266c5d132085e751a8

Andi is also investigating bug 30582
I updated the changeset and push it again (some missing options).
Now, we have:
 11112 files changed, 1163160 insertions(+), 1207249 deletions(-)
Depends on: 1313040
Depends on: 1317305
So, removing the sort of the includes, here are the stats by components:

accessible :
 134 files changed, 4939 insertions(+), 5729 deletions(-)
b2g :
 3 files changed, 108 insertions(+), 100 deletions(-)
browser :
 9 files changed, 496 insertions(+), 524 deletions(-)
build :
 7 files changed, 1806 insertions(+), 1644 deletions(-)
caps :
 8 files changed, 1532 insertions(+), 1535 deletions(-)
chrome :
 4 files changed, 378 insertions(+), 377 deletions(-)
devtools :
 15 files changed, 672 insertions(+), 635 deletions(-)
docshell :
 21 files changed, 967 insertions(+), 1152 deletions(-)
dom :
 1472 files changed, 165392 insertions(+), 168238 deletions(-)
editor :
 61 files changed, 11673 insertions(+), 12854 deletions(-)
embedding :
 41 files changed, 5084 insertions(+), 5144 deletions(-)
extensions :
 44 files changed, 5632 insertions(+), 5291 deletions(-)
gfx :
 346 files changed, 40335 insertions(+), 38090 deletions(-)
hal :
 37 files changed, 1155 insertions(+), 1047 deletions(-)
image :
 50 files changed, 2833 insertions(+), 3286 deletions(-)
intl :
 154 files changed, 8871 insertions(+), 25621 deletions(-)
ipc :
 80 files changed, 3897 insertions(+), 3945 deletions(-)
js :
 410 files changed, 110276 insertions(+), 109830 deletions(-)
layout :
 345 files changed, 58555 insertions(+), 61130 deletions(-)
media :
 42 files changed, 8620 insertions(+), 8884 deletions(-)
memory :
 18 files changed, 522 insertions(+), 529 deletions(-)
mfbt :
 6 files changed, 1012 insertions(+), 856 deletions(-)
mobile :
 3 files changed, 56 insertions(+), 43 deletions(-)
modules :
 19 files changed, 5069 insertions(+), 4990 deletions(-)
mozglue :
 28 files changed, 1779 insertions(+), 1684 deletions(-)
netwerk :
 285 files changed, 75610 insertions(+), 75392 deletions(-)
parser :
 50 files changed, 8030 insertions(+), 5977 deletions(-)
python :
 1 file changed, 255 insertions(+), 267 deletions(-)
rdf :
 18 files changed, 8324 insertions(+), 8562 deletions(-)
security :
 141 files changed, 11731 insertions(+), 12176 deletions(-)
services :
 1 file changed, 142 insertions(+), 137 deletions(-)
startupcache :
 4 files changed, 300 insertions(+), 287 deletions(-)
storage :
 24 files changed, 1343 insertions(+), 1725 deletions(-)
testing :
 7 files changed, 530 insertions(+), 546 deletions(-)
third_party :
 3 files changed, 161 insertions(+), 138 deletions(-)
toolkit :
 175 files changed, 21647 insertions(+), 21184 deletions(-)
tools :
 52 files changed, 5840 insertions(+), 4788 deletions(-)
uriloader :
 31 files changed, 5633 insertions(+), 5582 deletions(-)
view :
 2 files changed, 332 insertions(+), 295 deletions(-)
widget :
 224 files changed, 64032 insertions(+), 59813 deletions(-)
xpcom :
 228 files changed, 16121 insertions(+), 15794 deletions(-)
xpfe :
 11 files changed, 2254 insertions(+), 2092 deletions(-)


So, I don't think we can apply this module by module. I think we should integrate the coding style call as part of our processes (local & mozreview).
With bug 1322321, we are changing the number of changes required:

accessible:
 134 files changed, 5985 insertions(+), 5371 deletions(-)
addon-sdk:
b2g:
 3 files changed, 120 insertions(+), 100 deletions(-)
browser:
 9 files changed, 616 insertions(+), 505 deletions(-)
build:
 7 files changed, 1851 insertions(+), 1649 deletions(-)
caps:
 8 files changed, 1527 insertions(+), 1476 deletions(-)
chrome:
 4 files changed, 471 insertions(+), 368 deletions(-)
config:
db:
devtools:
 15 files changed, 729 insertions(+), 616 deletions(-)
docshell:
 20 files changed, 1329 insertions(+), 1020 deletions(-)
dom:
 1463 files changed, 186128 insertions(+), 162749 deletions(-)
editor:
 61 files changed, 12622 insertions(+), 12103 deletions(-)
embedding:
 43 files changed, 5571 insertions(+), 5155 deletions(-)
extensions:
 44 files changed, 6193 insertions(+), 5223 deletions(-)
gfx:
 347 files changed, 46042 insertions(+), 37167 deletions(-)
gradle:
hal:
 37 files changed, 1318 insertions(+), 1037 deletions(-)
image:
 50 files changed, 3439 insertions(+), 2886 deletions(-)
intl:
 154 files changed, 10526 insertions(+), 25550 deletions(-)
ipc:
 80 files changed, 4222 insertions(+), 3785 deletions(-)
js:
 411 files changed, 111830 insertions(+), 109970 deletions(-)
layout:
 347 files changed, 71208 insertions(+), 60249 deletions(-)
media:
 42 files changed, 9876 insertions(+), 8585 deletions(-)
memory:
 18 files changed, 645 insertions(+), 557 deletions(-)
mfbt:
 6 files changed, 1036 insertions(+), 860 deletions(-)
mobile:
 3 files changed, 59 insertions(+), 44 deletions(-)
modules:
 19 files changed, 5220 insertions(+), 4974 deletions(-)
mozglue:
 28 files changed, 2191 insertions(+), 1746 deletions(-)
netwerk:
 266 files changed, 78870 insertions(+), 72343 deletions(-)
nsprpub:
obj-x86_64-pc-linux-gnu:
other-licenses:
parser:
 50 files changed, 8818 insertions(+), 5842 deletions(-)
probes:
python:
 1 file changed, 255 insertions(+), 267 deletions(-)
rdf:
 18 files changed, 8533 insertions(+), 8483 deletions(-)
release:
security:
 141 files changed, 15989 insertions(+), 12239 deletions(-)
services:
 1 file changed, 180 insertions(+), 138 deletions(-)
startupcache:
 4 files changed, 237 insertions(+), 212 deletions(-)
storage:
 24 files changed, 1558 insertions(+), 1683 deletions(-)
taskcluster:
testing:
 7 files changed, 608 insertions(+), 539 deletions(-)
third_party:
 3 files changed, 161 insertions(+), 138 deletions(-)
toolkit:
 176 files changed, 24911 insertions(+), 20828 deletions(-)
tools:
 66 files changed, 7996 insertions(+), 5705 deletions(-)
uriloader:
 31 files changed, 5924 insertions(+), 5365 deletions(-)
view:
 2 files changed, 363 insertions(+), 294 deletions(-)
widget:
 226 files changed, 72377 insertions(+), 61101 deletions(-)
xpcom:
 229 files changed, 18958 insertions(+), 15616 deletions(-)
xpfe:
 11 files changed, 2454 insertions(+), 2043 deletions(-)
accessible:
 134 files changed, 5985 insertions(+), 5371 deletions(-)
addon-sdk:
b2g:
 3 files changed, 120 insertions(+), 100 deletions(-)
browser:
 9 files changed, 616 insertions(+), 505 deletions(-)
build:
 7 files changed, 1851 insertions(+), 1649 deletions(-)
caps:
 8 files changed, 1527 insertions(+), 1476 deletions(-)
chrome:
 4 files changed, 471 insertions(+), 368 deletions(-)
config:
db:
devtools:
 15 files changed, 729 insertions(+), 616 deletions(-)
docshell:
 20 files changed, 1329 insertions(+), 1020 deletions(-)
dom:
 1463 files changed, 186128 insertions(+), 162749 deletions(-)
editor:
 61 files changed, 12622 insertions(+), 12103 deletions(-)
embedding:
 43 files changed, 5571 insertions(+), 5155 deletions(-)
extensions:
 44 files changed, 6193 insertions(+), 5223 deletions(-)
gfx:
 347 files changed, 46042 insertions(+), 37167 deletions(-)
gradle:
hal:
 37 files changed, 1318 insertions(+), 1037 deletions(-)
image:
 50 files changed, 3439 insertions(+), 2886 deletions(-)
intl:
 154 files changed, 10526 insertions(+), 25550 deletions(-)
ipc:
 80 files changed, 4222 insertions(+), 3785 deletions(-)
js:
 411 files changed, 111830 insertions(+), 109970 deletions(-)
layout:
 347 files changed, 71208 insertions(+), 60249 deletions(-)
media:
 42 files changed, 9876 insertions(+), 8585 deletions(-)
memory:
 18 files changed, 645 insertions(+), 557 deletions(-)
mfbt:
 6 files changed, 1036 insertions(+), 860 deletions(-)
mobile:
 3 files changed, 59 insertions(+), 44 deletions(-)
modules:
 19 files changed, 5220 insertions(+), 4974 deletions(-)
mozglue:
 28 files changed, 2191 insertions(+), 1746 deletions(-)
netwerk:
 266 files changed, 78870 insertions(+), 72343 deletions(-)
nsprpub:
obj-x86_64-pc-linux-gnu:
other-licenses:
parser:
 50 files changed, 8818 insertions(+), 5842 deletions(-)
probes:
python:
 1 file changed, 255 insertions(+), 267 deletions(-)
rdf:
 18 files changed, 8533 insertions(+), 8483 deletions(-)
release:
security:
 141 files changed, 15989 insertions(+), 12239 deletions(-)
services:
 1 file changed, 180 insertions(+), 138 deletions(-)
startupcache:
 4 files changed, 237 insertions(+), 212 deletions(-)
storage:
 24 files changed, 1558 insertions(+), 1683 deletions(-)
taskcluster:
testing:
 7 files changed, 608 insertions(+), 539 deletions(-)
third_party:
 3 files changed, 161 insertions(+), 138 deletions(-)
toolkit:
 176 files changed, 24911 insertions(+), 20828 deletions(-)
tools:
 66 files changed, 7996 insertions(+), 5705 deletions(-)
uriloader:
 31 files changed, 5924 insertions(+), 5365 deletions(-)
view:
 2 files changed, 363 insertions(+), 294 deletions(-)
widget:
 226 files changed, 72377 insertions(+), 61101 deletions(-)
xpcom:
 229 files changed, 18958 insertions(+), 15616 deletions(-)
xpfe:
 11 files changed, 2454 insertions(+), 2043 deletions(-)
accessible:
 134 files changed, 5985 insertions(+), 5371 deletions(-)
b2g:
 3 files changed, 120 insertions(+), 100 deletions(-)
browser:
 9 files changed, 616 insertions(+), 505 deletions(-)
build:
 7 files changed, 1851 insertions(+), 1649 deletions(-)
caps:
 8 files changed, 1527 insertions(+), 1476 deletions(-)
chrome:
 4 files changed, 471 insertions(+), 368 deletions(-)
devtools:
 15 files changed, 729 insertions(+), 616 deletions(-)
docshell:
 20 files changed, 1329 insertions(+), 1020 deletions(-)
dom:
 1463 files changed, 186128 insertions(+), 162749 deletions(-)
editor:
 61 files changed, 12622 insertions(+), 12103 deletions(-)
embedding:
 43 files changed, 5571 insertions(+), 5155 deletions(-)
extensions:
 44 files changed, 6193 insertions(+), 5223 deletions(-)
gfx:
 347 files changed, 46042 insertions(+), 37167 deletions(-)
hal:
 37 files changed, 1318 insertions(+), 1037 deletions(-)
image:
 50 files changed, 3439 insertions(+), 2886 deletions(-)
intl:
 154 files changed, 10526 insertions(+), 25550 deletions(-)
ipc:
 80 files changed, 4222 insertions(+), 3785 deletions(-)
js:
 411 files changed, 111830 insertions(+), 109970 deletions(-)
layout:
 347 files changed, 71208 insertions(+), 60249 deletions(-)
media:
 42 files changed, 9876 insertions(+), 8585 deletions(-)
memory:
 18 files changed, 645 insertions(+), 557 deletions(-)
mfbt:
 6 files changed, 1036 insertions(+), 860 deletions(-)
mobile:
 3 files changed, 59 insertions(+), 44 deletions(-)
modules:
 19 files changed, 5220 insertions(+), 4974 deletions(-)
mozglue:
 28 files changed, 2191 insertions(+), 1746 deletions(-)
netwerk:
 266 files changed, 78870 insertions(+), 72343 deletions(-)
parser:
 50 files changed, 8818 insertions(+), 5842 deletions(-)
python:
 1 file changed, 255 insertions(+), 267 deletions(-)
rdf:
 18 files changed, 8533 insertions(+), 8483 deletions(-)
security:
 141 files changed, 15989 insertions(+), 12239 deletions(-)
services:
 1 file changed, 180 insertions(+), 138 deletions(-)
startupcache:
 4 files changed, 237 insertions(+), 212 deletions(-)
storage:
 24 files changed, 1558 insertions(+), 1683 deletions(-)
testing:
 7 files changed, 608 insertions(+), 539 deletions(-)
third_party:
 3 files changed, 161 insertions(+), 138 deletions(-)
toolkit:
 176 files changed, 24911 insertions(+), 20828 deletions(-)
tools:
 66 files changed, 7996 insertions(+), 5705 deletions(-)
uriloader:
 31 files changed, 5924 insertions(+), 5365 deletions(-)
view:
 2 files changed, 363 insertions(+), 294 deletions(-)
widget:
 226 files changed, 72377 insertions(+), 61101 deletions(-)
xpcom:
 229 files changed, 18958 insertions(+), 15616 deletions(-)
xpfe:
 11 files changed, 2454 insertions(+), 2043 deletions(-)
More or less done with the following:
$ run-clang-tidy-5.0.py -j 4 -p . -checks=-*,readability-braces-around-statements -fix .
# Two bugs in the update had to be fixed
$ hg revert $(cat ./tools/rewriting/ThirdPartyPaths.txt) js/
$ clang-format-5.0 -i $(find . -iname '*.cpp' -o -iname '*.h')
$ hg revert $(cat ./tools/rewriting/ThirdPartyPaths.txt) js/ build/clang-plugin/
I revert a change (too complex with a lot of #ifdef) and fixed an issue:
https://treeherder.mozilla.org/#/jobs?repo=try&revision=5f66dfdda71a65d8d66c175b04ba252155766bd6

I reported an upstream bug on clang-tidy
For js (with bug 1342665), otherwise, it blocks check_spidermonkey_style.py
$ (cd  js/src && clang-format-5.0 -i $(find . -iname '*.cpp' -o -iname '*.h'))
Depends on: 1342665
Product: Core → Firefox Build System
Component: Source Code Analysis → Lint and Formatting
Product: Firefox Build System → Developer Infrastructure
Severity: normal → S3

Given that clang-format has been shipping for a few years now, marking this as fixed.

Status: NEW → RESOLVED
Closed: 3 months ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: