Skip to content

bump clang-format to clang-15 #8561

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Feb 11, 2025
Merged

bump clang-format to clang-15 #8561

merged 1 commit into from
Feb 11, 2025

Conversation

kroening
Copy link
Member

clang 11 was released in 2007, and the last variant of Ubuntu with the clang-format-11 package is 22.04.

This bumps the clang-format used for the formatting check to clang 15, available with 24.04.

  • Each commit message has a non-empty body, explaining why the change was made.
  • n/a Methods or procedures I have added are documented, following the guidelines provided in CODING_STANDARD.md.
  • n/a The feature or user visible behaviour I have added or modified has been documented in the User Guide in doc/cprover-manual/
  • n/a Regression or unit tests are included, or existing tests cover the modified code (in this case I have detailed which ones those are in the commit message).
  • n/a My commit message includes data points confirming performance improvements (if claimed).
  • My PR is restricted to a single feature or bugfix.
  • n/a White-space or formatting changes outside the feature-related changed lines are in commits of their own.

@kroening kroening added the CI label Jan 10, 2025
@kroening kroening marked this pull request as ready for review January 10, 2025 15:02
@kroening kroening force-pushed the bump-clang-format branch 2 times, most recently from cbf6562 to 9030c33 Compare January 10, 2025 15:37
Copy link

codecov bot commented Jan 10, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 79.60%. Comparing base (6957a04) to head (08d558d).
Report is 4 commits behind head on develop.

Additional details and impacted files
@@           Coverage Diff            @@
##           develop    #8561   +/-   ##
========================================
  Coverage    79.60%   79.60%           
========================================
  Files         1733     1733           
  Lines       197358   197358           
  Branches     18165    18165           
========================================
  Hits        157111   157111           
  Misses       40247    40247           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

apt-get install clang-format-11 # Run this on Ubuntu, Debian etc.
brew install clang-format@11 # Run this on a Mac with Homebrew installed
apt-get install clang-format-15 # Run this on Ubuntu, Debian etc.
brew install clang-format # Run this on a Mac with Homebrew installed
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not an expert on brew, but this seems like it will break in future when the default is upgraded (where as all the other places and previous code are explicit about the version).

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems Brew only has versioned variants for clang-format 8 and clang-format 11, while the latest one will install clang-format 19.

Copy link
Collaborator

@tautschnig tautschnig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's just say "clang-format 11 or later" -- our clang-format configuration file does not use options that were introduced in any version greater than 11, so this should be just fine.

apt-get install clang-format-11 # Run this on Ubuntu, Debian etc.
brew install clang-format@11 # Run this on a Mac with Homebrew installed
apt-get install clang-format-15 # Run this on Ubuntu, Debian etc.
brew install clang-format # Run this on a Mac with Homebrew installed
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems Brew only has versioned variants for clang-format 8 and clang-format 11, while the latest one will install clang-format 19.

Comment on lines 344 to 346
clang-format-15, available in the standard Ubuntu 24.04 and Homebrew
repositories. To install on a Unix-like system, try installing using the
system package manager:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
clang-format-15, available in the standard Ubuntu 24.04 and Homebrew
repositories. To install on a Unix-like system, try installing using the
system package manager:
clang-format-11 or later, available in the standard Ubuntu and Homebrew
repositories. To install on a Unix-like system, try installing using the
system package manager:

```
apt-get install clang-format-11 # Run this on Ubuntu, Debian etc.
brew install clang-format@11 # Run this on a Mac with Homebrew installed
apt-get install clang-format-15 # Run this on Ubuntu, Debian etc.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
apt-get install clang-format-15 # Run this on Ubuntu, Debian etc.
apt-get install clang-format # Run this on Ubuntu, Debian etc.

@kroening
Copy link
Member Author

Let's just say "clang-format 11 or later" -- our clang-format configuration file does not use options that were introduced in any version greater than 11, so this should be just fine.

The reason for this PR is that the BreakBeforeBraces: Allman option does not work for the { before the body of a Lambda function. With clang-format 11, no break is added, requiring manual edits whenever using a later version. To see this, apply a later version of clang-format to the round_to_integral branch.

May I hence suggest the wording "clang-format 15 or later".

@kroening kroening assigned tautschnig and unassigned kroening Feb 11, 2025
@tautschnig
Copy link
Collaborator

Let's just say "clang-format 11 or later" -- our clang-format configuration file does not use options that were introduced in any version greater than 11, so this should be just fine.

The reason for this PR is that the BreakBeforeBraces: Allman option does not work for the { before the body of a Lambda function. With clang-format 11, no break is added, requiring manual edits whenever using a later version. To see this, apply a later version of clang-format to the round_to_integral branch.

May I hence suggest the wording "clang-format 15 or later".

Ah, I wasn't aware of that difference in behaviour. Sure, let's use "clang-format 15 or later" in that case. (apt-get install clang-format installs version 18 on Ubuntu 24.04, so that would actually be fine for simplified user instructions.)

@tautschnig tautschnig assigned kroening and unassigned tautschnig Feb 11, 2025
clang 11 was released in 2007, and the last variant of Ubuntu with the
clang-format-11 package is 22.04.

This bumps the clang-format used for the formatting check to clang 15,
available with Ubuntu 22.04 and 24.04.
@kroening kroening enabled auto-merge February 11, 2025 13:16
@kroening kroening merged commit e5e5c65 into develop Feb 11, 2025
41 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants