[cfe-dev] Repeated clang-format'ting keeps changing code / use of clang-format in CI

[Nico Weber via cfe-dev]

Thanks!

Here's a (not minimal) reduction of the first issue; it looks like the
other issues are basically the same thing:

$ cat fmt.c
void cdataSectionTok()
{
  switch (1) {
#define LEAD_CASE(n) \
  case BT_LEAD ## n: \
    break
  LEAD_CASE(2) LEAD_CASE(3) LEAD_CASE(4)
#undef LEAD_CASE
  default:
    break;
  }
}

$ out/gn/bin/clang-format fmt.c
void cdataSectionTok() {
  switch (1) {
#define LEAD_CASE(n)
    \
  case BT_LEAD##n:
    \
    break
    LEAD_CASE(2)
    LEAD_CASE(3) LEAD_CASE(4)
#undef LEAD_CASE
        default : break;
  }
}

As you can see by the weird "default : break" at the end, clang-format gets
confused about the whole switch statement. It looks like the macros confuse
it. Just adding semicolons after it is sufficient to unconfuse it:

$ cat fmt.c
void cdataSectionTok()
{
  switch (1) {
#define LEAD_CASE(n) \
  case BT_LEAD ## n: \
    break
  LEAD_CASE(2); LEAD_CASE(3); LEAD_CASE(4);
#undef LEAD_CASE
  default:
    break;
  }
}

$ out/gn/bin/clang-format fmt.c
void cdataSectionTok() {
  switch (1) {
#define LEAD_CASE(n)
    \
  case BT_LEAD##n:
    \
    break
    LEAD_CASE(2);
    LEAD_CASE(3);
    LEAD_CASE(4);
#undef LEAD_CASE
  default:
    break;
  }
}

If you're able to change expat's source, this might be a good approach.

Else, you can explicitly tell clang-format the name of macros that should
be handled as statements in your .clang-format file like so:

StatementMacros: ['LEAD_CASE']

That helps with LEAD_CASE. (clang-format should still converge within one
round, but it's going to converge to something bad without either of the
two things I suggest, so it wouldn't help you much).

Like Raphaël's mail says, XCS is slightly different because it's not a
statement. clang-format has special logic for formatting sequences of
string literals, and that doesn't fire if each literal is surrounded by a
macro.

You can fix this by doing `XCS("foo" "bar")` (with a linebreak in between
foo and bar) instead of `XCS("foo") XCS("bar")`. Semantically they do the
same thing as far as I can tell, but clang-format does much better with the
first form. Having done this transformation locally, this is arguably
easier to read for humans too.

There isn't a .clang-format setting for this case as far as I know.

(Again, clang-format should reach a fixed point in one hop, but again the
one hop gives you pretty bad formatting so fixing that bug wouldn't help
you all that much.)

With these two changes, clang-format does reach an end state in one step,
and its output (for the two cases I looked at) looks good too.

Hope this helps!

On Fri, Jul 26, 2019 at 5:03 PM Sebastian Pipping <sebastian at pipping.org>
wrote:

> Hi Nico,
>
>
> that's great to hear.  I have attached a script to reproduce the issue
> from public code of libexpat 2.2.7 as well as the second iteration diff
> I get from clang-format version 10.0.0
> (/var/tmp/portage/sys-devel/clang-10.0.0.9999/work/x/y/clang-10.0.0.9999
> c0048be7ff340ebba3092e95d82147bc9928b909) of Gentoo.
>
> Best
>
>
>
> Sebastian
>
>
> On 26.07.19 22:07, Nico Weber wrote:
> > clang-cl is supposed to reach a fixpoint in one iteration. If it
> > doesn't, that's a bug. If you can, please post a reduced repro case :)
> >
> > On Thu, Jul 25, 2019 at 5:38 PM Sebastian Pipping via cfe-dev
> > <cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>> wrote:
> >
> >     Hi!
> >
> >
> >     I'm trying to integrate clang-format version 9 with CI in a way that
> the
> >     CI run only passes if applying clang-format yields the exact same
> >     code, i.e. "git diff --exit-code" returns 0.  The idea is that every
> >     pull request would only pass if clang-format
> >
> >     When trying to put that approach to action with libexpat [1]
> >     I noticed that multiple runs to clang-format do not seem to produce
> the
> >     same code, at least not with the two version of Clang 9 that I tested
> >     [2], and at least not with libexpat code.
> >
> >     I wonder if that's a known problem, if it's fixed in later versions
> >     of clang-format, if you are aware of workarounds, or if I just need
> >     to say goodbye to combining clang-format and CI for stable style
> >     checking.
> >
> >     Thanks in advance!
> >
> >     Best
> >
> >
> >
> >     Sebastian
> >
> >
> >     [1] https://github.com/libexpat/libexpat/pull/293
> >     [2] 9.0.0.9999 commit 28c954cf961a846789a972a8ed179b7108244ae7
> >     _______________________________________________
> >     cfe-dev mailing list
> >     cfe-dev at lists.llvm.org <mailto:cfe-dev at lists.llvm.org>
> >     https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-dev
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20190729/aafc4ac0/attachment.html>