(Re)landing InsertBraces to our .clang-format

322 views
Skip to first unread message

Peter Boström

unread,
Dec 13, 2022, 1:59:26 PM12/13/22
to Chromium-dev
Hi!

Over at cxx@ we've discussed adding {}s unconditionally to if statements, one of the arguments being that {}s can be inserted by git cl format.

Historically the Google style guide has preferred braceless ifs, but have now transitioned over to preferring braces even to short if statements, with historical exceptions for braceless ifs that may be decided on a per-project basis. We've decided to not make use of that historical exception as clang-format as a tool can help us be consistent and spend less time considering these in code review.

The first land attempt as crrev.com/c/4048911 got reverted as crrev.com/c/4076867 two hours later, because a bug in upstream clang-format when formatting partial files. We believe this has been resolved upstream, since October, as https://github.com/llvm/llvm-project/issues/58161 which has been rolled into Chromium. I've tried to break control flow by modifying parts of braceless if-else statements that are already checked into chromium, and have been unable to do so. I will now attempt a reland as crrev.com/c/4097043, but if you see git cl format break control flow, please revert this reland immediately and file another crbug.com.

Thanks folks,
Peter

Reilly Grant

unread,
Dec 14, 2022, 1:08:24 PM12/14/22
to pb...@chromium.org, Chromium-dev
Hurrah! 🎉
Reilly Grant | Software Engineer | rei...@chromium.org | Google Chrome


--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.
To view this discussion on the web visit https://groups.google.com/a/chromium.org/d/msgid/chromium-dev/CAGFX3sGRKJEKpfN5zDLDAuifqEzjQn7mEZurGDXCBgpBx%2BeCdw%40mail.gmail.com.

Alex Newcomer

unread,
Dec 15, 2022, 4:01:00 PM12/15/22
to Chromium-dev, Reilly Grant, Chromium-dev, Peter Boström
Love it, thank you for doing this!

To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev+unsubscribe@chromium.org.

Fergal Daly

unread,
Dec 16, 2022, 3:18:06 AM12/16/22
to pb...@chromium.org, Chromium-dev
Thanks for doing this (I also favour {} for ease of adding printfs).

Do bugs go under Tools>LLVM or somewhere else?

F
 

Thanks folks,
Peter

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev
---
You received this message because you are subscribed to the Google Groups "Chromium-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email to chromium-dev...@chromium.org.

Peter Boström

unread,
Dec 16, 2022, 12:52:20 PM12/16/22
to Fergal Daly, Chromium-dev
Tools>LLVM should be good. Please cc me as well.

Adding spaces to any if()s that did not have braces is sufficient to trigger the formatter for those lines, so keep that in mind if it looks like you're getting unrelated code formatted. I've also heard that there are also parts of the code that format the full file on git cl format, those will start changing unrelated code on a clang-format roll or configuration changes. In this case we did both. :)

Thanks,
Peter

Brian Begnoche

unread,
Dec 19, 2022, 1:41:10 PM12/19/22
to Chromium-dev, pb...@chromium.org, Chromium-dev, Fergal Daly
I think I'm running into one of those parts of the code that triggers the full file format. It's some combination of git rebase with resolving merge conflicts in VSCode.

https://crbug.com/1402278
Reply all
Reply to author
Forward
0 new messages