Skip to content

Update clang-format include order rules #38

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 3 commits into from
Jan 24, 2025
Merged

Update clang-format include order rules #38

merged 3 commits into from
Jan 24, 2025

Conversation

DuncanMcBain
Copy link
Member

We should be including the sycl.hpp header from the sycl/ folder.

We should be including the sycl.hpp header from the sycl/ folder.
Changing from CL/sycl to sycl/sycl started forcing some header file
reordering, which caused pre-commit failures in the CI. So I have
changed the clang-format rules to better express what we want, which
has then cause other formatting changes for files that were not already
compliant to my dread will.

* all "headers.hpp"
* <third_party/includes.h>
* <sycl/sycl.hpp>
* <cppstdlibheaders>

Other than that, changing the matmul header caused the `cl::` namespace
to disappear, so was removed. Other formatting in the tuple_utils header
started becoming a bit nasty so knowing that SYCL requires C++ 17, I
changed to `decltype(auto)` for some of the function return types.
@DuncanMcBain DuncanMcBain requested a review from rafbiels January 22, 2025 19:07
Copy link
Collaborator

@rafbiels rafbiels left a comment

Choose a reason for hiding this comment

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

Nice one, thanks! I like the new .clang-format configuration as well - this was something I was thinking of having but never spent the time to figure out. Could you update the PR title and description to reflect also that change?

@DuncanMcBain DuncanMcBain changed the title Fix old include style causing warnings Update clang-format include order rules Jan 24, 2025
@DuncanMcBain
Copy link
Member Author

What I'll actually do is rewrite the commit message when I squash and merge just now

@DuncanMcBain DuncanMcBain merged commit 28f993e into main Jan 24, 2025
8 of 11 checks passed
@DuncanMcBain DuncanMcBain deleted the fix-includes branch January 24, 2025 17:42
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