Project

General

Profile

Bug #1701

Cannot have non-copyable types in constructor arguments

Added by Nils Deppe 17 days ago. Updated 6 days ago.

Status:
New
Priority:
Normal
Assignee:
Category:
-
Target version:
-
Start date:
09/30/2017
Due date:
% Done:

0%


Description

If I try to pass a non-copyable type to a chare constructor my only option is to move an lvalue reference, which is not good. The solution is again supporting C++11, though there may be a C++03 method of implementing a workaround.

History

#1 Updated by Phil Miller 12 days ago

I take it you're trying to pass an rvalue reference to such an object? It doesn't really make semantic sense for any entry method argument (passed to a constructor or otherwise) to be truly non-copyable, in that they're going to get serialized and desrialized, creating a copy in some form.

I think the code is happy to take a const &, though. I'll check that shortly.

#2 Updated by Nils Deppe 12 days ago

Looking now, this ticket could be made much more helpful on my part. The serialization is not the problem, it is the move into the impl_obj_void. Below is a modified def.h file where I have added the missing calls to std::move. The deserialized impl_noname_0 is normally passed as an lvalue reference, which is fine for copyable types, but not for non-copyable. Having the constructor take a non-const lvalue reference and moving that is a workaround, but this breaks using the class outside of a Charm++ context. So far this has only bitten us in unit tests, but as C++11 support is required this should be a relatively straightforward change to Charm++.

template <typename Metavariables>

void CkIndex_ConstGlobalCache<Metavariables>::_call_ConstGlobalCache_marshall1(
    void* impl_msg, void* impl_obj_void) {
  ConstGlobalCache<Metavariables>* impl_obj =
      static_cast<ConstGlobalCache<Metavariables>*>(impl_obj_void);
  CkMarshallMsg* impl_msg_typed = (CkMarshallMsg*)impl_msg;
  char* impl_buf = impl_msg_typed->msgBuf;
  /*Unmarshall pup'd fields: const tuples::TaggedTupleTypelist<typename
   * Metavariables::const_global_cache_tag_list > &impl_noname_0*/
  PUP::fromMem implP(impl_buf);
  tuples::TaggedTupleTypelist<
      typename Metavariables::const_global_cache_tag_list>
      impl_noname_0;
  implP | impl_noname_0;
  impl_buf += CK_ALIGN(implP.size(), 16);
  /*Unmarshall arrays:*/
  new (impl_obj_void) ConstGlobalCache<Metavariables>(std::move(impl_noname_0));
}
template <typename Metavariables>

int CkIndex_ConstGlobalCache<Metavariables>::
    _callmarshall_ConstGlobalCache_marshall1(char* impl_buf,
                                             void* impl_obj_void) {
  ConstGlobalCache<Metavariables>* impl_obj =
      static_cast<ConstGlobalCache<Metavariables>*>(impl_obj_void);
  /*Unmarshall pup'd fields: const tuples::TaggedTupleTypelist<typename
   * Metavariables::const_global_cache_tag_list > &impl_noname_0*/
  PUP::fromMem implP(impl_buf);
  tuples::TaggedTupleTypelist<
      typename Metavariables::const_global_cache_tag_list>
      impl_noname_0;
  implP | impl_noname_0;
  impl_buf += CK_ALIGN(implP.size(), 16);
  /*Unmarshall arrays:*/
  new (impl_obj_void) ConstGlobalCache<Metavariables>(std::move(impl_noname_0));
  return implP.size();
}

#3 Updated by Phil Miller 12 days ago

Indeed, CProxy_Foo's generated code takes its arguments by const& even when declared in the .ci file as taking them by value. So, a temporary object passed as an argument will do the right thing, and an actual lvalue can be moved or not, and still be consumed as const lvalue reference. As far as I can tell, this means there isn't an issue of code semantics that should work but doesn't, but there might be syntactic representations that have to be avoided. Do I have that right, or is there a case I haven't considered properly? Do you have a concrete example I could look at?

#4 Updated by Phil Miller 12 days ago

OK, our notes crossed paths as we were writing them. I'll look further in a bit.

#5 Updated by Phil Miller 12 days ago

OK, so I think what you're saying is that the generated code needs to move the temporary instances, so that the recipient can potentially take them by rvalue reference and consume them, rather than only being able to copy or hackishly moving from something that looks like an lvalue reference argument. Is that right? The difference between the current generated code and your example is the addition of std::move?

#6 Updated by Nils Deppe 12 days ago

Yep, that's exactly correct. The only difference between my modified code and the generated is the addition of std::move. Specifically,

new (impl_obj_void) ConstGlobalCache<Metavariables>(simpl_noname_0);
// was changed to
new (impl_obj_void) ConstGlobalCache<Metavariables>(std::move(impl_noname_0));

and the same for the second function.

#7 Updated by Eric Bohm 6 days ago

  • Assignee set to Eric Mikida

Also available in: Atom PDF