Skip to content
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

[Foundation] Fallback to presuming 32-bit platforms if we dont explicitly know that it is 64 bit; this allows for better portability #21290

Merged
merged 1 commit into from
Dec 13, 2018

Conversation

phausler
Copy link
Contributor

This should resolve rdar://problem/46683060

@phausler
Copy link
Contributor Author

@swift-ci please smoke test

Copy link
Contributor

@itaiferber itaiferber left a comment

Choose a reason for hiding this comment

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

Looks reasonable to me. We'll need to be careful to correctly add 64-bit platforms in the future, but being conservative is good.

@jrose-apple
Copy link
Contributor

This seems like a bad idea to me. Most platforms people are trying to port Swift to are 64-bit these days. #else + #error would be a lot safer.

@itaiferber
Copy link
Contributor

itaiferber commented Dec 13, 2018

@jrose-apple Is there a more future-proof way to avoid breaking the build?
I don't disagree that being explicit (and aiming for 64-bit) is a good idea, but there are also platforms we can't necessarily know to switch on.

@jrose-apple
Copy link
Contributor

This seems like one of those "trap early" philosophy things: it is better to break the build at the point where something is unknown, with an explicit message, then to stumble on and fail later when you don't know what's missing.

@itaiferber
Copy link
Contributor

@phausler I'll let you make the call here. I'm alright with taking care of this internally too.

@phausler phausler force-pushed the data_default_HalfInt branch from eb442e2 to 5bc09ae Compare December 13, 2018 18:35
@phausler
Copy link
Contributor Author

@swift-ci please smoke test

@phausler
Copy link
Contributor Author

I really wish we had a #if bits(64) or #if bits(32) or something like that. It would alleviate this type of error.

…itly know that it is 64 bit; this allows for better portability
@phausler phausler force-pushed the data_default_HalfInt branch from 5bc09ae to b0d7c05 Compare December 13, 2018 19:04
@phausler
Copy link
Contributor Author

@swift-ci please smoke test

@phausler phausler merged commit 77c7a69 into swiftlang:master Dec 13, 2018
phausler added a commit to phausler/swift that referenced this pull request Dec 13, 2018
…t explicitly know that it is 64 bit; this allows for better portability (swiftlang#21290)"

This reverts commit 77c7a69.
@xwu
Copy link
Collaborator

xwu commented Dec 14, 2018

@phausler I'm not really in a position to work on it at the moment, but I do have a start on that exact feature here: #14413.

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.

4 participants