Bug 231980 – The code decoding std::optional<ImagePaintingOptions> can't be compiled by PlayStation due to the ImagePaintingOptions template constructor

• Component: WebKit2
• Status: RESOLVED FIXED

[ujwal koneru]

Created attachment 441787 [details]
Patch for templatized constructor compilation issues

https://build.webkit.org/#/builders/65/builds/8136/steps/8/logs/errors

Error in compiling message with struct ImagePaintingOptions(https://github.com/WebKit/WebKit/blob/b78686d4f9686797562e0876902f30bf66b610cd/Source/WebCore/platform/graphics/ImagePaintingOptions.h)

[ujwal koneru]

Created attachment 441814 [details]
Patch for build fix

Fixed the style and changelog.

[Ross Kirsling]

This is a belated fix for r283941.

[Ross Kirsling]

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.

[Fujii Hironori]

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?

[ujwal koneru]

Fujii-san, your version looks better to me.

Just baffled why this would work and not the original code.

[Fujii Hironori]

Created attachment 441926 [details]
Patch

[Ross Kirsling]

Comment on attachment 441926 [details]
Patch

Nice!

[Fujii Hironori]

Comment on attachment 441926 [details]
Patch

Clearing flags on attachment: 441926

Committed r284566 (243308@main): <https://commits.webkit.org/243308@main>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.

[Radar WebKit Bug Importer]

<rdar://problem/84478389>

[Fujii Hironori]

Still failing after the change. Reopned.
https://build.webkit.org/#/builders/42/builds/8221

[Fujii Hironori]

Created attachment 442101 [details]
compile error with -ftemplate-backtrace-limit=0

[Fujii Hironori]

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.

[Fujii Hironori]

Created attachment 442123 [details]
Patch

[Ross Kirsling]

Hmm, now it seems like Ujwal's original patch might be less complicated...

[Fujii Hironori]

(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.

[Fujii Hironori]

(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]

[Darin Adler]

Comment on attachment 442123 [details]
Patch

There must be a more elegant fix. But this is OK with me for now.

[Fujii Hironori]

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.

[Fujii Hironori]

(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.

[Fujii Hironori]

Comment on attachment 442123 [details]
Patch

Clearing flags on attachment: 442123

Committed r284767 (243476@main): <https://commits.webkit.org/243476@main>

[Fujii Hironori]

All reviewed patches have been landed.  Closing bug.