Skip to content
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

Update clang-format version #10862

Open
3 tasks
jmarrec opened this issue Dec 17, 2024 · 2 comments
Open
3 tasks

Update clang-format version #10862

jmarrec opened this issue Dec 17, 2024 · 2 comments
Assignees
Labels
Developer Issue Related to cmake, packaging, installers, or developer tooling (CI, etc)

Comments

@jmarrec
Copy link
Contributor

jmarrec commented Dec 17, 2024

Issue overview

- name: Run clang-format style check for C/C++ source code.
uses: jidicula/clang-format-action@v4.14.0
if: always()
with:
clang-format-version: '10'
check-path: 'src/EnergyPlus'

clang-format-10 is no longer installable on Ubuntu 24.04 (the current LTS) which makes it quite annoying.

I can't seem to figure out a way to tweak the rules so that clang-format-18 for eg produces the same format as clang-format-10, in particular with respect to the way the trailing comments are aligned (AlignTrailingComments: true) when they are after closing braces and I've been looking for an hour.

Here's an example from ZoneTempPredictorCorrector.cc:

image

I would recommend picking a newer clang-format version, reformatting the entire codebase in one commit, and excluding it by adding it to .git-blame-ignore-revs.

Another diff is the way parameter packs are handled and attributes

IOFiles.hh parameter packs and spacing before braces in ctor

image

EPVector.hh attributes:

image


A couple of weird things can be found in the .clang-format config file too

# Commented out lines are those which are not yet valid
# in clang-3.8 which ships with Ubuntu

Standard: Cpp11

Details

Some additional details for this issue (if relevant):

  • Platform (Operating system, version): all
  • Version of EnergyPlus (if using an intermediate build, include SHA): develop, acda84e

Checklist

Add to this list or remove from it as applicable. This is a simple templated set of guidelines.

  • Defect file added (list location of defect file here)
  • Ticket added to EnergyPlus Defect Complexity (Github Project)
  • Pull request created (the pull request will have additional tasks related to reviewing changes that fix this defect)
@jmarrec jmarrec added the Developer Issue Related to cmake, packaging, installers, or developer tooling (CI, etc) label Dec 17, 2024
@mjwitte
Copy link
Contributor

mjwitte commented Dec 17, 2024

A recent Visual Studio update has been applying these newer rules (like the comment alignment) already, so this will be helpful.

@jmarrec
Copy link
Contributor Author

jmarrec commented Dec 18, 2024

Since v17.9, MSVC 2022 ships with Clang 17, https://developercommunity.visualstudio.com/t/update-clang-format-to-latest-version/1291564#TPIN-N10630932.

With the latest MSVC, it's clang 18.

$ Load-DevTools 2022 -A x64
Found Visual Studio Community 2022 (VisualStudio/17.12.3+35527.113), latest updated on 2024-12-18
**********************************************************************
** Visual Studio 2022 Developer PowerShell v17.12.3
** Copyright (c) 2022 Microsoft Corporation
**********************************************************************
Dev environment variables set

$ clang-cl --version
clang version 18.1.8
Target: i686-pc-windows-msvc
Thread model: posix
InstalledDir: C:\Program Files\Microsoft Visual Studio\2022\Community\VC\Tools\Llvm\bin

The default, stable, branch on https://apt.llvm.org/ (for Ubuntu for eg) is 18

I would propose to update to clang-format 18 as a result. @Myoldmopar can I do that?

@jmarrec jmarrec self-assigned this Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Developer Issue Related to cmake, packaging, installers, or developer tooling (CI, etc)
Projects
None yet
Development

No branches or pull requests

2 participants