1472806 - fix some clang-cl warnings for Windows-specific code

Status: RESOLVED FIXED

(Core :: General, enhancement)

Description

[Nathan Froyd [:froydnj]]

We mistakenly use some MSVC extensions, which clang-cl warns about, and those
warnings make the build noisy.  Let's fix that.

Comment 1

[Nathan Froyd [:froydnj]]

Attachment: fix -Wpessimizing-move warnings in Interceptor.cpp

There's no need to invoke std::move here, because Get() is already
returning a temporary that can be moved into the RefPtr.

Comment 2

[Nathan Froyd [:froydnj]]

Attachment: fix microsoft template lookup extensions in interceptor code

clang-cl complains about things like:

z:/build/build/src/obj-firefox/dist/include/mozilla/interceptor/VMSharingPolicies.h(53,50):  warning: use of identifier 'GetLocalView' found via unqualified lookup into dependent bases of class templates is a Microsoft extension [-Wmicrosoft-template]
      return TrampolineCollection<MMPolicy>(*this, GetLocalView(), GetRemoteView(),
                                                   ^

in various files in interceptor/, and since the warnings are in headers,
rather than in sources, they're rather annoying.  Let's fix this to be
standards-complaint and make clang-cl stop complaining.

Comment 3

[Nathan Froyd [:froydnj]]

Attachment: fix missing typename warning in COMPtrHolder.h

MSVC permits the missing `typename` as an extension, whereas clang-cl warns.
This is easy to fix, so let's fix the warning noise.

Comment 4

[(No longer employed by Mozilla) Aaron Klotz]

Thanks

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/b8e6e7b84ce5
fix -Wpessimizing-move warnings in Interceptor.cpp; r=aklotz
https://hg.mozilla.org/integration/mozilla-inbound/rev/53e44a1beb93
fix microsoft template lookup extensions in interceptor code; r=aklotz
https://hg.mozilla.org/integration/mozilla-inbound/rev/be73fc773100
fix missing typename warning in COMPtrHolder.h; r=aklotz

Comment 6

[Raul Gurzau (:RaulG)]

https://hg.mozilla.org/mozilla-central/rev/b8e6e7b84ce5
https://hg.mozilla.org/mozilla-central/rev/53e44a1beb93
https://hg.mozilla.org/mozilla-central/rev/be73fc773100

Comment 7

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Comment on attachment 8989264 [details] [diff] [review]
fix missing typename warning in COMPtrHolder.h

.

Comment 8

[Tom Ritter [:tjr] (OOTO until 4/30 at least)]

Attachment: fix missing typename warning in COMPtrHolder.h (esr60)

This is a rebased version of the original patch that includes an additional site to fix.  I'm taking the r+ from the original patch.

[ESR Uplift Approval Request]

If this is not a sec:{high,crit} bug, please state case for ESR consideration: Needed for MinGW Job.

User impact if declined: We won't be able to land the MinGW build job in the configuration Tor uses

Fix Landed on Version: 20180704

Risk to taking this patch: Low

Why is the change risky/not risky? (and alternatives if risky): It's just a compiler warning fix

String or UUID changes made by this patch:

Comment 9

[Ryan VanderMeulen [:RyanVM]]

Comment on attachment 9031027 [details] [diff] [review]
fix missing typename warning in COMPtrHolder.h (esr60)

Build fixes needed for Tor. Approved for ESR60.

Comment 10

[Ryan VanderMeulen [:RyanVM]]

This was pushed, but I noticed afterwards that the patch had a different bug number on it.
https://hg.mozilla.org/releases/mozilla-esr60/rev/aaa9d1093d37