-
Notifications
You must be signed in to change notification settings - Fork 787
[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
Conversation
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.
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 |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
sycl/include/sycl/ext/intel/math.hpp
Outdated
namespace ext { | ||
namespace intel { | ||
namespace math { | ||
namespace ext::intel::math { | ||
|
||
#if __cplusplus >= 201703L |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@rolandschulz @kbobrovs , please review this trivial change. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ESIMD LGTM
@intel/llvm-gatekeepers , PR is ready. |
There was a problem hiding this 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! :-)
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.