1478000 - nICEr and nrappkit should use the same warnings under clang-cl as with clang

Status: RESOLVED FIXED

(Core :: WebRTC: Networking, enhancement, P3)

Description

[(Away)]

nICEr and nrappkit are quite noisy under clang-cl, mostly because of 520 -Wparentheses complaints that are disabled on Linux and Mac.

https://searchfox.org/mozilla-central/rev/d160ac574a5798010eda4e33e91ee628aa268a33/media/mtransport/third_party/nICEr/nicer.gyp#209

and

https://searchfox.org/mozilla-central/rev/d160ac574a5798010eda4e33e91ee628aa268a33/media/mtransport/third_party/nrappkit/nrappkit.gyp#206

should be used in clang-cl as well.

I don't know how to say "if clang-cl" in gyp though...

Comment 1

[Nils Ohlmeier [:drno]]

Dan do you happen to know how to check for clang-cl in gyp?

Comment 2

[Dan Minor [:dminor]]

It looks like we set a clang variable here: https://searchfox.org/mozilla-central/source/build/gyp.mozbuild#28. I'll have a look.

Comment 3

[Dan Minor [:dminor]]

Attachment: Bug 1478000 - nICEr and nrappkit should use the same warnings under clang-cl as with clang; r=dmajor

This adds a clangcl flag to gyp.mozbuild and then uses that to set the same
warnings flags for clang-cl as for clang.

Comment 4

[Dan Minor [:dminor]]

Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=925464261bfa3f1c24c8ca221d1b05de7e9ea36f

With these changes we go from 334 hits for "place parentheses" in the build log to 75.

Comment 5

[Phabricator Automation]

Comment on attachment 8995183 [details]
Bug 1478000 - nICEr and nrappkit should use the same warnings under clang-cl as with clang; r=dmajor

David Major [:dmajor] has approved the revision.

https://phabricator.services.mozilla.com/D2427

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/22c01bee854b
nICEr and nrappkit should use the same warnings under clang-cl as with clang; r=dmajor

Comment 7

[Masatoshi Kimura [:emk]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/22c01bee854b#l2.15
>+              # Windows, clang-cl build
>+              [ 'clang_cl == 1', {
>+                'cflags_mozilla': [
>+                    '-Wall',

Please prepend `-Xclang` to `-Wall`. Otherwise clang-cl will translate `-Wall` into `-Weverything`.

Comment 8

[Dorel Luca [:dluca]]

https://hg.mozilla.org/mozilla-central/rev/22c01bee854b

Comment 9

[Dan Minor [:dminor]]

Attachment: Bug 1478000 - Prepend -Xclang to -Wall; r=dmajor

Apparently without -Xclang, -Wall is interpreted as -Weverything by clang-cl.

Comment 10

[Dan Minor [:dminor]]

Comparing try jobs it looks like my original patch did increase the number of warnings reported by Perfherder from 1105 to 1390 . With the fix suggested by :emk this is reduced to 969. Thanks!

Comment 11

[Phabricator Automation]

Comment on attachment 8995528 [details]
Bug 1478000 - Prepend -Xclang to -Wall; r=dmajor

David Major [:dmajor] has approved the revision.

https://phabricator.services.mozilla.com/D2468

Comment 12

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/5236d5fadfa0
Prepend -Xclang to -Wall; r=dmajor

Comment 13

[Daniel Varga [:dvarga]]

https://hg.mozilla.org/mozilla-central/rev/5236d5fadfa0