Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

P2785 Relocating prvalues and potential ABI impacts #187

Open
SebastienBini opened this issue Jul 3, 2024 · 2 comments
Open

P2785 Relocating prvalues and potential ABI impacts #187

SebastienBini opened this issue Jul 3, 2024 · 2 comments

Comments

@SebastienBini
Copy link

Last year P2785R3 was presented and did not reach consensus. The proposal has been reworked to a forth revision, visible here (not yet the official bump). As the main author of the paper, I'd like to discuss with you the potential ABI breaks this proposal might introduce.

P2785 introduces the relocation operation, by the means of a new relocation constructor T(T), which simultaneously constructs a new instance and destroys its source instance. To avoid introducing a new value category, we propose relocation to happen from prvalues. And to allow the relocation of automatic variables, we introduce the new reloc operator which simply changes the value category from lvalue to prvalue. Once the variable has changed its value category, a constructor is selected from the relocation, the move or the copy constructor. Relocation will only effectively happen if the relocation constructor is selected. Once a variable has been passed to reloc, it can no longer be used in the rest of its scope, regardless of which constructor was selected.

T const x = getX();
T const y = getY(x);
foo(reloc x, reloc y);
// If T has a relocation constructor, it is used to construct the parameters of foo
// x, y can no longer be used as they may have been destructed

So far so good, ABI problems start to happen in the following scenario:

void foo(T x)
{
  bar(reloc x);
}

Since x is the function parameter, on Itanium foo has no control over its lifetime, and hence calling the relocation constructor of x in that scenario will double the object destruction (once in the relocation constructor call, once when control returns to foo's caller).

We are well aware of that issue and are doing our best not to impose an ABI break. We came up with the following rule: When selecting a constructor, if the source object is a function parameter passed by value, and its type provides an eligible move constructor, then the relocation constructor (if any) may be discarded during overload resolution. Whether the relocation constructor is discarded is implementation-defined. The destruction of the source object, should the relocation constructor not be selected, is postponed until control returns at the caller site.
The corollary to this rule, is that functions that takes parameters by value, whose type declares a relocation constructor but no move constructor, must have control over the ownership of such parameters. Such functions may have a different mangling to better detect breakages.

With this rule, ABI breaks are opt-in. They will only happen when someone defines a relocation constructor in their class and defines no move constructor. P2785 does not discuss in details how the STL shall evolve to support relocation (it will be done in another proposal once P2785 is mature enough), but one thing is certain: a relocation constructor will be declared in classes where there is already a move constructor. This addition will cause no ABI breaks because of the rule above.

Now there may be the case of types that become relocate-only by accident. Let's consider a putative gsl::non_null with a relocation constructor. Then gsl::non_null<std::unique_ptr<int>> will only be relocatable because of its template parameter. We believe that such breakages can still be anticipated and controlled (advertise the change, use a new non_null wrapper type, etc...)

What do you think of this proposal in terms of ABI? Do you have any concerns in terms of ABI? Do you think the suggested rule is enough? Feel free to ask for clarifications. Thank you for your time.

@rjmccall
Copy link
Collaborator

rjmccall commented Jul 8, 2024

I have a few concerns, not having yet read your paper.

  1. I get worried when I see seemingly site-specific wording like "eligible move constructor" used together with a predicate that has to be universal to the type; we've had problems there in the past with copy/move construction of parameters. But in this case, maybe the conditions on reloc are tight enough to make that not a problem — the predicates end up perfectly coinciding. Still, I'd be happier if you just specified a single property of types that was used in both places.

  2. It seems really unfortunate that the basic feature works differently on different platforms. As specified, reloc on a parameter is only allowed on Windows unless the type contorts itself to support it. And "contorts" seems reasonable as a description here: I would expect that most types would like to declare reloc constructors without necessarily losing the ability to do non-relocation moves. It would be preferable for the feature to be consistent across platforms, which means either requiring the type to opt in to relocatable parameters or requiring specific parameters to opt in.

  3. Is there a concept of trivial reloc-ability?

@SebastienBini
Copy link
Author

Thank you for your response.

  1. I understand your concern, and as I wrote it, didn't know whether that "eligible" bit was going to be an issue. How would you phrase it then? I looked at Itanium ABI, that if I understand correctly, has different ways to pass value parameters depending on whether they are "trivial" or not. I quote:

A type is considered non-trivial for the purposes of calls if it has a non-trivial copy constructor, move constructor, or destructor, or all of its copy and move constructors are deleted.

I don't know what "has" mean here (declared? only publicly? implicitly? eligible?), but I'm open to align the constraints on the move constructor to something that fit the spirit of existing rules.

  1. Yes I would have liked the feature to work similarly across all platforms. Actually the first draft of the proposal did it and got a mitigated reception especially because that ABI break. Fallbacking to move + delayed destroy is an acceptable behavior in my opinion, as it preserves the ABI. At least you benefit from the rest of the reloc features: you can reloc/move constant objects, and you are sure not to have use-after-move errors.
    Also, it is not such a contortion. I'd argue that move semantics are more contorted sometimes because of the introduction of the moved-from state which needs to be supported (at least in the destructor), and which is sometimes creating loopholes in class invariants or preventing you from having constant data members altogether. With the relocation constructor, unless the class has some self-referencing pointers, the default implementation shall very likely do the right thing. No need to support the "empty" state, and one can declare constant data members (the move ctor would copy those instead of moving them, while relocation semantics simply ignore cv qualifiers).
    It is possible to mitigate this. I see two possibilities there: (a) we can provide an attribute on the reloc ctor to force the ABI break on functions that have such parameters. (b) compilers can provide an ABI option that will force the breakage on such functions. In the last solution, the entire program and consumed libraries will need to be compiled with that option. But issues can be detected at link time if the new ABI mangles such functions differently. (Option (b) makes more sense to me). None of those solutions need to be part of the ISO C++ standard though, since that goes beyond the language itself. But the proposal can discuss such solutions.
  2. Yes there is. Put simply, it is trivially relocatable if the type has a defaulted reloc ctor and all its subobjects are also trivially relocatable. Under those conditions, the reloc ctor is equivalent to memcpy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants