serenity icon indicating copy to clipboard operation
serenity copied to clipboard

Meta: Switch to clang-format-15 as the standard formatter

Open linusg opened this issue 2 years ago • 1 comments

Haven't tested this locally as I use Fedora, which ships with LLVM 15 already, but in theory using the updated apt repository should just work.

This finally fixes the remaining west consts.

linusg avatar Oct 16 '22 22:10 linusg

Waiting for our Arch friends' distro maintainers to do the thing.

linusg avatar Oct 24 '22 23:10 linusg

Note that since clang-format-15 has requires expression and requires clause support the following workarounds can be removed:

https://github.com/SerenityOS/serenity/blob/2af028132adca1ae9fbb81a655a9ce00420d8ad4/AK/StdLibExtras.h#L31-L34 https://github.com/SerenityOS/serenity/blob/8a5d2be617a82c93cfafa6c714dff67ab5ff0f7b/AK/NonnullRefPtr.h#L203-L206 https://github.com/SerenityOS/serenity/blob/998c1152efa116da5fabf89363d178c8fb729d77/Kernel/Library/NonnullLockRefPtr.h#L213-L216 https://github.com/SerenityOS/serenity/blob/b2164ad979ef3dd0761d833dcc4f26154bf40b29/Meta/Lagom/Tools/CodeGenerators/LibUnicode/GeneratorUtil.h#L184-L189

Additionally, https://github.com/llvm/llvm-project/issues/56602 landed in 15 so the following can be removed:

https://github.com/SerenityOS/serenity/blob/849499988e3db08dea73f2444d91936710b91c6f/AK/Platform.h#L71-L72

Additionally, this can also be removed: https://github.com/SerenityOS/serenity/blob/660d2b53b1206e868d5470eee80b5e62d7e30da7/Userland/Libraries/LibRegex/RegexParser.cpp#L2543-L2545 (I think that was fixed a couple versions ago actually)

Regarding this callout: https://github.com/SerenityOS/serenity/blob/7398fd107c8e68d19f758d7346c1ea13c31b4826/AK/Concepts.h#L54

clang-format now has proper support for requires-clauses, but I don't think it can be configured to use a style like shown in that file (because i've never seen such style before). And there are a bunch of bugs with requires-clauses specifically that I've fixed but will only land in clang-format-16, so that can wait

rymiel avatar Nov 21 '22 19:11 rymiel

I said this before and I will say this again (as an Arch user) - please don't wait for the Arch people to get around this as this can take literally months before they will package-and-ship llvm-15. This is a sad situation for how Arch handles llvm updates, but we should not care about how they package or when they package their stuff. What I will kindly ask is to provide a way for me to easily compile llvm-15 with clang-format (so me and other users can easily do that) and use that for Serenity. I am sure it's not an hard thing to do - we already compile clang for whoever that wants it, so I am sure generating clang-format-15 can be done without a problem.

supercomputer7 avatar Nov 27 '22 07:11 supercomputer7

I looked in the Toolchain directory I had locally since I compiled clang about a month ago and I see that there's a clang-format binary already there, waiting to be used. Now I am convinced we should never consider how or when Linux distros ship their llvm packages - if we simply can compile and use the latest, then everything is really OK. At least this is my opinion on the matter. Also, please note that there's already llvm with version 16, so maybe we should use that instead.

supercomputer7 avatar Nov 27 '22 07:11 supercomputer7

Yep! clang-format is directly part of the clang module, not as a separate project under i.e. clang-tools-extra (which seems to be built anyway, so doesn't matter)

Also, please note that there's already llvm with version 16, so maybe we should use that instead.

Also also note that 16 is currently the version of the main branch, and not a released version. It will initially release at about March of next year, if my time-keeping is correct. If that isn't a problem, though, 16 already has a bunch of clang-format fixes in it

Living on the bleeding edge also has the cost of compiling, which for LLVM is very very high

rymiel avatar Nov 27 '22 07:11 rymiel

@supercomputer7 alright, rebased & will merge once CI is green. :)

@rymiel thanks for pointing all of these out! I added another commit removing the now redundant clang-format off comments.

linusg avatar Nov 27 '22 11:11 linusg