Subzero: Add "make format-diff" target.

Subzero: Add "make format-diff" target.

This invokes clang-format-diff.py so you can easily reformat just
the code you touched.

(Caution, this may not apply to new files.)

BUG= none
R=jvoung@chromium.org

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

[Jim Stichnoth, 2014-07-07 22:04:59]

PTAL, especially that /usr/lib/clang-format/clang-format-diff.py exists in all
your dev environments and that clang-format is present in your $PATH.  If not,
let me know and maybe the makefile can somehow be generalized.

[jvoung (off chromium), 2014-07-08 23:56:25]

Thanks -- this will at least work on my workstation, but not necessarily my
laptop. Not sure if that's the case for you too.

https://codereview.chromium.org/372133002/diff/1/Makefile.standalone
File Makefile.standalone (right):

https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode81
Makefile.standalone:81: src/Ice*.h src/Ice*.cpp src/llvm2ice.cpp
btw, I think the reason Karl missed clang-formatting PNaClTranslator.cpp is
because it's missing from this list.

https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode88
Makefile.standalone:88: /usr/lib/clang-format/clang-format-diff.py -p1
-style=LLVM -i
Not sure of a better way... my desktop definitely has this, but my laptop does
not. There is a clang-format-diff.py script in the CLANG_SRC_PATH, but I think
it assumes the user has "clang-format" in the PATH.

[Jim Stichnoth, 2014-07-09 16:10:59]

On 2014/07/08 23:56:25, jvoung wrote:
> Thanks -- this will at least work on my workstation, but not necessarily my
> laptop. Not sure if that's the case for you too.

That is also true for me (not working out of the box on my laptop).  Since this
is only used as a convenience, and not as a presubmit check or other
requirement, I was thinking it might be OK to ask the user to put the
appropriate symlinks in their $PATH as necessary to make it work (and state that
in a Makefile comment).  Does that sound reasonable?

> 
> https://codereview.chromium.org/372133002/diff/1/Makefile.standalone
> File Makefile.standalone (right):
> 
> https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode81
> Makefile.standalone:81: src/Ice*.h src/Ice*.cpp src/llvm2ice.cpp
> btw, I think the reason Karl missed clang-formatting PNaClTranslator.cpp is
> because it's missing from this list.

Good point.  I think src/*.{h,cpp} should be fine.  It wasn't that way
originally because some files were pulled in from PNaCl - but have since been
removed.

> 
> https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode88
> Makefile.standalone:88: /usr/lib/clang-format/clang-format-diff.py -p1
> -style=LLVM -i
> Not sure of a better way... my desktop definitely has this, but my laptop does
> not. There is a clang-format-diff.py script in the CLANG_SRC_PATH, but I think
> it assumes the user has "clang-format" in the PATH.

[jvoung (off chromium), 2014-07-09 16:23:02]

On 2014/07/09 16:10:59, stichnot wrote:
> On 2014/07/08 23:56:25, jvoung wrote:
> > Thanks -- this will at least work on my workstation, but not necessarily my
> > laptop. Not sure if that's the case for you too.
> 
> That is also true for me (not working out of the box on my laptop).  Since
this
> is only used as a convenience, and not as a presubmit check or other
> requirement, I was thinking it might be OK to ask the user to put the
> appropriate symlinks in their $PATH as necessary to make it work (and state
that
> in a Makefile comment).  Does that sound reasonable?

Yeah, I'm fine with it working out of the box on desktop, but w/ a bit of work
on the laptop.

The other thing is, I don't know how different the /usr/bin/clang-format is from
the LLVM_BIN_PATH one. (this /usr/lib/clang-format/clang-format-diff.py
references /usr/bin/clang-format)

Otherwise LGTM.

> 
> > 
> > https://codereview.chromium.org/372133002/diff/1/Makefile.standalone
> > File Makefile.standalone (right):
> > 
> >
https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode81
> > Makefile.standalone:81: src/Ice*.h src/Ice*.cpp src/llvm2ice.cpp
> > btw, I think the reason Karl missed clang-formatting PNaClTranslator.cpp is
> > because it's missing from this list.
> 
> Good point.  I think src/*.{h,cpp} should be fine.  It wasn't that way
> originally because some files were pulled in from PNaCl - but have since been
> removed.
> 
> > 
> >
https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode88
> > Makefile.standalone:88: /usr/lib/clang-format/clang-format-diff.py -p1
> > -style=LLVM -i
> > Not sure of a better way... my desktop definitely has this, but my laptop
does
> > not. There is a clang-format-diff.py script in the CLANG_SRC_PATH, but I
think
> > it assumes the user has "clang-format" in the PATH.

[Jim Stichnoth, 2014-07-09 23:39:27]

On 2014/07/09 16:23:02, jvoung wrote:
> On 2014/07/09 16:10:59, stichnot wrote:
> > On 2014/07/08 23:56:25, jvoung wrote:
> > > Thanks -- this will at least work on my workstation, but not necessarily
my
> > > laptop. Not sure if that's the case for you too.
> > 
> > That is also true for me (not working out of the box on my laptop).  Since
> this
> > is only used as a convenience, and not as a presubmit check or other
> > requirement, I was thinking it might be OK to ask the user to put the
> > appropriate symlinks in their $PATH as necessary to make it work (and state
> that
> > in a Makefile comment).  Does that sound reasonable?
> 
> Yeah, I'm fine with it working out of the box on desktop, but w/ a bit of work
> on the laptop.
> 
> The other thing is, I don't know how different the /usr/bin/clang-format is
from
> the LLVM_BIN_PATH one. (this /usr/lib/clang-format/clang-format-diff.py
> references /usr/bin/clang-format)
> 
> Otherwise LGTM.
> 
> > 
> > > 
> > > https://codereview.chromium.org/372133002/diff/1/Makefile.standalone
> > > File Makefile.standalone (right):
> > > 
> > >
> https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode81
> > > Makefile.standalone:81: src/Ice*.h src/Ice*.cpp src/llvm2ice.cpp
> > > btw, I think the reason Karl missed clang-formatting PNaClTranslator.cpp
is
> > > because it's missing from this list.
> > 
> > Good point.  I think src/*.{h,cpp} should be fine.  It wasn't that way
> > originally because some files were pulled in from PNaCl - but have since
been
> > removed.
> > 
> > > 
> > >
> https://codereview.chromium.org/372133002/diff/1/Makefile.standalone#newcode88
> > > Makefile.standalone:88: /usr/lib/clang-format/clang-format-diff.py -p1
> > > -style=LLVM -i
> > > Not sure of a better way... my desktop definitely has this, but my laptop
> does
> > > not. There is a clang-format-diff.py script in the CLANG_SRC_PATH, but I
> think
> > > it assumes the user has "clang-format" in the PATH.

PTAL.

I fixed "make format" to reformat all .h and .cpp files in the src/ directory,
not just the Ice* files.

I also changed it to use the system-installed version of clang-format.  I'm not
sure I like all of its choices compared to the more recent versions of
clang-format, but at least it's likely to be much more consistent across our
workstations.

[jvoung (off chromium), 2014-07-09 23:50:37]

LGTM

[Jim Stichnoth, 2014-07-09 23:53:43]

Committed patchset #2 manually as r240e0f8 (presubmit successful).