-
Notifications
You must be signed in to change notification settings - Fork 795
[SYCL] Add size to detail::string_view #18661
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
steffenlarsen
merged 14 commits into
intel:sycl
from
Alexandr-Konovalov:Alexandr-Konovalov/string_view-size
Jun 3, 2025
Merged
Changes from 9 commits
Commits
Show all changes
14 commits
Select commit
Hold shift + click to select a range
74b4758
[SYCL] Add size to detail::string_view
Alexandr-Konovalov 036b4a5
Merge branch 'sycl' into Alexandr-Konovalov/string_view-size
Alexandr-Konovalov 34f2b76
Code formatting.
Alexandr-Konovalov 88dcfda
Unify MKernelName usage.
Alexandr-Konovalov d0914f4
Code formatting.
Alexandr-Konovalov c60080e
Code formatting.
Alexandr-Konovalov c64686e
Next round of removing excessive strlen().
Alexandr-Konovalov f923082
Merge branch 'sycl' into Alexandr-Konovalov/string_view-size
Alexandr-Konovalov df1a8df
Code formatting.
Alexandr-Konovalov 5cefaf1
Unify toKernelNameStrT().
Alexandr-Konovalov 585dd61
Remove sycl/test/native_cpu/multiple_definitions.cpp.
Alexandr-Konovalov 0b1006d
Merge branch 'sycl' into Alexandr-Konovalov/string_view-size
Alexandr-Konovalov a5b315c
Merge branch 'sycl' into Alexandr-Konovalov/string_view-size
Alexandr-Konovalov 4e5aa27
Merge branch 'sycl' into Alexandr-Konovalov/string_view-size
Alexandr-Konovalov File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,33 +17,80 @@ namespace detail { | |
|
|
||
| // This class and detail::string class are intended to support | ||
| // different ABIs between libsycl and the user program. | ||
| // This class is not inteded to replace std::string_view for general purpose | ||
| // This class is not intended to replace std::string_view for general purpose | ||
| // usage. | ||
|
|
||
| class string_view { | ||
| const char *str = nullptr; | ||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| size_t len = 0; | ||
| #endif | ||
|
|
||
| public: | ||
| string_view() noexcept = default; | ||
| string_view(const string_view &strn) noexcept = default; | ||
| string_view(string_view &&strn) noexcept = default; | ||
| string_view(std::string_view strn) noexcept : str(strn.data()) {} | ||
| string_view(const sycl::detail::string &strn) noexcept : str(strn.c_str()) {} | ||
| string_view(std::string_view strn) noexcept | ||
| : str(strn.data()) | ||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| , | ||
| len(strn.size()) | ||
| #endif | ||
| { | ||
| } | ||
| string_view(const sycl::detail::string &strn) noexcept | ||
| : str(strn.c_str()) | ||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| , | ||
| len(strlen(strn.c_str())) | ||
| #endif | ||
| { | ||
| } | ||
|
|
||
| string_view &operator=(string_view &&strn) noexcept = default; | ||
| string_view &operator=(const string_view &strn) noexcept = default; | ||
|
|
||
| string_view &operator=(std::string_view strn) noexcept { | ||
| str = strn.data(); | ||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| len = strn.size(); | ||
| #endif | ||
| return *this; | ||
| } | ||
|
|
||
| string_view &operator=(const sycl::detail::string &strn) noexcept { | ||
| str = strn.c_str(); | ||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| len = strlen(strn.c_str()); | ||
| #endif | ||
| return *this; | ||
| } | ||
|
|
||
| const char *data() const noexcept { return str ? str : ""; } | ||
|
|
||
| explicit operator std::string_view() const noexcept { | ||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| return std::string_view(str, len); | ||
| #else | ||
| return std::string_view(str); | ||
| #endif | ||
| } | ||
|
Comment on lines
+71
to
+77
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't we pass
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
|
|
||
| #ifdef __INTEL_PREVIEW_BREAKING_CHANGES | ||
| friend bool operator==(string_view lhs, std::string_view rhs) noexcept { | ||
| return rhs == std::string_view(lhs); | ||
| } | ||
| friend bool operator==(std::string_view lhs, string_view rhs) noexcept { | ||
| return lhs == std::string_view(rhs); | ||
| } | ||
|
|
||
| friend bool operator!=(string_view lhs, std::string_view rhs) noexcept { | ||
| return rhs != std::string_view(lhs); | ||
| } | ||
| friend bool operator!=(std::string_view lhs, string_view rhs) noexcept { | ||
| return lhs != std::string_view(rhs); | ||
| } | ||
| #else | ||
| friend bool operator==(string_view lhs, std::string_view rhs) noexcept { | ||
| return rhs == lhs.data(); | ||
| } | ||
|
|
@@ -57,6 +104,7 @@ class string_view { | |
| friend bool operator!=(std::string_view lhs, string_view rhs) noexcept { | ||
| return lhs != rhs.data(); | ||
| } | ||
| #endif | ||
| }; | ||
|
|
||
| } // namespace detail | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 the
strargument constness need to differ between the definitions? I would prefer we keep them the same, even if it means removingconstin the variant below. If it gets removed with breaking changes, I would argue it's better to have it the same and avoid the potential hidden faulty const use-cases we could have.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.
Sorry, probably I do not understand. Are you for passing
detail::string_viewby const reference, like this?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.
If that is an option, that would be good! My point is just to avoid differing constness of the reference argument in the function.
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.
Sure, let's unify.