hsutter commented on PR #51270:
URL: https://github.com/apache/arrow/pull/51270#issuecomment-5648246051

   I've now taken a pass with an updated skill that appears to do much better, 
correctly adding `std::move` in many more places while leaving the parameter 
passed by value. 
   
   Before I bother you with an update though, I have a question about these 
cases below -- could you please let me know whether you think the correct thing 
is to change the parameter to pass by `const&`, or to add `std::move()` on 
definite last use in the body, or no change at all?
   
      - arrow/acero/source_node.cc:304, TableSourceNode ctor
      - arrow/acero/unmaterialized_table_internal.h:245, AddEntry
      - arrow/adapters/orc/adapter_test.cc:131, GenerateRandomTimestampArray
      - arrow/array/array_dict_test.cc:906, 
TestDecimalDictionaryBuilderDoubleTableSize
      - arrow/array/array_run_end_test.cc:71, RunEndEncodedArrayFromJSON
      - arrow/array/diff_test.cc:118, BaseAndTargetFromRandomFilter
      - arrow/buffer_test.cc:90, MyMemoryManager ctor
   
   Thank you.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to