Summary: | The code decoding std::optional<ImagePaintingOptions> can't be compiled by PlayStation due to the ImagePaintingOptions template constructor | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | ujwal koneru <ujwal.koneru> | ||||||||||||||
Component: | WebKit2 | Assignee: | ujwal koneru <ujwal.koneru> | ||||||||||||||
Status: | RESOLVED FIXED | ||||||||||||||||
Severity: | Normal | CC: | darin, don.olmstead, Hironori.Fujii, kkinnunen, ross.kirsling, sabouhallawa, simon.fraser, ujwal.koneru, webkit-bug-importer, wenson_hsieh | ||||||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||||||
Version: | WebKit Nightly Build | ||||||||||||||||
Hardware: | Other | ||||||||||||||||
OS: | Other | ||||||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=231484 | ||||||||||||||||
Attachments: |
|
Description
ujwal koneru
2021-10-19 13:51:16 PDT
Created attachment 441814 [details]
Patch for build fix
Fixed the style and changelog.
Comment on attachment 441814 [details] Patch for build fix View in context: https://bugs.webkit.org/attachment.cgi?id=441814&action=review > Source/WebCore/ChangeLog:11 > + https://bugs.webkit.org/show_bug.cgi?id=231980 > + > + Reviewed by NOBODY (OOPS!). The explanation for the patch should come after these lines. Created attachment 441852 [details]
WIP patch
I don't know what's going on in the compiler, but the compiler can compile this patch. WDYT?
Fujii-san, your version looks better to me. Just baffled why this would work and not the original code. Created attachment 441926 [details]
Patch
Comment on attachment 441926 [details]
Patch
Nice!
Comment on attachment 441926 [details] Patch Clearing flags on attachment: 441926 Committed r284566 (243308@main): <https://commits.webkit.org/243308@main> All reviewed patches have been landed. Closing bug. Still failing after the change. Reopned. https://build.webkit.org/#/builders/42/builds/8221 Created attachment 442101 [details]
compile error with -ftemplate-backtrace-limit=0
I'd like to do something like the following, but didn't succeed. > template <typename Type> static constexpr bool isOptionType = std::is_same_v<decltype(std::declval<ImagePaintingOptions>().setOption(std::declval<std::decay_t<Type>>())), void>; > > template<typename First, typename... Rest, typename = std::enable_if_t<isOptionType<First>>> > ImagePaintingOptions(First first, Rest... rest) https://stackoverflow.com/q/59268030 Clang can't compile the code. Created attachment 442123 [details]
Patch
Hmm, now it seems like Ujwal's original patch might be less complicated... (In reply to Ross Kirsling from comment #15) > Hmm, now it seems like Ujwal's original patch might be less complicated... It doesn't take multiple ImagePaintingOptions. Can it compile? I didn't try. (In reply to Fujii Hironori from comment #16) > It doesn't take multiple ImagePaintingOptions. Can it compile? I didn't try. Oh, EWS were green. attachment#441814 [details] Comment on attachment 442123 [details]
Patch
There must be a more elegant fix. But this is OK with me for now.
Comment on attachment 442123 [details]
Patch
Thank you very much for the review. But, I have a new idea using explicit keyword now. I’ll try it befor landing this patch Monday.
(In reply to Fujii Hironori from comment #19) > I have a new idea using explicit keyword now. There are a lot of implicit construction, and it doesn't solve the PlayStation issue. I have no more ideas, I'm going to land comment#14 patch. Comment on attachment 442123 [details] Patch Clearing flags on attachment: 442123 Committed r284767 (243476@main): <https://commits.webkit.org/243476@main> All reviewed patches have been landed. Closing bug. |