[GitHub][workflows] Use latest clang-format version 18.1.1

[owenca]

No description provided.

[llvmbot]

@llvm/pr-subscribers-github-workflow

Author: Owen Pan (owenca)

Changes

---

Full diff: https://github.com/llvm/llvm-project/pull/85502.diff

1 Files Affected:

• (modified) .github/workflows/pr-code-format.yml (+1-1)

diff --git a/.github/workflows/pr-code-format.yml b/.github/workflows/pr-code-format.yml
index 3d0c23917bd403..1d1fa2483b658f 100644
--- a/.github/workflows/pr-code-format.yml
+++ b/.github/workflows/pr-code-format.yml
@@ -53,7 +53,7 @@ jobs:
       - name: Install clang-format
         uses: aminya/setup-cpp@v1
         with:
-          clangformat: 17.0.1
+          clangformat: 18.1.1
 
       - name: Setup Python env
         uses: actions/setup-python@v4

[boomanaiden154]

I've reverted this for now as I think it needs more discussion. https://discourse.llvm.org/t/rfc-clang-format-all-the-things/76614 is probably the most recent discussion on this, and the consensus there seems to be that we should be conservative with version upgrades due to churn.

Unless there's a specific bug fix/feature that landed in clang-format that is important for in-tree use, I think we should hold off on this for now until we have a better specified project-wide policy on what to do with bumping the formatter version. If there's a compelling reason to go forward with this though, I'm not opposed to relanding. I think upgrading just to upgrade doesn't make a lot of sense currently though given the lack of project-wide policy/discussion on this topic.

[poyaoc97]

You should probably revert 5ac784d also then.

[owenca]

I've relanded it.

I've reverted this for now as I think it needs more discussion. https://discourse.llvm.org/t/rfc-clang-format-all-the-things/76614 is probably the most recent discussion on this, and the consensus there seems to be that we should be conservative with version upgrades due to churn.

That RFC is about formatting entire codebases, and for me there is no consensus yet even though it seems more people favor at least the latest release of clang-format when formatting entire files.

The more relevant RFC is https://discourse.llvm.org/t/rfc-clang-format-github-action/73391. It seems the consensus is to use at least the latest release of clang-format for CI.

Unless there's a specific bug fix/feature that landed in clang-format that is important for in-tree use, I think we should hold off on this for now until we have a better specified project-wide policy on what to do with bumping the formatter version. If there's a compelling reason to go forward with this though, I'm not opposed to relanding. I think upgrading just to upgrade doesn't make a lot of sense currently though given the lack of project-wide policy/discussion on this topic.

There have been many bug fixes and a number of new options/features since clang-format 17.0.1. One of the new features is the support of .clang-format-ignore files, which can be used to ignore .td files mentioned in the RFC you linked above. There are bug fixes that fixed formatting errors in Polly. We also have a new clang-format style (from last November) that we can't really use unless the CI is bumped up to clang-format 18.1.1.

[tru]

Yeah I am pretty sure we have consensus to use the latest version of clang-format for the formatter in the pre-commit. I totally agree we can't move forward with any site wide formatting.

[boomanaiden154]

That RFC is about formatting entire codebases, and for me there is no consensus yet even though it seems more people favor at least the latest release of clang-format when formatting entire files.

Right. I'm aware of that. There was some discussion on holding back the version due to churn there, and there seemed to be a consensus on that point. Looking more closely at it, the point was made more in the context of after side-wide reformatting rather than the current state, like I originally though.

The more relevant RFC is https://discourse.llvm.org/t/rfc-clang-format-github-action/73391. It seems the consensus is to use at least the latest release of clang-format for CI.

Yep. That does seem to be the current consensus and hasn't been adjusted at all like I originally though.

There have been many bug fixes and a number of new options/features since clang-format 17.0.1. One of the new features is the support of .clang-format-ignore files, which can be used to ignore .td files mentioned in the RFC you linked above. There are bug fixes that fixed formatting errors in Polly. We also have a new clang-format style (from last November) that we can't really use unless the CI is bumped up to clang-format 18.1.1.

It sounds like it will be nice to take advantage of these features. Is it possible to include a short summary in the commit message next time?

The revert was a little bit overzealous on my part. Sorry about that. I still think we need to establish better policy on what exactly we want to do with tooling versioning, but that can be done elsewhere. I might try and see if I can start the process of codifying a policy somewhere this week.

[owenca]

Is it possible to include a short summary in the commit message next time?

Because I remembered that using the latest release of clang-format for the github workflow had already been decided, I didn't feel that any reason or justification was needed (other than 18.1.1 had been released for more than a week). Otherwise, I would have added an explanation in the commit message.