Move incorrectly placed closing curly brace of extern "C" block
#87853
Conversation
This file contains 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
For SYCL device, the closing curly brace would not be encountered in the current code. Compilers don't report an error or warning for such errors. This can lead to issues such as this one - https://stackoverflow.com/questions/61717819/template-with-c-linkage-error-when-using-template-keyword-in-main-cpp/61717908#61717908
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/87853
Note: Links to docs will display an error until the docs builds have been completed. ✅ No FailuresAs of commit d076d23: This comment was automatically generated by Dr. CI and updates every 15 minutes. |
sanchitintel
changed the title
extern "C"extern "C"
sanchitintel
changed the title
extern "C"extern "C" block
|
Hi @malfet, can you please help review this one-liner bug-fix? Thanks! :) |
sanchitintel
added
intel
kit1980
approved these changes
Oct 28, 2022
|
@pytorchbot merge |
Merge startedYour change will be merged once all checks pass (ETA 0-4 Hours). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
malfet
approved these changes
Oct 28, 2022
|
@pytorchbot merge -f "This is a build only change" |
|
The merge job was canceled. If you believe this is a mistake,then you can re trigger it through pytorch-bot. |
Merge startedYour change will be merged immediately since you used the force (-f) flag, bypassing any CI checks (ETA: 1-5 minutes). Learn more about merging in the wiki. Questions? Feedback? Please reach out to the PyTorch DevX Team |
|
Hey @sanchitintel. |
sgrigory
pushed a commit
to sgrigory/pytorch
that referenced
this pull request
Oct 28, 2022
…torch#87853) ### Bug description When `__SYCL_DEVICE_ONLY__` is defined, while building PyTorch, the output of the preprocessing step would not have the closing curly brace of the `extern "C"` block, as it has been incorrectly placed. Compilers don't seem to report an error or a warning for a missing closing brace of an `extern "C"` block. ### Impact of the bug If `c10/macros/Macros.h` would be included in a C++ file, and after the preprocessing stage, if the preprocessed source file would have some templated code after `extern "C" {`, then, after compilation, linking might fail with the error `templates must have c++ linkage`). eg. https://stackoverflow.com/questions/61717819/template-with-c-linkage-error-when-using-template-keyword-in-main-cpp/61717908#61717908 (its answer also has a small snippet of code to reproduce such an issue). ### Solution in this PR one-liner bug fix that rectifies the placement of closing curly brace (`}`), so that the `extern "C"` block ends properly when `__SYCL_DEVICE_ONLY__` is defined. Pull Request resolved: pytorch#87853 Approved by: https://github.com/jgong5, https://github.com/kit1980, https://github.com/malfet
kulinseth
pushed a commit
to kulinseth/pytorch
that referenced
this pull request
Nov 5, 2022
…torch#87853) ### Bug description When `__SYCL_DEVICE_ONLY__` is defined, while building PyTorch, the output of the preprocessing step would not have the closing curly brace of the `extern "C"` block, as it has been incorrectly placed. Compilers don't seem to report an error or a warning for a missing closing brace of an `extern "C"` block. ### Impact of the bug If `c10/macros/Macros.h` would be included in a C++ file, and after the preprocessing stage, if the preprocessed source file would have some templated code after `extern "C" {`, then, after compilation, linking might fail with the error `templates must have c++ linkage`). eg. https://stackoverflow.com/questions/61717819/template-with-c-linkage-error-when-using-template-keyword-in-main-cpp/61717908#61717908 (its answer also has a small snippet of code to reproduce such an issue). ### Solution in this PR one-liner bug fix that rectifies the placement of closing curly brace (`}`), so that the `extern "C"` block ends properly when `__SYCL_DEVICE_ONLY__` is defined. Pull Request resolved: pytorch#87853 Approved by: https://github.com/jgong5, https://github.com/kit1980, https://github.com/malfet
kulinseth
pushed a commit
to kulinseth/pytorch
that referenced
this pull request
Dec 10, 2022
…torch#87853) ### Bug description When `__SYCL_DEVICE_ONLY__` is defined, while building PyTorch, the output of the preprocessing step would not have the closing curly brace of the `extern "C"` block, as it has been incorrectly placed. Compilers don't seem to report an error or a warning for a missing closing brace of an `extern "C"` block. ### Impact of the bug If `c10/macros/Macros.h` would be included in a C++ file, and after the preprocessing stage, if the preprocessed source file would have some templated code after `extern "C" {`, then, after compilation, linking might fail with the error `templates must have c++ linkage`). eg. https://stackoverflow.com/questions/61717819/template-with-c-linkage-error-when-using-template-keyword-in-main-cpp/61717908#61717908 (its answer also has a small snippet of code to reproduce such an issue). ### Solution in this PR one-liner bug fix that rectifies the placement of closing curly brace (`}`), so that the `extern "C"` block ends properly when `__SYCL_DEVICE_ONLY__` is defined. Pull Request resolved: pytorch#87853 Approved by: https://github.com/jgong5, https://github.com/kit1980, https://github.com/malfet
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Bug description
When
__SYCL_DEVICE_ONLY__is defined, while building PyTorch, the output of the preprocessing step would not have the closing curly brace of theextern "C"block, as it has been incorrectly placed. Compilers don't seem to report an error or a warning for a missing closing brace of anextern "C"block.Impact of the bug
If
c10/macros/Macros.hwould be included in a C++ file, and after the preprocessing stage, if the preprocessed source file would have some templated code afterextern "C" {, then compilation might fail with the errortemplates must have c++ linkage). eg. https://stackoverflow.com/questions/61717819/template-with-c-linkage-error-when-using-template-keyword-in-main-cpp/61717908#61717908 (its answer also has a small snippet of code to reproduce such an issue).Solution in this PR
one-liner bug fix that rectifies the placement of closing curly brace (
}), so that theextern "C"block ends properly when__SYCL_DEVICE_ONLY__is defined.