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

[1/N] Use std::string_view #7297

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

[1/N] Use std::string_view #7297

wants to merge 3 commits into from

Conversation

cyyever
Copy link
Collaborator

@cyyever cyyever commented Jun 16, 2024

Another attempt to use STL.

@cyyever cyyever marked this pull request as draft June 16, 2024 23:49
@JackCaoG
Copy link
Collaborator

build failures seem real


2024-06-16T14:30:11.6423560Z     bazel-out/k8-opt/bin/external/torch/_virtual_includes/headers/torch/csrc/lazy/core/hash.h:240:26: error: no matching function for call to 'Hash(std::basic_string_view<char>&)'
2024-06-16T14:30:11.6425004Z       240 |   return HashCombine(Hash(value), MHash(Fargs...));
2024-06-16T14:30:11.6425646Z           |                      ~~~~^~~~~~~
2024-06-16T14:30:11.6427673Z     bazel-out/k8-opt/bin/external/torch/_virtual_includes/headers/torch/csrc/lazy/core/hash.h:66:8: note: candidate: 'template<class T, typename std::enable_if<std::is_arithmetic<_Tp>::value>::type* <anonymous> > torch::lazy::hash_t torch::lazy::Hash(const T&)'
2024-06-16T14:30:11.6429447Z        66 | hash_t Hash(const T& value) ***
2024-06-16T14:30:11.6429925Z           |        ^~~~
2024-06-16T14:30:11.6431094Z     bazel-out/k8-opt/bin/external/torch/_virtual_includes/headers/torch/csrc/lazy/core/hash.h:66:8: note:   template argument deduction/substitution failed:
2024-06-16T14:30:11.6433071Z     bazel-out/k8-opt/bin/external/torch/_virtual_includes/headers/torch/csrc/lazy/core/hash.h:65:68: error: no type named 'type' in 'struct std::enable_if<false, void>'
2024-06-16T14:30:11.6434434Z        65 |     typename std::enable_if<std::is_arithmetic<T>::value>::type* = nullptr>
2024-06-16T14:30:11.6435173Z           |                                                                    ^~~~~~~
2024-06-16T14:30:11.6436671Z     bazel-out/k8-opt/bin/external/torch/_virtual_includes/headers/torch/csrc/lazy/core/hash.h:72:18: note: candidate: 'torch::lazy::hash_t torch::lazy::Hash(const std::vector<bool>&)'
2024-06-16T14:30:11.6438023Z        72 | hash_t TORCH_API Hash(const std::vector<bool>& value);
2024-06-16T14:30:11.6438590Z           |                  ^~~~
2024-06-16T14:30:11.6440121Z     bazel-out/k8-opt/bin/external/torch/_virtual_includes/headers/torch/csrc/lazy/core/hash.h:72:48: note:   no known conversion for argument 1 from 'std::basic_string_view<char>' to 'const std::vector<bool>&'
2024-06-16T14:30:11.6441585Z        72 | hash_t TORCH_API Hash(const std::vector<bool>& value);
2024-06-16T14:30:11.6442201Z           |                       ~~~~~~~~~~~~~~~~~~~~~~~~~^~~~~
2024-06-16T14:30:11.6443666Z     bazel-out/k8-opt/bin/external/torch/_virtual_includes/headers/torch/csrc/lazy/core/hash.h:75:22: note: candidate: 'torch::lazy::hash_t torch::lazy::Hash(const c10::ScalarType&)'
2024-06-16T14:30:11.6445028Z        75 | static inline hash_t Hash(const c10::ScalarType& value) ***

@cyyever
Copy link
Collaborator Author

cyyever commented Jun 17, 2024

@JackCaoG I have to patch upstream first.

@cyyever cyyever changed the title Use std::string_view [1/N] Use std::string_view Jun 20, 2024
@cyyever cyyever marked this pull request as ready for review July 1, 2024 11:01
@cyyever cyyever force-pushed the string_view branch 2 times, most recently from c39406d to 5445a3c Compare July 3, 2024 00:49
@cyyever cyyever force-pushed the string_view branch 5 times, most recently from 8bcfe77 to fade1af Compare July 17, 2024 06:09
@cyyever
Copy link
Collaborator Author

cyyever commented Oct 30, 2024

@JackCaoG Upstream changes have been merged.

@cyyever cyyever marked this pull request as draft October 30, 2024 01:17
@cyyever cyyever marked this pull request as ready for review November 28, 2024 00:40
@cyyever cyyever enabled auto-merge (squash) November 28, 2024 00:41
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