Closed Bug 1488701 Opened 6 years ago Closed 6 years ago

Remove a few std::move's that prevent copy elision

Categories

(Core :: General, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla64
Tracking Status
firefox64 --- fixed

People

(Reporter: mozbugz, Assigned: mozbugz)

References

Details

Attachments

(1 file)

Doing std::move when returning a temporary or local object prevents C++ from performing copy elision.
Doing std::move when returning/assigning a local or temporary object is
preventing the compiler from performing copy elision.

In the case of UniquePtr, it's easier to `return MakeUnique<T>(...)` instead of
the move verbose `UniquePtr<T> r(new T(...)); return r;`
Comment on attachment 9006513 [details]
Bug 1488701 - Remove misleading std::move's - r?froydnj

Nathan Froyd [:froydnj] has approved the revision.
Attachment #9006513 - Flags: review+
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/2e439c0a6934
Remove misleading std::move's - r=froydnj
Backed out changeset 2e439c0a6934 (bug 1488701) for build bustage. CLOSED TREE

Log:
z:/build/build/src/obj-firefox/dist/include\mozilla/UniquePtr.h(680,27):  error: calling a protected constructor of class 'mozilla::gl::SurfaceFactory_D3D11Interop'
12:03:57     INFO -    return UniquePtr<T>(new T(std::forward<Args>(aArgs)...));
12:03:57     INFO -                            ^
12:03:57     INFO -  z:/build/build/src/gfx/gl/SharedSurfaceD3D11Interop.cpp(516,12):  note: in instantiation of function template specialization 'mozilla::MakeUnique<mozilla::gl::SurfaceFactory_D3D11Interop, mozilla::gl::GLContext *&, const mozilla::gl::SurfaceCaps &, mozilla::layers::LayersIPCChannel *&, const mozilla::layers::TextureFlags &, const RefPtr<mozilla::gl::DXInterop2Device> &>' requested here
12:03:57     INFO -      return MakeUnique<SurfaceFactory_D3D11Interop>(
12:03:57     INFO -             ^
12:03:57     INFO -  z:/build/build/src/gfx/gl/SharedSurfaceD3D11Interop.h(92,5):  note: declared protected here
12:03:57     INFO -      SurfaceFactory_D3D11Interop(GLContext* gl, const SurfaceCaps& caps,
12:03:57     INFO -      ^
12:03:57     INFO -  4 errors generated.
12:03:57     INFO -  z:/build/build/src/config/rules.mk:1110: recipe for target 'Unified_cpp_gfx_gl1.obj' failed
12:03:57     INFO -  mozmake.EXE[4]: *** [Unified_cpp_gfx_gl1.obj] Error 1
12:03:57     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/gl'
12:03:57     INFO -  mozmake.EXE[4]: *** Waiting for unfinished jobs....
12:03:57     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/graphite2/src'
12:03:57     INFO -  mozmake.EXE[4]: Leaving directory 'z:/build/build/src/obj-firefox/gfx/graphite2/src'
12:03:57     INFO -  mozmake.EXE[4]: Entering directory 'z:/build/build/src/obj-firefox/gfx/cairo/cairo/src'
12:03:57     INFO -  z:/build/build/src/sccache2/sccache.exe z:/build/build/src/clang/bin/clang-cl.exe -fms-compatibility-version=19.13.26128 -m32 -Focairo-d2d-surface.obj -c  -Iz:/build/build/src/obj-firefox/dist/stl_wrappers -DDEBUG=1 -DDISABLE_SOME_FLOATING_POINT -DCAIRO_WIN32_STATIC_BUILD '-DPACKAGE_VERSION="moz"' '-DPACKAGE_BUGREPORT="http://bugzilla.mozilla.org/"' -DCAIRO_HAS_PTHREAD -D_GNU_SOURCE -DMOZ_TREE_CAIRO -DMOZ_TREE_PIXMAN -DHAVE_UINT64_T -DSTATIC_EXPORTABLE_JS_API -DMOZ_HAS_MOZGLUE -DMOZILLA_INTERNAL_API -DIMPL_LIBXUL -Iz:/build/build/src/gfx/cairo/cairo/src -Iz:/build/build/src/obj-firefox/gfx/cairo/cairo/src -Iz:/build/build/src/obj-firefox/dist/include -Iz:/build/build/src/obj-firefox/dist/include/nspr -Iz:/build/build/src/obj-firefox/dist/include/nss -MD -FI z:/build/build/src/obj-firefox/mozilla-config.h -DMOZILLA_CLIENT -Qunused-arguments -Qunused-arguments -TP -nologo -w15038 -wd5026 -wd5027 -Zc:sizedDealloc- -wd4091 -wd4577 -D_HAS_EXCEPTIONS=0 -W3 -Gy -Zc:inline -arch:SSE2 -Gw -wd4251 -wd4244 -wd4267 -wd4800 -wd4595 -wd4065 -Wno-inline-new-delete -Wno-invalid-offsetof -Wno-microsoft-enum-value -Wno-microsoft-include -Wno-unknown-pragmas -Wno-ignored-pragmas -Wno-deprecated-declarations -Wno-invalid-noreturn -Wno-inconsistent-missing-override -Wno-implicit-exception-spec-mismatch -Wno-unused-local-typedef -Wno-ignored-attributes -Wno-used-but-marked-unused -we4553 -D_SILENCE_TR1_NAMESPACE_DEPRECATION_WARNING -GR- -Z7 -O2 -Oy-  -Xclang -MP -Xclang -dependency-file -Xclang .deps/cairo-d2d-surface.obj.pp -Xclang -MT -Xclang cairo-d2d-surface.obj    z:/build/build/src/gfx/cairo/cairo/src/cairo-d2d-surface.cpp
12:03:57     INFO -  In file included from z:/build/build/src/gfx/cairo/cairo/src/cairo-d2d-surface.cpp:38:
12:03:57     INFO -  z:/build/build/src/gfx/cairo/cairo/src/cairoint.h(64,9):  warning: '_USE_MATH_DEFINES' macro redefined [-Wmacro-redefined]
12:03:57     INFO -  #define _USE_MATH_DEFINES
12:03:57     INFO -          ^
12:03:57     INFO -  z:/build/build/src/obj-firefox/mozilla-config.h(139,9):  note: previous definition is here
12:03:57     INFO -  #define _USE_MATH_DEFINES 1
12:03:57     INFO -          ^
12:03:57     INFO -  In file included from z:/build/build/src/gfx/cairo/cairo/src/cairo-d2d-surface.cpp:38:
12:03:57     INFO -  z:/build/build/src/gfx/cairo/cairo/src/cairoint.h(184,5):  warning: 'register' storage class specifier is deprecated and incompatible with C++17 [-Wdeprecated-register]
12:03:57     INFO -      register int y;
12:03:57     INFO -      ^~~~~~~~~
12:03:57     INFO -  In file included from z:/build/build/src/gfx/cairo/cairo/src/cairo-d2d-surface.cpp:38:
12:03:57     INFO -  In file included from z:/build/build/src/gfx/cairo/cairo/src/cairoint.h:391:
12:03:57     INFO -  In file included from z:/build/build/src/gfx/cairo/cairo/src/cairo-scaled-font-private.h:45:
12:03:57     INFO -  In file included from z:/build/build/src/gfx/cairo/cairo/src/cairo-mutex-type-private.h:45:
12:03:57     INFO -  z:/build/build/src/gfx/cairo/cairo/src/cairo-mutex-impl-private.h(180,9):  warning: 'WIN32_LEAN_AND_MEAN' macro redefined [-Wmacro-redefined]
12:03:57     INFO -  #define WIN32_LEAN_AND_MEAN
12:03:57     INFO -          ^
12:03:57     INFO -  z:/build/build/src/obj-firefox/mozilla-config.h(133,9):  note: previous definition is here
12:03:57     INFO -  #define WIN32_LEAN_AND_MEAN 1

Push with failures:
https://treeherder.mozilla.org/#/jobs?repo=autoland&revision=2e439c0a693490c3e44ad25a080dc59cff2a6f9e

Backout:
https://hg.mozilla.org/integration/autoland/rev/99f868ed6c8c858c8d940f57d2b153bfae5a0eb4
Flags: needinfo?(gsquelart)
Thanks Dorel. I guess I didn't test on enough platforms. :-(
I'll have a look at it tomorrow...
Pushed by gsquelart@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/aa2322354969
Remove misleading std::move's - r=froydnj
https://hg.mozilla.org/mozilla-central/rev/aa2322354969
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla64
Depends on: 1489664
(Forgot to clear NI for backout in comment 4, corrected in comment 6)
Flags: needinfo?(gsquelart)
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: