1476307 - -Wlarge-by-value-copy=256 complains on runnable_utils.h

Status: NEW

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

Description

[Sylvestre Ledru [:Sylvestre]]

clang complains on this like:
https://dxr.mozilla.org/mozilla-central/source/media/mtransport/runnable_utils.h#178

'args' is a large (400 bytes) pass-by-value argument; pass it by reference instead ?

Comment 1

[Jean-Yves Avenard [:jya]]

Who should be handling this?

Comment 2

[Nathan Froyd [:froydnj]]

Since this is a template, it would be nice to know where the template is being instantiated from.

The error message is also not good, since `args` isn't really an object itself...unless the analysis is considering all the arguments in the parameter pack as a single object, which would be a bug in the analysis...

Comment 3

[Nils Ohlmeier [:drno]]

Byron, can I ask you to have a look at this please?

Comment 4

[Byron Campen [:bwc]]

This isn't actionable unless we have the place the template is being instantiated from.

Comment 5

[Sylvestre Ledru [:Sylvestre]]

Sorry, voila:

 In file included from /home/sylvestre/dev/mozilla/mozilla-central.hg/media/mtransport/test/test_nr_socket_unittest.cpp:27:
 /home/sylvestre/dev/mozilla/mozilla-central.hg/media/mtransport/runnable_utils.h:178:71: error: 'args' is a large (400 bytes) pass-by-value argument; pass it by reference instead ? [-Werror,-Wlarge-by-value-copy]
   runnable_args_memfn_ret(Ret* ret, Class obj, M method, Arguments... args)
                                                                       ^
 /home/sylvestre/dev/mozilla/mozilla-central.hg/media/mtransport/runnable_utils.h:198:14: note: in instantiation of function template specialization 'mozilla::runnable_args_memfn_ret<int, mozilla::TestNrSocketTest *, int (mozilla::TestNrSocketTest::*)(mozilla::TestNrSocket *, const nr_transport_addr_ &), mozilla::TestNrSocket *, nr_transport_addr_>::runnable_args_memfn_ret<mozilla::TestNrSocket *, nr_transport_addr_>' requested here
   return new runnable_args_memfn_ret<R, Class, M, typename mozilla::Decay<Args>::Type...>(ret, obj, method, std::forward<Args>(args)...);
              ^
 /home/sylvestre/dev/mozilla/mozilla-central.hg/media/mtransport/test/test_nr_socket_unittest.cpp:152:20: note: in instantiation of function template specialization 'mozilla::WrapRunnableRet<int, mozilla::TestNrSocketTest *, int (mozilla::TestNrSocketTest::*)(mozilla::TestNrSocket *, const nr_transport_addr_ &), mozilla::TestNrSocket *&, const nr_transport_addr_ &>' requested here
     sts_->Dispatch(WrapRunnableRet(&result,
                    ^
 1 error generated.

Comment 6

[Byron Campen [:bwc]]

Ah, that's test code for test code. We might be able to modify the stuff in runnable_utils to avoid the extra copy, but I suspect the better thing to do is start moving away from runnable_utils, since we have pretty good support for this kind of thing now (eg; NewRunnableMethod, c++ closures, etc).