Skip to content

[NFC][SYCL] Use C++17 nested namespaces #7692

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
Dec 10, 2022

Conversation

aelovikov-intel
Copy link
Contributor

The change was done semi-automtically via clang-tidy with subsequent manual review to revert incorrect changes followed by clang-format run to update the comments on the namespace closing bracket.

The change was done semi-automtically via clang-tidy with subsequent
manual review to revert incorrect changes followed by clang-format run
to update the comments on the namespace closing bracket.
Comment on lines +44 to +52
namespace property ::queue {
namespace __SYCL2020_DEPRECATED(
"use 'sycl::ext::oneapi::cuda::property::queue' instead") cuda {
class use_default_stream
: public ::sycl::ext::oneapi::cuda::property::queue::use_default_stream {};
// clang-format off
} // namespace cuda
// clang-format on
} // namespace queue
} // namespace property
} // namespace property::queue
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably we should move this before namespace ext::oneapi declarations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'd rather not do it here.

namespace ext {
namespace intel {
namespace math {
namespace ext::intel::math {

#if __cplusplus >= 201703L
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still intended to support compatibility with C++14 mode? I see that some C++17 features are being used unconditionally.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, we require at least C++17 since #6678. I'm removing the guards in #7687.

@aelovikov-intel
Copy link
Contributor Author

@rolandschulz @kbobrovs , please review this trivial change.

Copy link
Contributor

@kbobrovs kbobrovs left a comment

Choose a reason for hiding this comment

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

ESIMD LGTM

@aelovikov-intel
Copy link
Contributor Author

@intel/llvm-gatekeepers , PR is ready.

@bader bader merged commit 18718fd into intel:sycl Dec 10, 2022
Copy link
Contributor

@keryell keryell left a comment

Choose a reason for hiding this comment

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

The cleaning is always appreciated! :-)

@aelovikov-intel aelovikov-intel deleted the nested-namespace branch May 1, 2023 16:12
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.

5 participants