Skip to content
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

Remove std_classes test #883

Conversation

AlexeySachkov
Copy link
Contributor

The test seems to be 1.2.1 legacy, which doesn't make sense in SYCL 2020 testing:

  • the test does not check anything at runtime
  • the only compile-time "check" which is performed is successful compilation of the test source itself and there are no static assertions
  • test source code contains nothing SYCL-specific and only uses standard C++ functionality

The test used to check type aliases like cl::sycl::string_class, but it lost its purpose in e08cb45 when it was updated to match SYCL 2020 interface that directly uses standard C++ containers.

The test seems to be 1.2.1 legacy, which doesn't make sense in SYCL 2020
testing:
- the test does not check anything at runtime
- the only compile-time "check" which is performed is successful
  compilation of the test source itself and there are no static
  assertions
- test source code contains nothing SYCL-specific and only uses standard
  C++ functionality

The test used to check type aliases like `cl::sycl::string_class`, but
it lost its purpose in e08cb45 when it
was updated to match SYCL 2020 interface that directly uses standard C++
containers.
@AlexeySachkov AlexeySachkov requested a review from a team as a code owner March 22, 2024 10:02
Copy link
Member

@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.

Good catch!

@bader bader merged commit c6f9b16 into KhronosGroup:SYCL-2020 Mar 30, 2024
7 checks passed
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