0

I recently find a code snippet as follows:

  // To be specific: the "Item" can be viewed as "std::pair<xxx, xxx>*" here
  void moveItemDuringRehash(Item* itemAddr, Item& src) {
    // This is basically *itemAddr = src; src = nullptr, but allowing
    // for fancy pointers.
    // TODO(T31574848): clean up assume-s used to optimize placement new
    assume(itemAddr != nullptr);
    new (itemAddr) Item{std::move(src)};
    src = nullptr;
    src.~Item();
  }

The code is originated from the Folly Lib of Facebook. The functionality of this code is simple: copy std::pair* referenced by src to the memory pointed by itemAddr.

The implementation should be very simple, as mentioned in the comment. But actually, the code does not. The new operator with std::move is confusing, and I am not sure what is happening under the hood. I guess. Item{std::move(src)} construct a temp object with move ctor of std::pair*. And the temp object is copy to the object pointed by itemAddr by copy ctor of std::pair*. I am not sure if my guess is correct. Thank you for sharing your opinion. By the way, I was wondering if there is any performance benefit from this new operator with std::move.

Another question is why src.~Item() is needed? For safety, I need to set src (std::pair*) to nullptr. But why I need to use src.~Item() to dtor a nullptr?

user17732522
  • 53,019
  • 2
  • 56
  • 105
jiexray
  • 325
  • 4
  • 13
  • The assignment `src = nullptr` makes no sense, unless you have an assignment operator for the `Item` class that expects a pointer. And can handle a null pointer in a suitable way. – Some programmer dude Feb 09 '23 at 03:09
  • @Someprogrammerdude `Item` is supposed to be a pointer-like type. – user17732522 Feb 09 '23 at 03:52
  • @user17732522 That is right. The code is for F14NodeMap. The `Item` in the map uses indirectly memory layout, and is a pointer type. More specifically, `Item` is a `std::pair` pointer. – jiexray Feb 09 '23 at 03:55
  • @jiexray The important bit here is that `Item` might not be a pointer. It is the `::pointer` member type of some _Allocator_ type (see https://github.com/facebook/folly/blob/main/folly/container/detail/F14Policy.h#L782), which only needs to be pointer-like. (https://timsong-cpp.github.io/cppwp/n4868/utility.requirements#allocator.requirements.general-5) – user17732522 Feb 09 '23 at 04:18
  • @user17732522 Thank you for your reply. I think the Folly Lib is considering the "copy" of a pointer-like `src`. If `src` is bound to a native pointer, the simple implementation in the comment is fine. However, `src` can be a fancy pointer here, and Folly needs to consider more things. – jiexray Feb 09 '23 at 04:34

1 Answers1

0

My guess without much context:

The function implements a destructive move. It not only moves the value of the Item, but also destroys the Item object.

Similarly, it doesn't expect that there is already a Item object at the location itemAddr. Instead it constructs one.

For a scalar type like std::pair<xxx, xxx>*, there isn't really any difference between a destructive and a non-destructive move, but the function is written so that Item may also be a more complex type than a raw pointer. Types that are not actually raw pointers, but behave like them and can be used to replace raw pointers in allocators are called fancy pointers. The function is written in such a way that general allocators can be supported, including those with fancy pointers. (This isn't obvious from your shown code where no allocator type is mentioned, but that's how it looks in context of the Folly sources.)

A fancy pointer might have non-trivial construction and destruction, so that the destructive move operation and construction of a new object may behave differently than simple assignment.

new (itemAddr) Item{std::move(src)}; is a placement-new to construct a new Item object at the location itemAddr, move-constructed form src. Move-construction is a non-destructive move, so to destruct the source object that was moved from, src.~Item(); is still required.

I am not really sure why src = nullptr; is there. The user of the function may not access src after the destructor call either way. Since C++20 this is also true for the pseudo-destructor call of scalar types. For a fancy pointer the assignment may have side effects, but I don't see why that would matter here. The only reason I could think of is that this is used defensively.

There are no temporary objects involved here.

Please note that I am not familiar with the context in the library. This is just a guess based on what you showed and a quick look at the source file.

user17732522
  • 53,019
  • 2
  • 56
  • 105
  • The code snippet is from the file "F14Policy.h" (line 854, commit d506ac06). – jiexray Feb 09 '23 at 03:57
  • @jiexray Ah, I just noticed that there are two functions with the same name and I was looking at the other one. – user17732522 Feb 09 '23 at 03:58
  • I don't know the exact context this function is used in, but if the fancy pointer has non-trivial destructor/constructor, shouldn't the destructor be explicitly called before using placement new? Unless of course, this function assumes its caller has done that already. – alagner Feb 09 '23 at 07:07
  • 1
    @alagner I think the function assumes that no object has been constructed in the destination prior. Looking at the callsites it seems to be applied only to newly obtained storage as destination. Basically it does what e.g. `std::vector` needs to do per element when reallocating to a new allocation in one operation (move construction and destruction). – user17732522 Feb 09 '23 at 07:17