1489944 - Fix some std::move-related warnings

Status: RESOLVED FIXED

(Core :: General, defect)

Description

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Fixing some more std::move warnings noticed during a local clang/win build -- i.e., not exhaustive.

> dom/media/gmp/CDMStorageIdProvider.cpp(63,10):  warning:
> local variable 'storageId' will be copied despite being returned by name [-Wreturn-std-move]
>   return storageId;
nsAutoCString -> nsCString, will add std::move().

> layout/painting/DisplayItemClip.cpp(581,10):  warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
>   return str;
nsAutoCString -> nsCString, will add std::move().

> layout/painting/DisplayItemClipChain.cpp(88,10):  warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
>   return str;
nsAutoCString -> nsCString, will add std::move().

> layout/painting/nsDisplayList.cpp(179,10):  warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
>   return str;
nsAutoCString -> nsCString, will add std::move().

> gfx/thebes/gfxWindowsPlatform.cpp(454,10):  warning:
> moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
>   return std::move(data);
Will remove std::move().

> gfx/thebes/gfxFontEntry.cpp(245,20):  warning:
> local variable 'name' will be copied despite being returned by name [-Wreturn-std-move]
>             return name;
nsAutoCString -> nsCString, will add std::move().

> netwerk/cookie/nsCookieService.cpp(4460,10):  warning:
> local variable 'path' will be copied despite being returned by name [-Wreturn-std-move]
>   return path;
GetPathFromURI() result is stored in an nsAutoCString, so it might as well return that type.

> toolkit/components/extensions/WebExtensionPolicy.cpp(462,12):  warning:
> local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
>     return result;
> toolkit/components/extensions/WebExtensionPolicy.cpp(475,10):  warning:
> local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
>   return result;
`result` may be empty or may be arbitrarily long, so I'll use nsCString inside the function.

> toolkit/xre/CmdLineAndEnvUtils.h(349,10):  warning:
> moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
>   return std::move(s);
Returning an UniquePtr, will remove std::move().
Also will `return s` instead of `return nullptr` when `(!s)`, to avoid extra construction which could also prevent elision (not entirely sure, but it's at least not worse!); and it's clearer that the two `return`s return the same already-constructed on-stack object.

> tools/profiler/core/shared-libraries-win32.cc(111,10):  warning:
> local variable 'version' will be copied despite being returned by name [-Wreturn-std-move]
>   return version;
nsPrintfCString -> nsCString, will add std::move().

> xpcom/glue/FileUtils.cpp(179,10):  warning:
> local variable 'fullName' will be copied despite being returned by name [-Wreturn-std-move]
>   return fullName;
> xpcom/glue/FileUtils.cpp(209,10):  warning:
> local variable 'path' will be copied despite being returned by name [-Wreturn-std-move]
>   return path;
nsAuto{,C}String -> ns{,C}String, will add std::move().

Comment 1

[Masatoshi Kimura [:emk]]

Please check if AllowCompilerWarnings() is still required in moz.build after the fix.

Comment 2

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Oh, that's what you did in bug 1489664 after bug 1488701, now I see. Thanks!
Ok, I will check for that in the affected directories...

Comment 3

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Attachment: Bug 1489944 - Fixed some std::move warnings - r?froydnj

> dom/media/gmp/CDMStorageIdProvider.cpp(63,10):  warning:
> local variable 'storageId' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().

> layout/painting/DisplayItemClip.cpp(581,10):  warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().

> layout/painting/DisplayItemClipChain.cpp(88,10):  warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().

> layout/painting/nsDisplayList.cpp(179,10):  warning:
> local variable 'str' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().

> gfx/thebes/gfxWindowsPlatform.cpp(454,10):  warning:
> moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
Will remove std::move().

> gfx/thebes/gfxFontEntry.cpp(245,20):  warning:
> local variable 'name' will be copied despite being returned by name [-Wreturn-std-move]
nsAutoCString -> nsCString, will add std::move().

> netwerk/cookie/nsCookieService.cpp(4460,10):  warning:
> local variable 'path' will be copied despite being returned by name [-Wreturn-std-move]
GetPathFromURI() result is stored in an nsAutoCString, so it might as well return that type.

> toolkit/components/extensions/WebExtensionPolicy.cpp(462,12):  warning:
> local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
> toolkit/components/extensions/WebExtensionPolicy.cpp(475,10):  warning:
> local variable 'result' will be copied despite being returned by name [-Wreturn-std-move]
`result` may be empty or may be arbitrarily long, so I'll use nsCString inside the function.

> toolkit/xre/CmdLineAndEnvUtils.h(349,10):  warning:
> moving a local object in a return statement prevents copy elision [-Wpessimizing-move]
Returning an UniquePtr, will remove std::move().
Also will `return s` instead of `return nullptr` when `(!s)`, to avoid extra construction which could also prevent elision (not entirely sure, but it's at least not worse!); and it's clearer that the two `return`s return the same already-constructed on-stack object.

> tools/profiler/core/shared-libraries-win32.cc(111,10):  warning:
> local variable 'version' will be copied despite being returned by name [-Wreturn-std-move]
nsPrintfCString -> nsCString, will add std::move().

> xpcom/glue/FileUtils.cpp(179,10):  warning:
> local variable 'fullName' will be copied despite being returned by name [-Wreturn-std-move]
> xpcom/glue/FileUtils.cpp(209,10):  warning:
> local variable 'path' will be copied despite being returned by name [-Wreturn-std-move]
nsAuto{,C}String -> ns{,C}String, will add std::move().

This allowed removals of 'AllowCompilerWarnings' from layout/painting,
netwerk/cookie, and toolkit/components/extensions.

Comment 4

[Nathan Froyd [:froydnj]]

Comment on attachment 9007766 [details]
Bug 1489944 - Fixed some std::move warnings - r?froydnj

Nathan Froyd [:froydnj] has approved the revision.

Comment 5

[Gerald Squelart (he/him) (not at Mozilla since 2022-09-15)]

Thank you Nathan for the review.

Try: https://treeherder.mozilla.org/#/jobs?repo=try&revision=4184ebc2de096d6e18278061ba8b713616d2863c

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/510225640144
Fixed some std::move warnings - r=froydnj

Comment 7

[Cosmin Sabou [:CosminS]]

https://hg.mozilla.org/mozilla-central/rev/510225640144