Skip to content

[MAINT] Change clang-format for pre-commit#289

Open
ndgrigorian wants to merge 4 commits intomasterfrom
change-pre-commit-clang-check
Open

[MAINT] Change clang-format for pre-commit#289
ndgrigorian wants to merge 4 commits intomasterfrom
change-pre-commit-clang-check

Conversation

@ndgrigorian
Copy link
Collaborator

@ndgrigorian ndgrigorian commented Mar 12, 2026

This PR proposes

  • moving to use mirrors-clang-format for clang-format
  • adding .clang-format
  • moving to clang-format-22 from clang-format-15

Copilot AI review requested due to automatic review settings March 12, 2026 21:16
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR updates the repository’s C/C++ formatting tooling by switching the pre-commit clang-format hook to pre-commit/mirrors-clang-format, introducing a project .clang-format configuration, and reformatting existing headers accordingly.

Changes:

  • Switch pre-commit clang-format hook source and upgrade to clang-format 22.
  • Add a repository-wide .clang-format file and reformat affected headers.
  • Record the formatting commit in .git-blame-ignore-revs.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
mkl_fft/src/multi_iter.h clang-format reflow; touched iterator implementation code.
mkl_fft/src/mklfft.h clang-format reflow of public C API declarations.
.pre-commit-config.yaml Switch clang-format hook to mirrors-clang-format and bump version.
.git-blame-ignore-revs Add ignore entry for the reformatting commit.
.clang-format New formatting configuration used by clang-format/pre-commit.

You can also share your feedback on Copilot code review. Take the survey.

return 0;
else {
MultiIter_IndexElem(*mi, j) = 0;
if (!k) {
Copy link

Copilot AI Mar 12, 2026

Choose a reason for hiding this comment

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

In multi_iter_masked_next, the termination condition inside the rollover branch is if (!k), but the loop invariant is k > 0, so this branch never sets MultiIter_Done(*mi). This makes the masked iterator inconsistent with multi_iter_next (which sets done on the final rollover) and prevents the fast-path if (MultiIter_Done(*mi)) return 1; from ever triggering. Consider setting done when the last masked dimension rolls over (e.g., when k == 1).

Suggested change
if (!k) {
if (k == 1) {

Copilot uses AI. Check for mistakes.
@ndgrigorian ndgrigorian force-pushed the change-pre-commit-clang-check branch from 5b14095 to e6c3401 Compare March 12, 2026 21:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants