-
Notifications
You must be signed in to change notification settings - Fork 484
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
Replacing long int type with int64_t #3739
Replacing long int type with int64_t #3739
Conversation
The code changes involve updating the type `long` or `long int`. The type `long int` is replaced with `int64_t` in several places to ensure compatibility and improve code clarity.
WalkthroughWalkthroughThe recent updates across various source files primarily focus on enhancing data type consistency for array and index size handling. Changes include shifting from Changes
Recent Review DetailsConfiguration used: CodeRabbit UI Files selected for processing (1)
Files skipped from review as they are similar to previous changes (1)
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
for more information, see https://pre-commit.ci
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #3739 +/- ##
==========================================
- Coverage 82.19% 82.18% -0.01%
==========================================
Files 513 513
Lines 47642 47642
Branches 2980 2980
==========================================
- Hits 39159 39156 -3
- Misses 7572 7575 +3
Partials 911 911 ☔ View full report in Codecov by Sentry. |
should it be included? |
After giving it some thought, because this observed behavior with |
Following the discussion in #3657 this pull request addresses the usage of
long
orlong int
by replacing them withint64_t
in multiple instances. This change aims to enhance code compatibility across different platforms and improve code clarity.The
int64_t
type is a feature introduced in C++11, defined in the<cstdint>
header. Due to historical reasons, the compilation behavior ofint64_t
is platform- and system-specific. On Linux,int64_t
is compiled tolong
, whereas on macOS, it's compiled tolong long
.In relevant codebases such as PyTorch and TensorFlow,
int64_t
is preferred over explicit declarations oflong
orlong long
. Consequently, for precompiled libraries, on Linux, symbols are defined exclusively tolong
, while on macOS, symbols are defined exclusively based onlong long
.For these reasons,
data_ptr<long int>()
is unable to compile on macOS.References
Examples
Example 1
For the code used here,
torch::from_blob
, is defined usingwhere
IntArrayRef
is defined asExample 2
Dumping the symbols in
libtorch_cpu.dylib
on macOSdumping symbols in
libtorch_cpu.dylib
on LinuxSummary by CodeRabbit