Closed Bug 1489944 Opened Last year Closed Last year

Fix some std::move-related warnings

Categories

(Core :: General, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: gerald, Assigned: gerald)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

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().
Please check if AllowCompilerWarnings() is still required in moz.build after the fix.
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...
> 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 on attachment 9007766 [details]
Bug 1489944 - Fixed some std::move warnings - r?froydnj

Nathan Froyd [:froydnj] has approved the revision.
Attachment #9007766 - Flags: review+
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/510225640144
Fixed some std::move warnings - r=froydnj
https://hg.mozilla.org/mozilla-central/rev/510225640144
Status: NEW → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
You need to log in before you can comment on or make changes to this bug.