Closed Bug 984073 Opened 6 years ago Closed 6 years ago

Suppress clang and gcc warnings in MFBT's third-party code: double-conversion and lz4

Categories

(Core :: MFBT, defect, P4)

defect

Tracking

()

RESOLVED FIXED
mozilla31
Tracking Status
firefox30 --- wontfix
firefox31 --- fixed

People

(Reporter: cpeterson, Assigned: cpeterson)

References

(Depends on 1 open bug, Blocks 1 open bug)

Details

(Whiteboard: [qa-])

Attachments

(1 file, 2 obsolete files)

Attached patch suppress-mfbt-warnings.patch (obsolete) — Splinter Review
clang and gcc on OS X and Linux report the following warnings in MFBT's third-party code. Unfortunately, this mfbt/moz.build change also suppresses -Wunused-function and -Wunused-variable warnings in Mozilla's MFBT source files, too. Is it preferable to patch the third-party files to fix these warnings than to suppress these warnings in Mozilla source files?

mfbt/double-conversion/strtod.cc:509:9 [-Wunused-variable] unused variable 'f2'

mfbt/lz4.c:406:7 [-Wunused-function] unused function 'LZ4_create'
mfbt/lz4.c:407:7 [-Wunused-function] unused function 'LZ4_free'
mfbt/lz4_encoder.h:75:5 [-Wunused-function] unused function 'LZ4_compress_heap'
mfbt/lz4_encoder.h:75:5 [-Wunused-function] unused function 'LZ4_compress64k_heap'
mfbt/lz4_encoder.h:75:5 [-Wunused-function] unused function 'LZ4_compress_heap_limitedOutput'
mfbt/lz4_encoder.h:75:5 [-Wunused-function] unused function 'LZ4_compress64k_heap_limitedOutput'
Attachment #8391820 - Flags: review?(jwalden+bmo)
Attached patch suppress-mfbt-warnings-v2.patch (obsolete) — Splinter Review
Instead of suppressing -Wunused-function and -Wunused-variable warnings in all MFBT source files, only suppress these warnings in the two guilty third-party files.
Attachment #8391820 - Attachment is obsolete: true
Attachment #8391820 - Flags: review?(jwalden+bmo)
Attachment #8401689 - Flags: review?(jwalden+bmo)
Comment on attachment 8401689 [details] [diff] [review]
suppress-mfbt-warnings-v2.patch

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

Patching the files seems reasonable here.  We already have an import/update script for the decimal-conversion bits, that can be simply amended to apply extra changes.  No such thing for the lz4 stuff yet, as far as I remember.  Let's do the warning-flag-addition for now for that and file a followup to make the lz4 import more regular (put it in a subdirectory with an update script and post-update patch applications).
Attachment #8401689 - Flags: review?(jwalden+bmo)
Depends on: 993267
patch v3:
1. Fix double-conversion warning with update.sh patch.
2. Suppress gcc warning in LZ4.
3. Filed bug 993267 about moving LZ4 to its own subdirectory with an update.sh.
Attachment #8401689 - Attachment is obsolete: true
Attachment #8403074 - Flags: review?(jwalden+bmo)
Attachment #8403074 - Flags: review?(jwalden+bmo) → review+
https://hg.mozilla.org/mozilla-central/rev/a2c2c5ed7e9b
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla31
Whiteboard: [qa-]
Depends on: 1202568
You need to log in before you can comment on or make changes to this bug.