[webkit-reviews] review granted: [Bug 235004] Add helpers to access CoreGraphics color spaces more easily in generic contexts : [Attachment 448694] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Sam Weinig <sam at webkit.org>'s request
for review:
Bug 235004: Add helpers to access CoreGraphics color spaces more easily in
generic contexts
https://bugs.webkit.org/show_bug.cgi?id=235004

Attachment 448694: Patch

https://bugs.webkit.org/attachment.cgi?id=448694&action=review




--- Comment #3 from Darin Adler <darin at apple.com> ---
Comment on attachment 448694
  --> https://bugs.webkit.org/attachment.cgi?id=448694
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=448694&action=review

> Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:141
> +    template<ColorSpace C> static auto test(int) ->
TrueIfDefined<decltype(CGColorSpaceMapping<C>::colorSpace())>;

Consider "space" or "colorSpace" instead of C? Here and just below.

> Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:142
> +    template<ColorSpace C> static auto test(long) -> std::false_type;

Omit "C" here.

> Source/WebCore/platform/graphics/cg/ColorSpaceCG.h:145
> +template<ColorSpace C> inline constexpr bool HasCGColorSpaceMapping =
decltype(HasCGColorSpaceMappingHelper::test<C>(0))();

This looks more complicated than what ColorTypes.h uses for
HasDescriptorMember, etc. And HasBeginFunctionMember in Hasher.h is what I came
up with when I needed to solve a similar problem. I am not sure if the problems
are close enough to the same, but I am surprised we need more code here.

HasModernDecoder in ArgumentCoder.h is more complex, closer to what we have
here.

Would like it if the idioms were similar for cases like these, and I don’t
understand what leads to this more complex thing.