Skip to content

Addressing 32-bit issues with NSXMLNodeOptions #252

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

Merged
merged 1 commit into from
Feb 4, 2016
Merged

Addressing 32-bit issues with NSXMLNodeOptions #252

merged 1 commit into from
Feb 4, 2016

Conversation

hpux735
Copy link
Contributor

@hpux735 hpux735 commented Feb 3, 2016

This patch aims to address the issues with NSXMLNodeOptions on 32-bit platforms in a way that doesn't impact the API. The current problem (that NSXMLNodeOptionRawType aimed to fix) is that the assignment of the sign bit of an Int is flagged by Swift as an overflow. By assigning this bit with the acknowledgement that it is becoming negative quiets the compiler while maintaining the effect.

@@ -43,55 +43,55 @@
@constant NSXMLDocumentIncludeContentTypeDeclaration Include a content type declaration for HTML or XHTML
*/

#if arch(x86_64) || arch(arm64)
public typealias NSXMLNodeOptionRawType = Int
#if arch(x86_64) || arch(arm64) || arch(powerpc64le)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so we have to keep bumping this when we get other 64 bit platforms? mips etc? is there not a swift defined or 64 bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I believe so. There are no configuration statements listed for bit-width in the Swift Programming Language book. I would love for that to exist.

@hpux735
Copy link
Contributor Author

hpux735 commented Feb 4, 2016

I dont know what happened to your comment, but yes. The bitmask seems to have worked as expected on 32 & 64 bit machines. That's much better!

@phausler
Copy link
Contributor

phausler commented Feb 4, 2016

👍 I like this change, builds cleanly and tests pass

phausler added a commit that referenced this pull request Feb 4, 2016
Addressing 32-bit issues with NSXMLNodeOptions
@phausler phausler merged commit ceb15d8 into swiftlang:master Feb 4, 2016
@hpux735
Copy link
Contributor Author

hpux735 commented Feb 4, 2016

Thanks the the help, Philippe! 😀

@hpux735 hpux735 deleted the xml branch February 4, 2016 00:30
atrick pushed a commit to atrick/swift-corelibs-foundation that referenced this pull request Jan 12, 2021
Make BuildSystemManager passthrough notifications fully `async
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