Sylvestre Ledru [:Sylvestre] Reporter
	Description • 6 years ago

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 ?

	Jean-Yves Avenard [:jya]
	Comment 1 • 6 years ago

Who should be handling this?

	Nathan Froyd [:froydnj]
	Comment 2 • 6 years ago

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...

	Nils Ohlmeier [:drno]
	Comment 3 • 6 years ago

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

	Byron Campen [:bwc]
	Comment 4 • 6 years ago

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

	Sylvestre Ledru [:Sylvestre] Reporter
	Comment 5 • 6 years ago

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.

	Byron Campen [:bwc]
	Comment 6 • 6 years ago

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).