1238635 - don't warn on self-assignment for android support code

Status: RESOLVED INCOMPLETE

(Firefox Build System :: General, defect)

Description

[Nathan Froyd [:froydnj]]

clang turns this warning on by default, which interferes with using
clang on Android.  Just disable the warning to avoid cluttering up the
logs.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: don't warn on self-assignment for android support code

Comment 2

[Michael Shal [:mshal]]

Comment on attachment 8706619 [details] [diff] [review]
don't warn on self-assignment for android support code

> # We allow warnings for third-party code that can be updated from upstream.
> ALLOW_COMPILER_WARNINGS = True

It might be helpful to move the new flags here, after the ALLOW_COMPILER_WARNINGS line, along with a comment explaining why we need to turn it off.

Though from https://bugzilla.mozilla.org/show_bug.cgi?id=1215903#c1 it sounds like the intention is to keep all the warnings and fix them upstream for cases like this. :njn, your thoughts?

>+
>+if CONFIG['CLANG_CXX']:
>+    CFLAGS += [
>+        '-Wno-self-assign',
>+    ]
>

Comment 3

[Nicholas Nethercote [inactive]]

> Though from https://bugzilla.mozilla.org/show_bug.cgi?id=1215903#c1 it
> sounds like the intention is to keep all the warnings and fix them upstream
> for cases like this. :njn, your thoughts?

In the past there have been cases (like that bug) where we enabled fatal warnings for third-party code directories and then dealt with bustage by turning off individual warnings, which is a losing strategy -- any time you update the third-party code you're at risk of causing build bustage. So the intention is that ALLOW_COMPILER_WARNINGS be used for third-party code to make warnings non-fatal and thus avoid this problem.

Having said that, if one particular kind of warning is frequent in third-party code I think it's fine to specifically disable it to avoid lots of output. (I did this myself with -Wunused in some third-party directories recently.) And if warnings get fixed upstream, that's a bonus, but we certainly shouldn't rely on that.

Comment 4

[Michael Shal [:mshal]]

Ahh, makes sense. Thanks!

Comment 5

[Nathan Froyd [:froydnj]]

other-licenses/android/ was removed when we dropped support for older android versions.