FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

Handle C long size on Windows. by filmor · Pull Request #497 · pythonnet/pythonnet · GitHub

  1. pythonnet
  2. pythonnet

Repository navigation

Handle C long size on Windows. - #497

Merged
filmor merged 1 commit into
pythonnet:masterfrom
filmor:handle-clong-windows
Jun 26, 2017
ConversationCommits (1)ChecksFiles changed
Merged

Handle C long size on Windows.#497
filmor merged 1 commit into
pythonnet:masterfrom
filmor:handle-clong-windows

Conversation

filmor commented Jun 16, 2017

Copy link
Copy Markdown
Member

On Windows 64bit systems, a C long is a 32 bit integer while the rest
of the world has agreed on making sizeof(long) == sizeof(void*).

What does this implement/fix? Explain your changes.

New attempt on #376

Any other comments?

@vmuriart Do you have a test-case for PR #376?

Checklist

Check all those that are applicable and complete.

  • Make sure to include one or more tests for your change
  • If an enhancement PR, please create docs and at best an example
  • Add yourself to AUTHORS
  • Updated the CHANGELOG

Handle C long size on Windows.
cd16bab 
On Windows 64bit systems, a C `long` is a 32 bit integer while the rest
of the world has agreed on making `sizeof(long) == sizeof(void*)`.

mention-bot commented Jun 16, 2017

Copy link
Copy Markdown

@filmor, thanks! @vmuriart, @tiran, @brianlloyd, @hsoft and @tonyroberts, please review this.

den-run-ai commented Jun 19, 2017

Copy link
Copy Markdown
Contributor

@filmor tp_flags is c_long in Python 2.7 and unsigned c_long in Python 3.6. This may not fit into CLR int (System.Int32) on *NIX and should be unsigned UInt32 or UInt64 for PYTHON3.

den-run-ai commented Jun 19, 2017

Copy link
Copy Markdown
Contributor

https://docs.python.org/3/c-api/typeobj.html
https://docs.python.org/2/c-api/typeobj.html

codecov Bot commented Jun 19, 2017 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #497 into master will decrease coverage by 0.05%.
The diff coverage is 65%.

@@            Coverage Diff             @@
##           master     #497      +/-   ##
==========================================
- Coverage   76.38%   76.33%   -0.06%     
==========================================
  Files          64       65       +1     
  Lines        5573     5582       +9     
  Branches      896      898       +2     
==========================================
+ Hits         4257     4261       +4     
- Misses       1021     1024       +3     
- Partials      295      297       +2
Flag Coverage Δ
#setup_linux 75.71% <ø> (ø) ⬆️
#setup_windows 75.69% <65%> (-0.06%) ⬇️
Impacted Files Coverage Δ
src/runtime/metatype.cs 70.73% <100%> (ø) ⬆️
src/runtime/clrobject.cs 96.15% <100%> (ø) ⬆️
src/runtime/runtime.cs 90.87% <100%> (+0.03%) ⬆️
src/runtime/Util.cs 37.5% <37.5%> (ø)
src/runtime/managedtype.cs 50% <50%> (ø) ⬆️
src/runtime/typemanager.cs 84.03% <75%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12af794...cd16bab. Read the comment docs.

filmor commented Jun 19, 2017

Copy link
Copy Markdown
Member Author

Does it matter, though? Bitwise it doesn't make a difference, and I'm reading the actual value of tp_flags using the ReadInt functions, so they will just interpret too large numbers as negative.

filmor merged commit 0ce6301 into pythonnet:master Jun 26, 2017
testrunner123 pushed a commit to testrunner123/pythonnet that referenced this pull request Sep 22, 2017
Handle C long size on Windows. (pythonnet#497)
3c28ca3 
On Windows 64bit systems, a C `long` is a 32 bit integer while the rest
of the world has agreed on making `sizeof(long) == sizeof(void*)`.
filmor deleted the handle-clong-windows branch September 26, 2019 06:42
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
Show hidden characters
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

tonyroberts Awaiting requested review from tonyroberts
vmuriart Awaiting requested review from vmuriart
1 more reviewer
den-run-ai den-run-ai approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

Back | FazBrowse Home | New Git URL