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.

Sign up for GitHub

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

Jump to bottom

JSStringCreateWithCFString can now use CFIndex #9542

Merged
merged 1 commit into from Feb 9, 2023
Merged

JSStringCreateWithCFString can now use CFIndex #9542

merged 1 commit into from Feb 9, 2023

Conversation

AtariDreams
Copy link
Contributor

@AtariDreams AtariDreams commented Feb 2, 2023 •
edited by webkit-early-warning-system

115f237

JSStringCreateWithCFString can now use CFIndex
https://bugs.webkit.org/show_bug.cgi?id=251619

<rdar://problem/6806478>
Reviewed by Yusuke Suzuki.

The original size_t workaround came about 14 years ago, right when Apple was still compiling for PowerPC. Basically, so many devices ran on 32 bits.

A long type can hold onto the required length now, so we no longer need to worry about it.

*Source/JavaScriptCore/API/JSStringRefCF.cpp: JSStringRef JSStringCreateWithCFString(CFStringRef string):

Canonical link: https://commits.webkit.org/260083@main

558c9d7

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 🧪 merge ✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@AtariDreams AtariDreams requested a review from a team as a code owner February 2, 2023 19:02
@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 4de6dec)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 98d0245)

Misc iOS, tvOS & watchOS macOS Linux Windows
⏳ 🧪 style ⏳ 🛠 ios ⏳ 🛠 mac ⏳ 🛠 wpe ⏳ 🛠 🧪 win
⏳ 🧪 bindings ⏳ 🛠 ios-sim ⏳ 🛠 mac-AS-debug ⏳ 🛠 gtk ⏳ 🛠 wincairo
⏳ 🧪 webkitperl ⏳ 🧪 ios-wk2 ⏳ 🧪 api-mac ⏳ 🧪 gtk-wk2
⏳ 🧪 api-ios ⏳ 🧪 mac-wk1 ⏳ 🧪 api-gtk
⏳ 🛠 🧪 jsc ⏳ 🛠 tv ⏳ 🧪 mac-wk2 ⏳ 🛠 jsc-armv7
⏳ 🛠 🧪 jsc-arm64 ⏳ 🛠 tv-sim ⏳ 🧪 mac-AS-debug-wk2 ⏳ 🧪 jsc-armv7-tests
⏳ 🧪 services ⏳ 🛠 watch ⏳ 🧪 mac-wk2-stress ⏳ 🛠 jsc-mips
⏳ 🛠 watch-sim ⏳ 🧪 jsc-mips-tests

@Constellation
Copy link
Member

CC @JonWBedard

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 3d4bab3)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ⏳ 🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl   🧪 ios-wk2   🧪 api-mac   🧪 gtk-wk2
  🧪 api-ios   🧪 mac-wk1   🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv   🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch   🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 watch-sim   🧪 jsc-mips-tests

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 3719ff3)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@webkit-early-warning-system
Copy link
Collaborator

EWS run on previous version of this PR (hash 96f6630)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ⏳ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe 💥 🛠 🧪 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk ✅ 🛠 wincairo
✅ 🧪 webkitperl   🧪 ios-wk2 ✅ 🧪 api-mac   🧪 gtk-wk2
  🧪 api-ios ✅ 🧪 mac-wk1   🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv   🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim   🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@webkit-early-warning-system
Copy link
Collaborator

EWS run on current version of this PR (hash 558c9d7)

Misc iOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🛠 gtk
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 gtk-wk2
✅ 🧪 api-ios ✅ 🧪 mac-wk1 ✅ 🧪 api-gtk
✅ 🛠 🧪 jsc ✅ 🛠 tv ✅ 🧪 mac-wk2 ✅ 🛠 jsc-armv7
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 tv-sim ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 jsc-armv7-tests
✅ 🛠 watch ✅ 🧪 mac-wk2-stress ✅ 🛠 jsc-mips
✅ 🛠 🧪 merge ✅ 🛠 watch-sim ✅ 🧪 jsc-mips-tests

@AtariDreams
Copy link
Contributor Author

AtariDreams commented Feb 9, 2023 •
edited

Do you think we can merge this? @emw-apple

@AtariDreams
Copy link
Contributor Author

@Constellation May we please commit this?

@Ahmad-S792 Ahmad-S792 added New Bugs Unclassified bugs are placed in this component until the correct component can be determined. merge-queue Applied to send a pull request to merge-queue labels Feb 9, 2023
@AtariDreams
JSStringCreateWithCFString can now use CFIndex
115f237 
https://bugs.webkit.org/show_bug.cgi?id=251619

<rdar://problem/6806478>
Reviewed by Yusuke Suzuki.

The original size_t workaround came about 14 years ago, right when Apple was still compiling for PowerPC. Basically, so many devices ran on 32 bits.

A long type can hold onto the required length now, so we no longer need to worry about it.

*Source/JavaScriptCore/API/JSStringRefCF.cpp: JSStringRef JSStringCreateWithCFString(CFStringRef string):

Canonical link: https://commits.webkit.org/260083@main
@webkit-commit-queue
Copy link
Collaborator

Committed 260083@main (115f237): https://commits.webkit.org/260083@main

Reviewed commits have been landed. Closing PR #9542 and removing active labels.

@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Feb 9, 2023
@webkit-early-warning-system webkit-early-warning-system merged commit 115f237 into WebKit:main Feb 9, 2023
@AtariDreams AtariDreams deleted the CFIndex branch February 10, 2023 04:32
@Constellation
Copy link
Member

watchOS ARM64_32's CFIndex is 32bit. So we should revert this.

@Constellation
Copy link
Member

Reverted in #9941

@AtariDreams AtariDreams restored the CFIndex branch February 11, 2023 20:15
@AtariDreams AtariDreams deleted the CFIndex branch February 12, 2023 16:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Constellation Constellation Constellation approved these changes

Assignees

@AtariDreams AtariDreams

Labels
New Bugs Unclassified bugs are placed in this component until the correct component can be determined.
Projects
None yet
Milestone
No milestone
5 participants
@AtariDreams @webkit-early-warning-system @Constellation @webkit-commit-queue @Ahmad-S792