Skip to content

Remove Vector's non-template span constructor#29955

Merged
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
achristensen07:eng/Remove-Vectors-non-template-span-constructor
Jun 19, 2024
Merged

Remove Vector's non-template span constructor#29955
webkit-commit-queue merged 1 commit intoWebKit:mainfrom
achristensen07:eng/Remove-Vectors-non-template-span-constructor

Conversation

@achristensen07
Copy link
Contributor

@achristensen07 achristensen07 commented Jun 18, 2024

f073c57

Remove Vector's non-template span constructor
https://bugs.webkit.org/show_bug.cgi?id=275638
rdar://130130388

Reviewed by Sam Weinig.

A std::span can be constructed from a range, so when any compiler is using Microsoft's STL
using C++23 and compiles a file that includes IDBKeyData.h, it causes a confusing compiler
error because the std::variant implementation checks std::is_trivially_move_constructible
on all the types, which tries to instantiate the move constructor of each type, which finds
the std::span constructor of Vector that is not a templatized constructor so it is not a
substitution error, it is a compiler error.  The constructor was only there to help with
constructing Vectors from std::arrays, so instead we just add a template constructor that
takes a std::array and makes a std::span from it.  This covers all the uses except 2 in
CryptoKeyOKPCocoa.cpp where we were trying to make a Vector from a uint8[32] so for those
cases we just explicitly make a std::span and 2 in {Audio,Video}EncoderGStreamer.cpp where we need
to explicitly call the std::span constructor to make an optional vector.

I think this is the second to last blocker to us adopting C++23.

* Source/WTF/wtf/Vector.h:
(WTF::Vector::Vector):
* Source/WebCore/crypto/cocoa/CryptoKeyOKPCocoa.cpp:
(WebCore::CryptoKeyOKP::platformGeneratePair):
* Source/WebCore/platform/audio/gstreamer/AudioEncoderGStreamer.cpp:
(WebCore::GStreamerInternalAudioEncoder::GStreamerInternalAudioEncoder):

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

182eafe

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 wincairo
✅ 🧪 bindings ✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug ✅ 🧪 wpe-wk2 ✅ 🧪 wincairo-tests
✅ 🧪 webkitperl ❌ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
❌ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc 🧪 api-ios ✅ 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ⏳ 🛠 vision ✅ 🧪 mac-AS-debug-wk2 ✅ 🧪 gtk-wk2
⏳ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
⏳ 🧪 vision-wk2 🛠 jsc-armv7
✅ 🛠 🧪 unsafe-merge ✅ 🛠 tv 🧪 jsc-armv7-tests
✅ 🛠 tv-sim
✅ 🛠 watch
✅ 🛠 watch-sim

@achristensen07 achristensen07 self-assigned this Jun 18, 2024
@achristensen07 achristensen07 added the WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore). label Jun 18, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 18, 2024
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Jun 19, 2024
@achristensen07 achristensen07 force-pushed the eng/Remove-Vectors-non-template-span-constructor branch from 818639a to 4d93ec5 Compare June 19, 2024 05:27
@achristensen07 achristensen07 requested a review from philn as a code owner June 19, 2024 05:27
@achristensen07 achristensen07 force-pushed the eng/Remove-Vectors-non-template-span-constructor branch from 4d93ec5 to 713336a Compare June 19, 2024 06:10
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 19, 2024
@achristensen07 achristensen07 removed the merging-blocked Applied to prevent a change from being merged label Jun 19, 2024
@achristensen07 achristensen07 force-pushed the eng/Remove-Vectors-non-template-span-constructor branch from 713336a to 182eafe Compare June 19, 2024 06:26
@achristensen07 achristensen07 added the safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks label Jun 19, 2024
@webkit-ews-buildbot webkit-ews-buildbot added the merging-blocked Applied to prevent a change from being merged label Jun 19, 2024
@achristensen07 achristensen07 added unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing and removed merging-blocked Applied to prevent a change from being merged safe-merge-queue Applied to automatically send a pull-request to merge-queue after passing EWS checks labels Jun 19, 2024
https://bugs.webkit.org/show_bug.cgi?id=275638
rdar://130130388

Reviewed by Sam Weinig.

A std::span can be constructed from a range, so when any compiler is using Microsoft's STL
using C++23 and compiles a file that includes IDBKeyData.h, it causes a confusing compiler
error because the std::variant implementation checks std::is_trivially_move_constructible
on all the types, which tries to instantiate the move constructor of each type, which finds
the std::span constructor of Vector that is not a templatized constructor so it is not a
substitution error, it is a compiler error.  The constructor was only there to help with
constructing Vectors from std::arrays, so instead we just add a template constructor that
takes a std::array and makes a std::span from it.  This covers all the uses except 2 in
CryptoKeyOKPCocoa.cpp where we were trying to make a Vector from a uint8[32] so for those
cases we just explicitly make a std::span and 2 in {Audio,Video}EncoderGStreamer.cpp where we need
to explicitly call the std::span constructor to make an optional vector.

I think this is the second to last blocker to us adopting C++23.

* Source/WTF/wtf/Vector.h:
(WTF::Vector::Vector):
* Source/WebCore/crypto/cocoa/CryptoKeyOKPCocoa.cpp:
(WebCore::CryptoKeyOKP::platformGeneratePair):
* Source/WebCore/platform/audio/gstreamer/AudioEncoderGStreamer.cpp:
(WebCore::GStreamerInternalAudioEncoder::GStreamerInternalAudioEncoder):

Canonical link: https://commits.webkit.org/280183@main
@webkit-commit-queue webkit-commit-queue force-pushed the eng/Remove-Vectors-non-template-span-constructor branch from 182eafe to f073c57 Compare June 19, 2024 18:19
@webkit-commit-queue
Copy link
Collaborator

Committed 280183@main (f073c57): https://commits.webkit.org/280183@main

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

@webkit-commit-queue webkit-commit-queue merged commit f073c57 into WebKit:main Jun 19, 2024
@webkit-commit-queue webkit-commit-queue removed the unsafe-merge-queue Applied to send a pull request to merge-queue, but skip building and testing label Jun 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

WebKit Misc. For miscellaneous bugs in the WebKit framework (and not JavaScriptCore or WebCore).

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

Comments