Skip to content

WIP: update clang-format for Zephyr #27058

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

Closed
wants to merge 4 commits into from

Conversation

pabigot
Copy link
Collaborator

@pabigot pabigot commented Jul 22, 2020

The goal of #21392 to be able to reduce arguments about style by establishing a baseline policy: if the file content is left unmodified by clang-format, the style is acceptable. Released versions of clang-format lack a configuration option required by Zephyr's style.

clang-format-12 (expected for release this fall) eliminates this gap by adding an option for SpaceBeforeParens that does not introduce a space in FOR_EACH control macro expressions like:

SYS_SLIST_FOR_EACH_CONTAINER_SAFE(&dlci_active_t1_timers, dlci, next, node) { ... }

This branch updates the in-tree .clang-format, which tracks the Linux one, to:

  • enable the options that were disabled because they were unavailable to clang-format-4 or clang-format-5
  • add Zephyr-specific options not yet upstream (ATT only SpaceBeforeParens: ControlStatementsExceptForEachMacros
  • additional updates to remain in sync with Linux

If you are interested in evaluating clang-format as an alternative to uncrustify please try the version in this branch, and propose changes to the configuration, so when clang-12 is released we're able to make progress on #21392.

@pabigot
Copy link
Collaborator Author

pabigot commented Jul 22, 2020

git-clang-format is intended to support checking conformance of changes. I'll be evaluating this over the next couple months.

@galak
Copy link
Collaborator

galak commented Jul 22, 2020

Do you know if there is ubuntu package or pre-built binary for the clang-format v12?

@pabigot
Copy link
Collaborator Author

pabigot commented Jul 22, 2020

Do you know if there is ubuntu package or pre-built binary for the clang-format v12?

Probably not, since 12 is the next release of the entire system. I did something like the following.

git clone https://github.com/llvm/llvm-project.git
cd llvm-project
mkdir build && cd build
TAG=$(git describe --tags)-$(date +%Y%m%d)
cmake -G Ninja \
  -DLLVM_ENABLE_PROJECTS="clang;libcxx;libcxxabi" \
  -DCMAKE_INSTALL_PREFIX=/usr/local/llvm-${TAG} \
  -DCMAKE_BUILD_TYPE=Release \
  ../llvm \
  2>&1 | tee cmo
ninja
ninja install
cd /usr/local
rm -f llvm
ln -s llvm-${TAG} llvm
cd bin
ln -s ../llvm/bin/clang-format

pabigot and others added 4 commits September 5, 2020 09:06
A large number of options are disabled because they are marked as
unknown to older versions of clang-format.  We're going to need
clang-format-12 to meet the style guide, so enable them.

Signed-off-by: Peter Bigot <[email protected]>
NamespaceIndentation has been removed upstream as it impedes
readability, and only one built-in style (WebKit) selects it.

Signed-off-by: Peter Bigot <[email protected]>
Identified FOR_EACH macros are an exception to the requirement that
there be a space between a control keyword and its parenthesized
control expressions.

Signed-off-by: Peter Bigot <[email protected]>
More have been added.

Signed-off-by: Peter A. Bigot <[email protected]>
@chrta
Copy link
Collaborator

chrta commented Oct 6, 2020

Do you know if there is ubuntu package or pre-built binary for the clang-format v12?

It is available in the llvm apt repo now, e.g. for focal via deb http://apt.llvm.org/focal/ llvm-toolchain-focal main, see https://apt.llvm.org/

Edit: I commited the changes after running clang-format-12 on the current master, see lemonbeat@0d578b6

chrta added a commit to lemonbeat/zephyr that referenced this pull request Oct 6, 2020
Unchanged file from zephyrproject-rtos#27058
rebased on current master

find . -iname *.h -o -iname *.cpp -o -iname *.c | xargs clang-format-12 -style=file -i
@github-actions
Copy link

github-actions bot commented Dec 6, 2020

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Dec 6, 2020
@github-actions github-actions bot closed this Dec 20, 2020
@galak galak reopened this Mar 11, 2021
@galak galak removed the Stale label Mar 11, 2021
@pabigot
Copy link
Collaborator Author

pabigot commented Mar 15, 2021

I have no plans to do more with this, so whoever wants to take it on is free to do so.

@github-actions
Copy link

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jul 14, 2021
@github-actions github-actions bot closed this Jul 29, 2021
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