SandboxExtension::Handle creation should return std::optional instead of bool
Created attachment 435089 [details] Patch
Nice!
EWS is all red.
Created attachment 435452 [details] Patch
Created attachment 435460 [details] Patch
Comment on attachment 435460 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435460&action=review > Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm:298 > +static SandboxExtension::HandleArray createHandlesForResources(const Vector<T>& resources, Function<std::optional<SandboxExtension::Handle>(const T&)>&& createFunction) Does not seem we need a Function<>&& here. > Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm:319 > + return createHandlesForResources(paths, Function<std::optional<Handle>(const String&)>([&logLabel] (const String& path) { Would it work with: return createHandlesForResources<String>(paths, [&logLabel](auto& path) { ... }); > Source/WebKit/Shared/Cocoa/SandboxExtensionCocoa.mm:399 > + return createHandlesForResources(services, Function<std::optional<Handle>(const ASCIILiteral&)>([auditToken, flags] (const ASCIILiteral& service) -> std::optional<Handle> { Ditto here, createHandlesForResources<ASCIILiteral>(...) ? Maybe below as well
Created attachment 435502 [details] Patch
Committed r281032 (240512@main): <https://commits.webkit.org/240512@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 435502 [details].
<rdar://problem/81914833>
Comment on attachment 435502 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435502&action=review > Source/WebKit/Shared/SandboxExtension.h:169 > +inline auto SandboxExtension::createHandleForTemporaryFile(const String& /*prefix*/, Type) -> std::optional<std::pair<Handle, String>> { return std::optional<std::pair<Handle, String>> { std::pair<Handle, String> { Handle { }, String { } } }; } Pretty sure we can omit "Handle { }, String { }".
Perhaps. The compilers were unable to infer the meaning of {{{ }, { }}} so I was just completely explicit.
(In reply to Alex Christensen from comment #11) > Perhaps. The compilers were unable to infer the meaning of {{{ }, { }}} so > I was just completely explicit. I’m really puzzled by this. I can’t reproduce this problem with a test program.
Just leaving #229233 as a reference here, the std::optional<Handle> retval construction triggered compilation errors on older GCC setups due to ambiguity between the desired constructor and the in-place constructor of std::optional.