1012218 - Cherrypick clang warning fixes from upstream harfbuzz

Status: RESOLVED FIXED

(Core :: Graphics: Text, defect)

Description

[Chris Peterson [:cpeterson]]

Attachment: cherrypick-upstream-harfbuzz-fixes.patch

In bug 984081, I posted a moz.build patch to suppress these harfbuzz warnings:

gfx/harfbuzz/src/hb-common.cc:238:6 [-Wunused-function] unused function 'free_langs'
gfx/harfbuzz/src/hb-private.hh:236:3 [-Wdeprecated-register] 'register' storage class specifier is deprecated
gfx/harfbuzz/src/hb-private.hh:252:3 [-Wdeprecated-register] 'register' storage class specifier is deprecated

Instead of suppressing the warnings in mozilla-central, they were fixed in upstream harfbuzz:

https://github.com/behdad/harfbuzz/commit/0082dbeae6c25a7859960b7e791a540ad04246d9
https://github.com/behdad/harfbuzz/commit/a949cd329e49d2c0ad6f1e023f324790d886dafe

I'd like to cherrypick those trivial fixes to mozilla-central so gfx/harfbuzz is warning-free on OS X, Linux, Android, and Windows:

https://tbpl.mozilla.org/?tree=Try&rev=f9d518d0cc4e

Unfortunately, harfbuzz can't be marked FAIL_ON_WARNINGS (warnings as errors) because the "B2G Desktop OS X Opt" simulator build still has one warning:

gfx/harfbuzz/src/hb-common.cc:346:5: error: expression result unused [-Werror,-Wunused-value]
    hb_atomic_ptr_cmpexch (&default_language, HB_LANGUAGE_INVALID, language);
    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
gfx/harfbuzz/src/hb-atomic-private.hh:126:106: note: expanded from macro 'hb_atomic_ptr_cmpexch'
#define hb_atomic_ptr_cmpexch(P,O,N)    (* (void **) (P) == (void *) (O) ? (* (void **) (P) = (void *) (N), true) : false)
                                                                                                            ^~~~
1 error generated.

Comment 1

[John Daggett (:jtd)]

Comment on attachment 8424409 [details] [diff] [review]
cherrypick-upstream-harfbuzz-fixes.patch

This looks fine but I think Jonathan should take a quick look.

Comment 2

[Jonathan Kew [:jfkthame]]

Comment on attachment 8424409 [details] [diff] [review]
cherrypick-upstream-harfbuzz-fixes.patch

Review of attachment 8424409 [details] [diff] [review]:
-----------------------------------------------------------------

Yes, I guess it's fine to cherry-pick these. They'll be part of our next harfbuzz update anyway, but currently I'm not aware of any great urgency to do that, so in the meantime this is an easy way to reduce warnings.

Comment 3

[Behdad Esfahbod]

I'm going to make a new harfbuzz release today or tomorrow.  There have been a few small shaping fixes, so if new roll is not too much work, I think you can do that.

Comment 4

[Chris Peterson [:cpeterson]]

In the meantime, I landed these trivial fixes which can be overwritten when upstream harfbuzz is next imported:
https://hg.mozilla.org/integration/mozilla-inbound/rev/3d1a7ebc340a

Comment 5

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/3d1a7ebc340a