Received-SPF: pass (google.com: best guess record for domain of linux-kernel-owner@vger.kernel.org designates 209.132.180.67 as permitted sender) client-ip=209.132.180.67;
MIME-Version: 1.0
References: <CAHmME9ofzanQTBD_WYBRW49d+gM77rCdh8Utdk4+PM9n_bmKwA@mail.gmail.com>
In-Reply-To: <CAHmME9ofzanQTBD_WYBRW49d+gM77rCdh8Utdk4+PM9n_bmKwA@mail.gmail.com>
From:   Miguel Ojeda <miguel.ojeda.sandonis@gmail.com>
Date:   Mon, 8 Oct 2018 09:31:08 +0200
Message-ID: <CANiq72=hXXvzfYz-1EdgDNBVfYMiRp2RbjjNF=wwiiPVU+jmuQ@mail.gmail.com>
Subject: Re: list iterator spacing: clang-format vs checkpatch
To:     Jason@zx2c4.com
Cc:     Joe Perches <joe@perches.com>, Andrew Lunn <andrew@lunn.ch>,
        linux-kernel <linux-kernel@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Sender: linux-kernel-owner@vger.kernel.org
Precedence: bulk

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