-
Notifications
You must be signed in to change notification settings - Fork 25.6k
Fix/device type prefixed with c10 namespace in headers #91342
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
Fix/device type prefixed with c10 namespace in headers #91342
Conversation
|
Looks ok to me! Can you sign the CLA? |
Thanks Brian !
I’ll sign the CLA soon, I’m on break right now.
… On Dec 27, 2022, at 3:15 PM, Brian Hirsh ***@***.***> wrote:
Looks ok to me! Can you sign the CLA?
—
Reply to this email directly, view it on GitHub <#91342 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AC2O6UJPF2WWCLE77CTG2M3WPN2AJANCNFSM6AAAAAATHH6NTY>.
You are receiving this because you were mentioned.
|
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.
Sounds good.
Could you also add a lint entry for this to make sure we don't add new ones.
It can be similar to the one below I think:
Lines 766 to 785 in dfb6514
[[linter]] | |
code = 'ONCE_FLAG' | |
include_patterns = [ | |
'c10/**', | |
'aten/**', | |
'torch/csrc/**', | |
] | |
command = [ | |
'python3', | |
'tools/linter/adapters/grep_linter.py', | |
'--pattern=std::once_flag', | |
'--linter-name=ONCE_FLAG', | |
'--error-name=invalid once_flag', | |
'--replace-pattern=s/std::once_flag/c10::once_flag/', | |
"""--error-description=\ | |
Use of std::once_flag is forbidden and should be replaced with c10::once_flag\ | |
""", | |
'--', | |
'@{{PATHSFILE}}' | |
] |
Looks like this PR hasn't been updated in a while so we're going to go ahead and mark this as |
Hi there, would it be OK to bring this in without the linter as a start ? And make adding the linter rule a follow-up ? I can make a new PR with the latest state of the master branch. |
…4364) Fixes #91338 Follow up from #91342 > 🚀 The feature, motivation and pitch > We have an existing DeviceType class all over the place in our code base, and it conflicts with the one that is used in torch. > Thankfully the pytorch DeciceType enum class is under the c10 namespace. ``` In file included from /xxx/build/_deps/torch-src/../../aten/src/ATen/ops/view.h:5: /xxx/_deps/torch-src/aten/src/ATen/Context.h:265:14: error: reference to 'DeviceType' is ambiguous if (p == DeviceType::HIP) { ^ /xxx/include/Common_types.h:178:8: note: candidate found by name lookup is 'DeviceType' struct DeviceType { ^ /xxx/build/_deps/torch-src/c10/../c10/core/DeviceType.h:32:12: note: candidate found by name lookup is 'c10::DeviceType' enum class DeviceType : int8_t { ``` Pull Request resolved: #104364 Approved by: https://github.com/albanD
Fixes #91338
cc @mcarilli @ptrblck @leslie-fang-intel @jgong5