Skip to content

[SYCL][NFC] Fix Windows warnings, sync usage of class/struct with new… #332

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 1 commit into from
Jul 17, 2019

Conversation

v-klochkov
Copy link
Contributor

… SYCL SPEC

Signed-off-by: Klochkov [email protected]

@v-klochkov v-klochkov requested a review from bader July 17, 2019 03:57
bader
bader previously approved these changes Jul 17, 2019
@@ -40,7 +40,8 @@ getOrWaitEvents(std::vector<cl::sycl::event> DepEvents,

void waitEvents(std::vector<cl::sycl::event> DepEvents);

struct Builder {
class Builder {
Copy link
Contributor

Choose a reason for hiding this comment

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

Builder struct is not part of the SYCL specification.
Did you decide to avoid using struct for all types in our project - even for internal?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is already 3rd time when I fix something for Builder. Have no idea why, but Builder tends to be treated as class by other developers.

@v-klochkov
Copy link
Contributor Author

v-klochkov commented Jul 17, 2019

It looks like I forgot to run check-sycl before uploading it yesterday. Testing showed that I forgot 'public:' keyword at the beginning of 'class item' definition.
Fixed it now.

@bader bader merged commit df3552c into intel:sycl Jul 17, 2019
@v-klochkov v-klochkov deleted the public_windows_warnings branch July 18, 2019 03:16
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Feb 23, 2023
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Feb 23, 2023
aelovikov-intel pushed a commit to aelovikov-intel/llvm that referenced this pull request Mar 27, 2023
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