ArenaSafeUniquePtr : When to use std::move(), and when not to use it?

[John Nolan]

Hello protobuf@,

In the code below, do we really need to use std::move() when invoking  candidate_logging_context->AddFeature() ?  Please see the code snippet below. 

The documentation for "Arenas, Messages and Pointers" contains examples that show the use of std::move() when invoking the Movers library functions.

https://g3doc.corp.google.com/net/proto2/contrib/arena_utils/g3doc/arenas-ptrs-best-practices.md?cl=head

However, candidate_logging_context.AddFeature() is not one of these Movers library functions, and I'm not sure it is a similar case. (Is it?) The source for AddFeature is here: https://cs.corp.google.com/piper///depot/google3/assistant/assistant_server/context/candidate_logging_context_impl.cc?l=355

There are some documents for std::move() that recommend against using std::move() when returning a value. I'm pretty sure that this advice applies here, since we are passing the value to an entirely different function. The value cannot be moved, so std::move() is a no-op.

Am I misunderstanding something? 

https://g3doc.corp.google.com/devtools/library_club/g3doc/totw/labs/should-i-return-std-move.md?cl=head

Thanks!   My apologies if I have misunderstood.

void AddTagOnToAssistantLogs(

    const assistant::btw::TagOn& tag_on, bool experimental,

    proto2::Arena* arena, CandidateLoggingContext* candidate_logging_context) {

  if (!tag_on.has_btw()) return;

  const auto* ve_type = GetFeatureByBtwResponseSource(tag_on.btw().source());

  auto ve = MakeArenaSafeUnique<::logs::VisualElementProto>(arena);

  ve->mutable_assistant_btw_data()->set_rule_id(tag_on.btw().rule_id());

  if (experimental) {

    // Not logging the BTW for counterfactual triggers.

    ve->set_visible(

        ::logs::VisualElementProto::VISIBILITY_REPRESSED_COUNTERFACTUAL);

  } else if (!tag_on.btw().is_personalized_content()) {

    ve->mutable_assistant_btw_data()->set_btw_text(tag_on.btw().text());

  }

  if (tag_on.btw().secondary_ve_source() != BtwResponseSource::UNKNOWN) {

    auto secondary_ve = Copy(ve);

    const auto* secondary_ve_type =

        GetFeatureByBtwResponseSource(tag_on.btw().secondary_ve_source());

    candidate_logging_context->AddFeature(secondary_ve_type,

                                          std::move(secondary_ve));

  }

  candidate_logging_context->AddFeature(ve_type, std::move(ve));

}