P2300 LWG 2024/02/07 review

[mhoemmen]

shared_ptr has language saying that the deletes are globally visible. This implies a particular strength of atomic update for decrements. (Lewis has a suggestion on PR #173.) inc-ref definition is good.

para 8: Sndr template parameter of shared-wrapper needs disambiguation from the text's Sndr type. (They aren't even the same type.) We can remove Sndr from the text and leave the template parameter as-is, by using decltype etc. in the text.

shared-wrapper needs a Note to explain lack of assignability and the two cases? or split the copy assignment. The latter would make it clear that the move assignment is always allowed for both tag types, but the copy is not. We want to split the assignment into move assignment and copy assignment. Right now, it's doing double duty.

[jwakely]

The suggestion for shared-wrapper is to split the assignment op into an unconstrained move assignment and a constrained copy assignment:

  shared-wrapper& operator=(shared-wrapper&& other) = default;

  shared-wrapper& operator=(const shared-wrapper& other) noexcept 
  requires same_as<Tag, split_t>
  {
    return *this = shared-wrapper(other);
  }

This makes it clearer that copy assignment is only valid for the split_t tag case, because of the constrained copy ctor.

[mhoemmen]

"to the following lambda" -> "to the following lambda expression" (9.1, specialization of impls-for for shared-impl-tag)

[mhoemmen]

Reading and writing state.sh_state->head? Need a release-acquire pair, not just relaxed.

Add front matter (an opening paragraph, not a Note) explaining that this is a linked list.

Move the "if old_head is equal to state.sh_state" first, above the "if old_head is not equal to state.sh_state" case? In general, these effects don't quite make sense. If the effects happen top to bottom, then some of them return, so the remaining effects don't make sense. (Christian says otherwise.) Also, in the Note, what's the "previous" step? Eric says it's "writes old_head into state.next."

We may be able to use relaxed instead of acquire for the __head.load(std::memory_order_acquire), because it follows a chain of releases.

The "if old_head is equal to state.sh_state" test belongs inside the CAS loop. What if we had wording about linked lists -- e.g., "atomically inserts the item into the linked list"? "In a loop, attempts to..."? Lewis and Eric may need to wordsmith this offline. Can we talk about "if the operation is completed" rather than "equal to state.sh_state"? It's an implementation detail that we're using the tombstone value as the query for whether it's completed or not.

[lewissbaker]

@ericniebler were all of these changes/suggestions applied?

[ericniebler]

They were.