[webkit-reviews] review granted: [Bug 54446] Share the helper functions used by Latin-1 and UTF-8 text codecs : [Attachment 82469] Proposed patch v2

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Andreas Kling <kling at webkit.org>'s
request for review:
Bug 54446: Share the helper functions used by Latin-1 and UTF-8 text codecs
https://bugs.webkit.org/show_bug.cgi?id=54446

Attachment 82469: Proposed patch v2
https://bugs.webkit.org/attachment.cgi?id=82469&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=82469&action=review

r=me -- also had some ideas for further cleanup

> Source/WebCore/platform/text/TextCodecLatin1.cpp:137
> -		       if (chunk & NonASCIIMask<sizeof(uintptr_t)>::value())
> +		       if (chunk & NonASCIIMask<sizeof(MachineWord)>::value())
>			   goto useLookupTable;

I realized we can make this even cleaner by defining a function in the header:

    inline MachineWord NonASCIIMachineWordMask()
    {
	return NonASCIIMask<sizeof(MachineWord)>::value();
    }

Or:

    inline bool isAllASCII(MachineWord word)
    {
	return word & NonASCIIMask<sizeof(MachineWord)>::value();
    }

And use one of these functions in the files. Then the template magic stays in
the header.

> Source/WebCore/platform/text/TextCodecLatin1.cpp:139
> +		       UCharByteFiller<sizeof(MachineWord)>::copy(destination,
source);

And here:

    inline void copyASCIIMachineWord(UChar* destination, const uint8_t* source)

    {
	UCharByteFiller<sizeof(MachineWord)>::copy(destination, source);
    }

Or the type of source could be const MachineWord* and we could cast it.

It’s nice to keep the template goop in the header.