Skip to content

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.

3 participants