Skip to content

[SYCL][NFC] Handle a bunch of warnings coming from SYCL RT #1802

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Jun 5, 2020
Merged

[SYCL][NFC] Handle a bunch of warnings coming from SYCL RT #1802

merged 14 commits into from
Jun 5, 2020

Conversation

alexbatashev
Copy link
Contributor

  • -Wstring-conversion: get rid of converting const char* to bool in assert
  • -Winconsistent-missing-override: add missing final or override
  • -Wcovered-switch-default: remove unreachable default clauses
  • -Wpessimizing-move: allow temporary object copy elision by removing std::move
  • -Wcast-qual: explicitly cast away const qualifier
  • -Wunused-parameter: remove some unused parameters

Alexander Batashev added 10 commits June 2, 2020 11:32
Remove unreachable default: branches in image_accessor_util.hpp
89b8539 
Signed-off-by: Alexander Batashev <[email protected]>
Handle a few warnings coming from the scheduler
1883fa0 
Signed-off-by: Alexander Batashev <[email protected]>
More warnings in commands.hpp
20c545d 
Signed-off-by: Alexander Batashev <[email protected]>
More of scheduler
7fb60d1 
Signed-off-by: Alexander Batashev <[email protected]>
Fix a few warnings coming from tests
68257fc 
Signed-off-by: Alexander Batashev <[email protected]>
Silence warning in handler
09d8ce7 
Signed-off-by: Alexander Batashev <[email protected]>
Fix handler warning
5a06014 
Signed-off-by: Alexander Batashev <[email protected]>
Fix a few memory manager warnings
f91b717 
Signed-off-by: Alexander Batashev <[email protected]>
More test warnings
1220a26 
Signed-off-by: Alexander Batashev <[email protected]>
clang-format
69dd1ad
@alexbatashev alexbatashev requested a review from a team as a code owner June 2, 2020 09:33
AGindinson
AGindinson reviewed Jun 2, 2020
View reviewed changes
Copy link
Contributor

@AGindinson AGindinson left a comment

Choose a reason for hiding this comment

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

The PiMock part LGTM!

@bader
Copy link
Contributor

bader commented Jun 2, 2020

  • -Wstring-conversion: get rid of converting const char* to bool in assert
  • -Winconsistent-missing-override: add missing final or override
  • -Wcovered-switch-default: remove unreachable default clauses
  • -Wpessimizing-move: allow temporary object copy elision by removing std::move
  • -Wcast-qual: explicitly cast away const qualifier
  • -Wunused-parameter: remove some unused parameters

CI system must prevent us from adding new warnings enabled by these options.

@alexbatashev
Copy link
Contributor Author

  • -Wstring-conversion: get rid of converting const char* to bool in assert
  • -Winconsistent-missing-override: add missing final or override
  • -Wcovered-switch-default: remove unreachable default clauses
  • -Wpessimizing-move: allow temporary object copy elision by removing std::move
  • -Wcast-qual: explicitly cast away const qualifier
  • -Wunused-parameter: remove some unused parameters

CI system must prevent us from adding new warnings enabled by these options.

I'm afraid, some of those are specific to newer compiler version than what we have in CI.

s-kanaev
s-kanaev reviewed Jun 2, 2020
View reviewed changes
s-kanaev
s-kanaev previously approved these changes Jun 2, 2020
View reviewed changes
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

LGTM commands.cpp, commands.hpp, handler.hpp

s-kanaev
s-kanaev reviewed Jun 2, 2020
View reviewed changes
romanovvlad
romanovvlad reviewed Jun 2, 2020
View reviewed changes
Copy link
Contributor

@romanovvlad romanovvlad left a comment

Choose a reason for hiding this comment

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

LGTM

garimagu
garimagu reviewed Jun 2, 2020
View reviewed changes
Alexander Batashev added 2 commits June 3, 2020 11:16
@alexbatashev alexbatashev requested a review from s-kanaev June 3, 2020 08:21
s-kanaev
s-kanaev approved these changes Jun 4, 2020
View reviewed changes
Copy link
Contributor

@s-kanaev s-kanaev left a comment

Choose a reason for hiding this comment

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

LGTM

@bader bader merged commit 1eda3e3 into intel:sycl Jun 5, 2020
@alexbatashev alexbatashev deleted the warnings branch September 17, 2021 06:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bader bader bader left review comments

@romanovvlad romanovvlad romanovvlad left review comments

@AGindinson AGindinson AGindinson left review comments

@garimagu garimagu garimagu left review comments

@s-kanaev s-kanaev s-kanaev approved these changes

@rbegam rbegam Awaiting requested review from rbegam

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@alexbatashev @bader @romanovvlad @AGindinson @garimagu @s-kanaev