This is an archive of the discontinued LLVM Phabricator instance.

[NFC] clang-format InstructionSimplify.cpp
AbandonedPublic

Authored by simoll on Jun 1 2022, 8:03 AM.

Diff Detail

Event Timeline

simoll created this revision.Jun 1 2022, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2022, 8:03 AM
Herald added a subscriber: hiraditya. · View Herald Transcript
simoll requested review of this revision.Jun 1 2022, 8:03 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 1 2022, 8:03 AM
simoll added a comment.Jun 1 2022, 8:05 AM

Making this a Diff mostly because it changes (although non-functionally) such a huge file.

I don't know how people feel nowadays about clang-formatting an entire file.

In the past, we used to only format around local changes and hope that, in time, it would converge.

The bad side of formatting everything is that git-blame needs one more level of indirection. It's not terrible, though.

I'd ask other people around how they still feel about it. I don't personally mind, but I also don't personally work on this file often.

Adding the top five people blamed for changed lines in 2022 as reviewers.. Are you okay with auto-formatting the whole file with one commit?

#lines_blamed_in_2022 Name
---------------------------------------------
                   79 Philip Reames         
                   69 Nikita Popov          
                   61 Florian Hahn          
                   37 Sanjay Patel          
                   31 Hirochika Matsumoto   
                   20 Kevin P. Neal         
                   10 Roman Lebedev         
                    9 Serge Pavlov          
                    5 Nuno Lopes
nikic accepted this revision.Jun 2 2022, 5:09 AM

Fine by me

This revision is now accepted and ready to land.Jun 2 2022, 5:09 AM
spatel accepted this revision.Jun 2 2022, 5:20 AM

LGTM - I'd go further and update the old "FunctionName" formatting too, so we are consistent with the current "functionName" style.

Many of the changed lines would already overlap, and if it's done in this patch, then that's still just this one potential commit that anyone would be looking past with 'blame'.

reames accepted this revision.Jun 2 2022, 7:27 AM

LGTM to me too.

LGTM - I'd go further and update the old "FunctionName" formatting too, so we are consistent with the current "functionName" style.

Many of the changed lines would already overlap, and if it's done in this patch, then that's still just this one potential commit that anyone would be looking past with 'blame'.

Following up on this idea in D126889

rengolin accepted this revision.Jun 2 2022, 8:54 AM
simoll abandoned this revision.Jun 9 2022, 7:50 AM

D126889 has landed, which includes clang-formatting.