linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
To: Jason@zx2c4.com
Cc: Joe Perches <joe@perches.com>, Andrew Lunn <andrew@lunn.ch>,
	linux-kernel <linux-kernel@vger.kernel.org>
Subject: Re: list iterator spacing: clang-format vs checkpatch
Date: Mon, 8 Oct 2018 09:31:08 +0200	[thread overview]
Message-ID: <CANiq72=hXXvzfYz-1EdgDNBVfYMiRp2RbjjNF=wwiiPVU+jmuQ@mail.gmail.com> (raw)
In-Reply-To: <CAHmME9ofzanQTBD_WYBRW49d+gM77rCdh8Utdk4+PM9n_bmKwA@mail.gmail.com>

Hi Jason,

On Mon, Oct 8, 2018 at 4:01 AM Jason A. Donenfeld <Jason@zx2c4.com> wrote:
>
> Hi Joe, Miguel, others,
>
> The shiny new .clang-format file lists a number of nice iterators in
> the ForEachMacros category, the consequence being that there is a
> space between the iterator name and the opening parenthesis. This
> strikes me as the right thing to do.
>
> However, checkpatch.pl complains about it:
>
> WARNING: space prohibited between function name and open parenthesis '('
> #65: FILE: ratelimiter.c:65:
> +               hlist_for_each_entry_safe (entry, temp, &table_v4[i], hash) {
>
> It would seem that .clang-format is right and checkpatch.pl is wrong?

Checking quickly, it would seem most of the kernel does not put a
space there (a minority does), e.g.:

  git grep 'list_for_each[a-zA-Z0-9_]* (' | wc -l # 67
  git grep 'list_for_each[a-zA-Z0-9_]*(' | wc -l # 13892

So in that sense, checkpatch.pl is right (even if it is not a function call).

Now, I put the list there because otherwise clang-format would put
braces in the next line, which looks even worse.

I am not sure if there is a way to make clang-format do what we need:
  * SpaceBeforeParens does not have an option to distinguish normal
loops from macro ones.
  * SpaceBeforeRangeBasedForLoopColon does not do it (which makes
sense, but it was a nice try :-)

We would probably need to implement SpaceBeforeForEachMacros (or a new
option for SpaceBeforeParens). I haven't had the time to look into
adding the missing support for the few things that the kernel style
requires, sadly, but it is in my TODO list.

Cheers,
Miguel

  reply	other threads:[~2018-10-08  7:31 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-10-08  2:00 list iterator spacing: clang-format vs checkpatch Jason A. Donenfeld
2018-10-08  7:31 ` Miguel Ojeda [this message]
2018-10-08 15:40   ` Joe Perches
2018-10-08 16:01     ` Miguel Ojeda
2018-10-08 16:06       ` Jason A. Donenfeld
2022-01-17 12:47         ` Jason A. Donenfeld
2022-01-17 18:05           ` Joe Perches
2022-01-17 21:41             ` Miguel Ojeda
2022-01-18  4:25               ` Joe Perches
2022-01-18  9:27             ` David Laight
2022-01-17 21:41           ` Miguel Ojeda
2022-01-22 13:16             ` Miguel Ojeda
2022-04-21 23:53               ` Brian Norris
2022-04-22 10:57                 ` Miguel Ojeda

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to='CANiq72=hXXvzfYz-1EdgDNBVfYMiRp2RbjjNF=wwiiPVU+jmuQ@mail.gmail.com' \
    --to=miguel.ojeda.sandonis@gmail.com \
    --cc=Jason@zx2c4.com \
    --cc=andrew@lunn.ch \
    --cc=joe@perches.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).