Page MenuHomePhabricator

[cpu-detection] Run clang-format over the Host CPU detection code.
AcceptedPublic

Authored by asbirlea on Jun 2 2016, 11:00 AM.

Details

Summary

This patch is in preparation for a substantial refactoring of the code. No functionality changed.
To make the diffs easier to read, I'm planning to first get everything using clang-format, then get the naming conventions consistent, and finally post the more substantial refactoring patch.

Diff Detail

Event Timeline

asbirlea updated this revision to Diff 59427.Jun 2 2016, 11:00 AM
asbirlea retitled this revision from to [cpu-detection] Run clang-format over the Host CPU detection code..
asbirlea updated this object.
echristo accepted this revision.Jun 2 2016, 1:54 PM
echristo edited edge metadata.

LGTM.

Thanks!

-eric

This revision is now accepted and ready to land.Jun 2 2016, 1:54 PM
craig.topper added inline comments.Jun 2 2016, 2:43 PM
lib/Support/Host.cpp
194

I liked the alignment with the line above.

224

I liked those equal signs being aligned.

236

This looks odd. Why did clang-format decide on a new break here?

789

I don't really like the = signs becoming unaligned here and in the later code. They emphasized the similarity of the code.

896

Why did clang format collapse this enum? That seems odd.

echristo added inline comments.Jun 2 2016, 2:45 PM
lib/Support/Host.cpp
789

FWIW I sortof agree, that said, this code is going to get a little nicer here shortly and the formatting is just to make it easier to review :)

896

I've gone ahead and committed this already under the "clang-format is always ok" rule, but I'd love to see bugs filed against clang-format if you don't agree with it :)

asbirlea added inline comments.Jun 2 2016, 2:56 PM
lib/Support/Host.cpp
789

Honestly, I agree as well. I'm open to realign the = signs after the reformatting patch, assuming it's preferable to clang format.

probinson added inline comments.
lib/Support/Host.cpp
236

Probably it prefers to break after '=' rather than '&&' given that the entire RHS will still fit in 80 columns.