Subzero: Use a "known" version of clang-format.

Subzero: Use a "known" version of clang-format.

There are two problems with "make format" and "make format-diff" in
Makefile.standalone:

1. You have to make sure clang-format and clang-format-diff.py are
available in $PATH.

2. Different users may have different versions installed (even for the
same user on different machines), leading to whitespace wars.  Can't we
all just get along?

Since the normal LLVM build that Subzero depends on also exposes and
builds clang-format and friends, we might as well use it.  The
clang-format binary is found in $LLVM_BIN_PATH, and clang-format-diff.py
is found relative to $LLVM_SRC_PATH.  As long as the user's LLVM build
is fairly up to date, whitespace wars are unlikely.

Given this, there's a much higher incentive to use "make format"
regularly instead of "make format-diff".  In particular, inline comments
on variable/field declaration lists can get lined up more nicely by
looking at the entire context, rather than the small diff window.

BUG= none
R=jvoung@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=dd842dbb57b825ed0dd6400648d0602b74c90aff

[Jim Stichnoth, 2015-01-27 13:46:17]

stichnot@chromium.org changed reviewers:
+ jvoung@chromium.org, kschimpf@google.com

[Jim Stichnoth, 2015-01-27 13:53:41]

The only actual change is in Makefile.standalone, but this CL also includes
running "make format", making it appear massive.

I don't think this will cause conflicts in any in-flight CLs, but if I'm wrong,
let me know which files and I'll back them out of this CL.

[jvoung (off chromium), 2015-01-27 18:08:12]

https://codereview.chromium.org/877003003/diff/20001/Makefile.standalone
File Makefile.standalone (right):

https://codereview.chromium.org/877003003/diff/20001/Makefile.standalone#newc...
Makefile.standalone:202: `find . -name "*.c" -o -name "*.cpp" -o -name "*.h"`
Would clang-format know what to do with .def files (like take the first line
comment -*- C++ -*- to guess that it is C++)?

https://codereview.chromium.org/877003003/diff/20001/crosstest/mem_intrin_mai...
File crosstest/mem_intrin_main.cpp (right):

https://codereview.chromium.org/877003003/diff/20001/crosstest/mem_intrin_mai...
crosstest/mem_intrin_main.cpp:30: do_test_fixed(memcpy_test_fixed_len)
do_test_fixed(memmove_test_fixed_len)
This is a bit odd. I wonder if having semi-colons at the end would have helped.

https://codereview.chromium.org/877003003/diff/20001/crosstest/mem_intrin_mai...
crosstest/mem_intrin_main.cpp:54: do_test_variable(memcpy_test)
do_test_variable(memmove_test)
similar

https://codereview.chromium.org/877003003/diff/20001/unittest/IceParseInstsTe...
File unittest/IceParseInstsTest.cpp (right):

https://codereview.chromium.org/877003003/diff/20001/unittest/IceParseInstsTe...
unittest/IceParseInstsTest.cpp:31: 1,                             
naclbitc::BLK_CODE_ENTER,
Hmm... these used to be arranged such that each row is a record, but now you'd
have to look for "Terminator" which can be either on the left or right columns.

[Jim Stichnoth, 2015-01-27 19:24:11]

Jan, for looking through the mind-numbing changes, you da real MVP!

I added a "make format" blacklist to preserve the careful formatting in the one
file.  Another possibility might be to parse the source files for special
comment markers and convert them to "clang-format -lines" arguments.

https://codereview.chromium.org/877003003/diff/20001/Makefile.standalone
File Makefile.standalone (right):

https://codereview.chromium.org/877003003/diff/20001/Makefile.standalone#newc...
Makefile.standalone:202: `find . -name "*.c" -o -name "*.cpp" -o -name "*.h"`
On 2015/01/27 18:08:11, jvoung wrote:
> Would clang-format know what to do with .def files (like take the first line
> comment -*- C++ -*- to guess that it is C++)?

Actually, clang-format is precisely the reason why I started using .def files
for x-macros instead of the more appropriate .h files.  They are deliberately
left out of "make format", and fortunately clang-format-diff.py chooses to
ignore them.

https://codereview.chromium.org/877003003/diff/20001/crosstest/mem_intrin_mai...
File crosstest/mem_intrin_main.cpp (right):

https://codereview.chromium.org/877003003/diff/20001/crosstest/mem_intrin_mai...
crosstest/mem_intrin_main.cpp:30: do_test_fixed(memcpy_test_fixed_len)
do_test_fixed(memmove_test_fixed_len)
On 2015/01/27 18:08:11, jvoung wrote:
> This is a bit odd. I wonder if having semi-colons at the end would have
helped.

Right - adding semicolons fixed it.

https://codereview.chromium.org/877003003/diff/20001/crosstest/mem_intrin_mai...
crosstest/mem_intrin_main.cpp:54: do_test_variable(memcpy_test)
do_test_variable(memmove_test)
On 2015/01/27 18:08:11, jvoung wrote:
> similar

Done.

https://codereview.chromium.org/877003003/diff/20001/unittest/IceParseInstsTe...
File unittest/IceParseInstsTest.cpp (right):

https://codereview.chromium.org/877003003/diff/20001/unittest/IceParseInstsTe...
unittest/IceParseInstsTest.cpp:31: 1,                             
naclbitc::BLK_CODE_ENTER,
On 2015/01/27 18:08:11, jvoung wrote:
> Hmm... these used to be arranged such that each row is a record, but now you'd
> have to look for "Terminator" which can be either on the left or right
columns.

Ugh.  Reverting the whole file change, and quarantining it.

[jvoung (off chromium), 2015-01-27 19:29:58]

lgtm

[Jim Stichnoth, 2015-01-27 20:53:57]

Committed patchset #3 (id:40001) manually as
dd842dbb57b825ed0dd6400648d0602b74c90aff (presubmit successful).