[flang-dev] Which version of clang-format to use?

[Michael Kruse via flang-dev]

Am Mo., 17. Mai 2021 um 11:19 Uhr schrieb Diana Picus <diana.picus at linaro.org>:
> I think it is actually sensible to use the latest released version (so in this case 12, as 13 is not out yet), otherwise whenever you rebase a patch that's in review you also need to rebuild clang-format locally (instead of just flang + tests). In addition, there's no connection in Phabricator between a patch and the commit it is based on, so it's not clear what the in-tree version is. If you mean ToT, then that can change while a patch is in review. Think about what it would mean for a flang patch if a clang-format patch is committed, then reverted for bug-fixes, then committed again, all while the flang patch is still in review. Does that mean the pre-merge checks should be run 3 times, potentially with different results each time? That would be both confusing and very resource intensive. On the other hand, if the pre-merge checks are only run once, when you first upload the patch, then you'd essentially have non-deterministic formatting, depending on what happened to be in tree at that exact moment. Using the release binaries gives much more stability.

As you mentioned, a new version of clang can be released during an
ongoing review as well. But I can expect that patches are rebased to
ToT, i.e. the matching version of clang-format will be available
anyway. If using the latest release, I am effectively forced to also
have compiled clang around that matches the pre-merge bot's version.

I wouldn't call calling "git clang-format origin/main" once before
uploading a new patch labour-intensive. You should do this anyway.
Arcanist is even configured to use clang-format before updating a
patch.


> There may be some delay between when the release binaries become available and when the pre-merge bots are updated, but that's probably something that can be mitigated.

This would have to be discussed with the maintainer which I would not
impose additional work on. It is an event occuring only twice a year,
it will be open whether it actually works until the next release and
thus I doubt it is worth automating. Not only the pre-merge bots are
affected, but every single developer as well.

Michael