Closed Bug 1012218 Opened 6 years ago Closed 6 years ago

Cherrypick clang warning fixes from upstream harfbuzz

Categories

(Core :: Graphics: Text, defect)

x86
macOS
defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32
Tracking Status
firefox31 --- wontfix
firefox32 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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.
Attachment #8424409 - Flags: review?(jdaggett)
Comment on attachment 8424409 [details] [diff] [review]
cherrypick-upstream-harfbuzz-fixes.patch

This looks fine but I think Jonathan should take a quick look.
Attachment #8424409 - Flags: review?(jdaggett) → review?(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.
Attachment #8424409 - Flags: review?(jfkthame) → review+
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.
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
https://hg.mozilla.org/mozilla-central/rev/3d1a7ebc340a
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
You need to log in before you can comment on or make changes to this bug.