Skip to content

Conversation

@chnmrc
Copy link
Contributor

@chnmrc chnmrc commented May 19, 2018

Resolve a bug that affects ARMv7 system
https://bugs.swift.org/browse/SR-6812

@hpux735
Copy link
Contributor

hpux735 commented May 19, 2018

Thanks @chnmrc ! This seems pretty straight-forward to me.

@millenomi
Copy link
Contributor

I assume this just moves the CF types to the memory layout Swift objects have on 32 bit platforms? Is it the same on other 32 bit platforms or does it need a specific armv7 guard?

@hpux735
Copy link
Contributor

hpux735 commented May 20, 2018

That's a good question. We don't have testing on other non-supported 32-bit platforms. As there aren't any other 32-bit platforms in community CI, I can't be sure who represents those platforms, if they're being used at all.

uintptr_t _cfisa;
uintptr_t _swift_rc;
// This is for CF's use, and must match _NSCFType layout
#if __LP64__
Copy link
Contributor

Choose a reason for hiding this comment

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

Presumably LLP64, too.

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, for Windows also, maybe we must add an || condition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like the .c file was updated but not the .h file?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is that a question to me?

@parkera
Copy link
Contributor

parkera commented May 21, 2018

@swift-ci please test

@millenomi
Copy link
Contributor

@swift-ci please test and merge

1 similar comment
@spevans
Copy link
Contributor

spevans commented May 26, 2018

@swift-ci please test and merge

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.

6 participants