This is an archive of the discontinued LLVM Phabricator instance.

[llvm-readobj][nfc] Run clang-format on the directory's .cpp and .h files
Needs ReviewPublic

Authored by oontvoo on Mar 31 2022, 11:14 AM.

Details

Reviewers
jhenderson
Summary

Motivation: Prevent unrelated formatting changes on lines that are current il-formatted.

Diff was generated with:

clang-format -i --style=llvm llvm/tools/llvm-readobj/*.h
clang-format -i --style=llvm llvm/tools/llvm-readobj/*.cpp

(With the exception of 'ARMEHABIPrinter.h', which had a 81'th char
on the first line, hence needed manual edit).

If there is any part that should not have been formatted, please let me know - can put clang-format off around it.

Diff Detail

Event Timeline

oontvoo created this revision.Mar 31 2022, 11:14 AM
Herald added a project: Restricted Project. · View Herald Transcript
oontvoo requested review of this revision.Mar 31 2022, 11:15 AM
jhenderson added inline comments.Mar 31 2022, 10:31 PM
llvm/tools/llvm-readobj/ARMEHABIPrinter.h
1

Perhaps while you're at it put the C++ marker here.

llvm/tools/llvm-readobj/COFFDumper.cpp
365

Not that I'm suggesting you change what clang-format has done here, but I find it odd that the closing brace is on the same line as an entry, but the opening one isn't. Perhaps worth asking a clang-format person about,

1765–1772

There's an active discussion on discourse about whether this style should be allowed. I'm not sure we should change it at this time, although preventing clang-format doing anything to it seems wrong too.

https://discourse.llvm.org/t/enable-single-line-case-statements-in-llvm-clang-format-style/61062

I've asked there for thoughts.

llvm/tools/llvm-readobj/ELFDumper.cpp
83

Looks like clang-format doesn't agree with itself?

988–990

Here and throughout these tables, I think the existing style is more readable, so possibly decorate them (though do remove the extra space in this column for example).

If this style isn't available as a clang-format option, please make a feature request. If it is, I'd start a Discourse thread on enabling it, citing this code as an example.

2088

Another linter complaint.

llvm/tools/llvm-readobj/WasmDumper.cpp
25

And more here.

llvm/tools/llvm-readobj/XCOFFDumper.cpp
141

And more (multiple times in this file).

llvm/tools/llvm-readobj/llvm-readobj.h
47

Ditto.

mehdi_amini added inline comments.
llvm/tools/llvm-readobj/ELFDumper.cpp
83

The check is supposed to be using the most recent release of clang-format (not HEAD)