rjmccall added inline comments.
================
Comment at: docs/LanguageExtensions.rst:1096
+ equivalent to copying the underlying bytes and then dropping the source
object
+ on the floor.
* ``__is_destructible`` (MSVC 2013)
----------------
Quuxplusone wrote:
> rjmccall wrote:
> > Quuxplusone wrote:
> > > rjmccall wrote:
> > > > Quuxplusone wrote:
> > > > > rjmccall wrote:
> > > > > > Quuxplusone wrote:
> > > > > > > @rjmccall wrote:
> > > > > > > > trivial_abi permits annotated types to be passed and returned
> > > > > > > > in registers, which is ABI-breaking. Skimming the blog post, it
> > > > > > > > looks like trivially_relocatable does not permit this — it
> > > > > > > > merely signifies that destruction is a no-op after a move
> > > > > > > > construction or assignment.
> > > > > > >
> > > > > > > Not necessarily a "no-op"; my canonical example is a
> > > > > > > CopyOnlyCXX03SharedPtr which increments a refcount on
> > > > > > > construction and decrements on destruction. But move-construction
> > > > > > > plus destruction should "balance out" and result in no observable
> > > > > > > side effects.
> > > > > > >
> > > > > > > > This is usefully different in the design space, since it means
> > > > > > > > you can safely add the attribute retroactively to e.g.
> > > > > > > > std::unique_ptr, and other templates can then detect that
> > > > > > > > std::unique_ptr is trivially-relocatable and optimize
> > > > > > > > themselves to use memcpy or realloc or whatever it is that they
> > > > > > > > want to do. So in that sense trivial_abi is a *stronger*
> > > > > > > > attribute, not a *weaker* one: the property it determines ought
> > > > > > > > to imply trivially_relocatable.
> > > > > > >
> > > > > > > `trivial_abi` is an "orthogonal" attribute: you can have
> > > > > > > `trivial_abi` types with non-trivial constructors and
> > > > > > > destructors, which can have observable side effects. For example,
> > > > > > > ```
> > > > > > > struct [[clang::trivial_abi]] DestructionAnnouncer {
> > > > > > > ~DestructionAnnouncer() { puts("hello!"); }
> > > > > > > };
> > > > > > > ```
> > > > > > > is `trivial_abi` (because of the annotation) yet not trivially
> > > > > > > relocatable, because its "move plus destroy" operation has
> > > > > > > observable side effects.
> > > > > > >
> > > > > > > > The only interesting question in the language design that I
> > > > > > > > know of is what happens if you put the attribute on a template
> > > > > > > > that's instantiated to contain a sub-object that is definitely
> > > > > > > > not trivially relocatable / trivial-ABI. For trivial_abi, we
> > > > > > > > decided that the attribute is simply ignored — it implicitly
> > > > > > > > only applies to specializations where the attribute would be
> > > > > > > > legal. I haven't dug into the design enough to know what
> > > > > > > > trivially_relocatable decides in this situation, but the three
> > > > > > > > basic options are:
> > > > > > > >
> > > > > > > > - the attribute always has effect and allows trivial relocation
> > > > > > > > regardless of the subobject types; this is obviously unsafe, so
> > > > > > > > it limits the safe applicability of the attribute to templates
> > > > > > > > - the attribute is ignored, like trivial_abi is
> > > > > > > > - the attribute is ill-formed, and you'll need to add a
> > > > > > > > [[trivially_relocatable(bool)]] version to support templates
> > > > > > >
> > > > > > > What happens is basically the first thing you said, except that I
> > > > > > > disagree that it's "obviously unsafe." Right now, conditionally
> > > > > > > trivial relocation is possible via template metaprogramming; see
> > > > > > > the libcxx patch at e.g.
> > > > > > > https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> > > > > > > Since the attribute is an opt-in mechanism, it makes perfect
> > > > > > > sense to me that if you put it on a class (or class template),
> > > > > > > then it applies to the class, without any further sanity-checking
> > > > > > > by the compiler. The compiler has no reason to second-guess the
> > > > > > > programmer here.
> > > > > > >
> > > > > > > However, there's one more interesting case. Suppose the
> > > > > > > programmer puts the attribute on a class that isn't relocatable
> > > > > > > at all! (For example, the union case @erichkeane mentioned, or a
> > > > > > > class type with a deleted destructor.) In that case, this patch
> > > > > > > *does* give an error... *unless* the class was produced by
> > > > > > > instantiating a template, in which case we *don't* give an error,
> > > > > > > because it's not the template-writer's fault.
> > > > > > > https://p1144.godbolt.org/z/wSZPba
> > > > > > > trivial_abi is an "orthogonal" attribute: you can have
> > > > > > > trivial_abi types with non-trivial constructors and destructors,
> > > > > > > which can have observable side effects.
> > > > > >
> > > > > > Let me cut this conversation short. `trivial_abi` is not such an
> > > > > > old and widely-established attribute that we are unable to revise
> > > > > > its definition. I am comfortable making the same semantic
> > > > > > guarantees for `trivial_abi` that you're making for
> > > > > > `trivially_relocatable`, because I think it is in the language's
> > > > > > interest for `trivial_abi` to be strictly stronger than
> > > > > > `trivially_relocatable`.
> > > > > >
> > > > > > > What happens is basically the first thing you said, except that I
> > > > > > > disagree that it's "obviously unsafe."
> > > > > >
> > > > > > Under your semantics, the attribute is an unchecked assertion about
> > > > > > all of a class's subobjects. A class template which fails to
> > > > > > correctly apply the template metaprogramming trick to all of its
> > > > > > dependently-typed subobjects — which can be quite awkward because
> > > > > > it creates an extra dimension of partial specialization, and which
> > > > > > breaks ABI by adding extra template parameters — will be silently
> > > > > > miscompiled to allow objects to be memcpy'ed when they're
> > > > > > potentially not legal to memcpy. That is a footgun, and it is
> > > > > > indeed "obviously unsafe".
> > > > > >
> > > > > > Now, it's fair to say that it's unsafe in a useful way: because the
> > > > > > attribute isn't checked, you can wrap a type you don't control in a
> > > > > > `trivially_relocatable` struct and thereby get the advantages of
> > > > > > triviality on the wrapper. The model used by `trivial_abi` doesn't
> > > > > > allow that. But I feel pretty strongly that that is not the right
> > > > > > default behavior for the language.
> > > > > > Under your semantics, the attribute is an unchecked assertion about
> > > > > > all of a class's subobjects.
> > > > >
> > > > > The attribute is an unchecked assertion about the class's //special
> > > > > member functions//. The attribute doesn't have anything to do with
> > > > > subobjects, period.
> > > > > Vice versa, the property currently expressed by
> > > > > "IsNaturallyTriviallyRelocatable" is deduced from all of the class's
> > > > > subobjects. The programmer can overrule the "natural" property in an
> > > > > "unnatural" way by annotating their class with the attribute.
> > > > >
> > > > > And we know this is true because it is possible to make a
> > > > > trivially-relocatable class type containing non-trivially-relocatable
> > > > > members (e.g. a class having a member of type
> > > > > boost::interprocess::offset_ptr), and vice versa it is possible to
> > > > > make a non-trivially-relocatable class containing
> > > > > trivially-relocatable members (e.g. boost::interprocess::offset_ptr
> > > > > itself, which has only one member, of integral type).
> > > > >
> > > > > > A class template which fails to correctly apply the template
> > > > > > metaprogramming trick to all of its dependently-typed subobjects —
> > > > > > which can be quite awkward because it creates an extra dimension of
> > > > > > partial specialization
> > > > >
> > > > > Agreed that it's awkward. The libc++ implementation was awkward, but
> > > > > definitely not challenging. The only thing that makes it at all
> > > > > tricky in the STL is that the STL allocator model permits fancy
> > > > > "pointer" types that can make e.g. std::vector non-trivially
> > > > > relocatable. If it weren't for fancy pointers, you wouldn't need the
> > > > > extra dimension.
> > > > >
> > > > > > and which breaks ABI by adding extra template parameters
> > > > >
> > > > > The libc++ implementation does not break ABI. The extra template
> > > > > parameter is concealed in a private base class.
> > > > > https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912
> > > > >
> > > > > > I feel pretty strongly that that is not the right default behavior
> > > > > > for the language.
> > > > >
> > > > > Can you elaborate on that feeling (maybe in private email)? My intent
> > > > > with P1144 is that no industry programmer should ever see this
> > > > > attribute; the right default for industry programmers is to use the
> > > > > Rule of Zero. The reason we need the attribute is as an opt-in
> > > > > mechanism for the implementor of `unique_ptr`, `shared_ptr`,
> > > > > `vector`, and so on, //so that// the end-user can just use the Rule
> > > > > of Zero and everything will work fine. End-users shouldn't be messing
> > > > > with attributes.
> > > > > And we know this is true because it is possible to make a
> > > > > trivially-relocatable class type containing non-trivially-relocatable
> > > > > members (e.g. a class having a member of type
> > > > > boost::interprocess::offset_ptr), and vice versa it is possible to
> > > > > make a non-trivially-relocatable class containing
> > > > > trivially-relocatable members (e.g. boost::interprocess::offset_ptr
> > > > > itself, which has only one member, of integral type).
> > > >
> > > > Why would a class containing a member of type
> > > > `boost::interprocess::offset_ptr` be trivially-relocatable? If you
> > > > actually trivially relocate an object of the class, the pointer will
> > > > not be rebased and so will be invalidated. It would have to be an
> > > > `offset_ptr` where you happen to know that the referent will always be
> > > > copied simultaneously, e.g. because it's a member of the object itself.
> > > > Of course that's possible, but it's also such a corner case that we
> > > > shouldn't balk at saying that the programmer ought to be more explicit
> > > > about recognizing it.
> > > >
> > > > > Agreed that it's awkward. The libc++ implementation was awkward, but
> > > > > definitely not challenging. The only thing that makes it at all
> > > > > tricky in the STL is that the STL allocator model permits fancy
> > > > > "pointer" types that can make e.g. std::vector non-trivially
> > > > > relocatable. If it weren't for fancy pointers, you wouldn't need the
> > > > > extra dimension.
> > > >
> > > > Sure. My point about the awkwardness is quite narrow: making the
> > > > attribute take a `bool` argument is just a superior way of managing
> > > > this over requiring a partial specialization. Several other language
> > > > attributes have been heading in this same direction.
> > > >
> > > > > The libc++ implementation does not break ABI. The extra template
> > > > > parameter is concealed in a private base class.
> > > >
> > > > Ah, apologies.
> > > >
> > > > > My intent with P1144 is that no industry programmer should ever see
> > > > > this attribute; the right default for industry programmers is to use
> > > > > the Rule of Zero. ... End-users shouldn't be messing with attributes.
> > > >
> > > > Neither of these statements matches my experience. This is an "expert"
> > > > feature to be sure, but the C++ community is full of experts who write
> > > > their own rule-of-five types and who will happily use whatever
> > > > attributes are available to them to make them faster.
> > > >
> > > > Also, I assume you are intending for this attribute to be standardized
> > > > eventually, which will greatly expand its reach.
> > > > Why would a class containing a member of type
> > > > `boost::interprocess::offset_ptr` be trivially-relocatable? If you
> > > > actually trivially relocate an object of the class, the pointer will
> > > > not be rebased and so will be invalidated. It would have to be an
> > > > offset_ptr where you happen to know that the referent will always be
> > > > copied simultaneously, e.g. because it's a member of the object itself.
> > >
> > > Exactly! (And to preserve the class invariant, you'd have to add a
> > > copy-constructor.)
> > >
> > > > Of course that's possible, but it's also such a corner case that we
> > > > shouldn't balk at saying that the programmer ought to be more explicit
> > > > about recognizing it.
> > >
> > > Exactly — and the way for the programmer to explicitly recognize (or I
> > > say "warrant") that their class has the property is for them to annotate
> > > it with `[[trivially_relocatable]]`. So I guess maybe I don't understand
> > > what you mean by "more explicit"?
> > >
> > > > making the attribute take a `bool` argument is just a superior way of
> > > > managing this
> > >
> > > That's possible, but it's also possible that it would increase the
> > > complexity of parsing attributes for some implementations. I mean, we're
> > > talking about something like the following, right? (Using the libc++
> > > patch as the example, but I've de-uglified some of the names.) So I think
> > > it's a tradeoff and I'm ambivalent about it, so far. (This is one of the
> > > [[
> > > https://quuxplusone.github.io/blog/2018/11/11/trivially-relocatable-in-san-diego/#if-you-feel-comfortable-respondi
> > > | straw poll questions in P1144R0 ]].)
> > > ```
> > > template <class T, class A = allocator<T>>
> > > class [[trivially_relocatable(__deque_base<T,
> > > A>::__allow_trivial_relocation::value)]] deque
> > > : private __deque_base<T, A>
> > > ```
> > >
> > > > This is an "expert" feature to be sure, but the C++ community is full
> > > > of experts who write their own rule-of-five types and who will happily
> > > > use whatever attributes are available to them to make them faster.
> > >
> > > Agreed. But the C++ community is //also// full of working programmers who
> > > just write simple code with strings and vectors. :) I want
> > > `[[trivially_relocatable]]` to be approximately as frequently seen in
> > > real codebases as `[[no_unique_address]]` — i.e. maybe a couple times in
> > > that smart-pointer library the contractor wrote, but nowhere near the
> > > user code. If it's seen frequently in user code, then we've failed those
> > > users.
> > > Exactly! (And to preserve the class invariant, you'd have to add a
> > > copy-constructor.)
> >
> > But then it still wouldn't be trivially relocatable, because there's
> > user-defined code that has to run to copy it correctly. The only way such
> > a type could ever be meaningfully trivially relocatable outside of
> > obviously unknowable external conditions is if it has fields that it never
> > uses after it's been relocated.
> >
> > > Exactly — and the way for the programmer to explicitly recognize (or I
> > > say "warrant") that their class has the property is for them to annotate
> > > it with [[trivially_relocatable]]. So I guess maybe I don't understand
> > > what you mean by "more explicit"?
> >
> > I think it is far more likely that some well-intentioned library author
> > will add `[[trivially_relocatable]]` incorrectly than that they'll actually
> > intend to override the trivial relocatability of their subobjects.
> >
> > By "more explicit", I was suggesting that you add some kind of "force"
> > syntax to the attribute (straw-man suggestion:
> > `[[trivially_relocatable!]]`). Without the force, the attribute will
> > negate non-triviality from special members in the class but won't override
> > natural non-triviality from subobjects.
> >
> > > That's possible, but it's also possible that it would increase the
> > > complexity of parsing attributes for some implementations.
> >
> > All conforming implementations have to do the work to support things like
> > this already because of `alignas`, `noexcept`, etc.
> >
> > > Agreed. But the C++ community is also full of working programmers who
> > > just write simple code with strings and vectors. :) I want
> > > [[trivially_relocatable]] to be approximately as frequently seen in real
> > > codebases as [[no_unique_address]] — i.e. maybe a couple times in that
> > > smart-pointer library the contractor wrote, but nowhere near the user
> > > code. If it's seen frequently in user code, then we've failed those users.
> >
> > I think you are underestimating the sophistication of "working programmers"
> > and overestimating the sophistication of library developers. A
> > `[[trivially_relocatable]]` that doesn't override subobject triviality is
> > far easier for library authors to use correctly and will avoid an endless
> > cascade of oversights.
> >
> > Case in point, every single subtle thing that you had to anticipate and
> > call out in your patch to `std::deque` would just go away if the language
> > simply propagated non-triviality of subobjects.
> > I think it is far more likely that some well-intentioned library author
> > will add `[[trivially_relocatable]]` incorrectly than that they'll actually
> > intend to override the trivial relocatability of their subobjects.
>
> But overriding "natural" non-trivial relocatability is precisely the reason
> for P1144 `[[trivially_relocatable]]`! If you just have a plain old
> Rule-of-Zero object with trivially relocatable subobjects, and you want your
> object to be trivially relocatable as a result, the core language takes care
> of that for you (just like with trivial {con,de}structibility and
> trivial-abi-ness: a Rule-of-Zero composite of trivial objects is itself
> trivial). The //only// use-case for the `[[trivially_relocatable]]` attribute
> is when you are trying to tell the compiler that you know exactly what you're
> doing. (Which is why normal working programmers won't generally use it.)
>
> > By "more explicit", I was suggesting that you add some kind of "force"
> > syntax to the attribute (straw-man suggestion:
> > `[[trivially_relocatable!]]`). Without the force, the attribute will negate
> > non-triviality from special members in the class but won't override natural
> > non-triviality from subobjects.
>
> Can you give an example of how it would work with `deque`, for example? What
> part of the `deque` implementation would become simpler, in exchange for this
> added complexity of specification?
>
> You say:
> > Case in point, every single subtle thing that you had to anticipate and
> > call out in your patch to `std::deque` would just go away if the language
> > simply propagated non-triviality of subobjects.
>
> But I don't understand what you mean by this. Are you saying that you want to
> be able to write `class [[trivially_relocatable]] deque { ... }` to mean that
> you want the trivially-relocatable-ness of `deque` to match the
> trivially-relocatable-ness of its least relocatable subobject? But then for
> example in the libc++ patch, I'd either have to go out of my way to make sure
> that `__map` and `__deque_base` were trivially relocatable (which would
> require metaprogramming similar to what's there now, except one turtle lower
> down in the stack — or were you thinking of adding
> `[[trivially_relocatable]]` to //all// the turtles in the stack?), or else
> I'd have to use `class [[trivially_relocatable!]] deque` to overrule the
> non-trivial-relocatability of `deque`'s `__map` and `__deque_base`
> subobjects, in which case I'd still need the four lines you were trying to
> eliminate.
> https://github.com/Quuxplusone/libcxx/commit/6524822c009e#diff-38adc80cec663f2f29c22e9ffc0de912R957
>
> My kneejerk reaction is that it is a bad idea to have two similarly-named
> things, one of which has subtly correct semantics and the other of which has
> subtly incorrect semantics, especially when it's not obvious which one is
> correct in any specific situation. (OT: This is basically the reason behind
> my [[ http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2018/p1155r0.html |
> P1155 ]].) But even beyond that //general// reaction, I //specifically// have
> not understood how `[[trivially_relocatable!]]` would help the programmer of
> `deque`.
>
> > All conforming implementations have to do the work to support things like
> > this already because of alignas, noexcept, etc.
>
> Yes, but those aren't attributes, grammatically... Well, I guess there is
> already a standard grammar for parsing unknown attributes in terms of
> `balanced-token-seq`, and I can't think of any boolean expression that is not
> a `balanced-token-seq`, so okay, I'll retract my FUD over the //technical//
> difficulties of `[[trivially_relocatable(bool)]]`. I am still ambivalent as
> to whether it'd be a good tradeoff. (Complexity of specification and arcane
> terseness of code, versus simplicity of specification and boilerplate
> verbosity of code.)
> But then for example in the libc++ patch, I'd either have to go out of my way
> to make sure that __map and __deque_base were trivially relocatable (which
> would require metaprogramming similar to what's there now, except one turtle
> lower down in the stack — or were you thinking of adding
> [[trivially_relocatable]] to all the turtles in the stack?),
You would need the attribute only at the level(s) that actually defined the
special members; all the "rule of zero" levels would of course propagate
trivial relocatability.
> I specifically have not understood how [[trivially_relocatable!]] would help
> the programmer of deque.
It wouldn't. I don't think it's worth adding at all, actually. I'm just
saying it's possible to add it if you really think that "trivially-relocatable
type with a non-trivially-relocatable subobject" is a relevant use case.
> Yes, but those aren't attributes, grammatically...
True, `alignas` is not spelled as an attribute (which I constantly forget — it
was a very odd decision).
Recent drafts of the standard do include `expects` and `ensures`, which take
arbitrary expressions as operands. The spelling's a bit different from the
strawman I suggested, though: it's `[[expects: x > 0]]`.
> I can't think of any boolean expression that is not a balanced-token-seq
That's not a coincidence: all expressions are `balanced-token-seq`s, as is
every other major production in the C++ grammar. The grammar allows a
`balanced-token-seq` there precisely because the committee was anticipating
attributes that take arbitrarily complex expressions as operands, which are
quite common in all the vendor extensions they were looking at.
> I am still ambivalent as to whether it'd be a good tradeoff. (Complexity of
> specification and arcane terseness of code, versus simplicity of
> specification and boilerplate verbosity of code.)
My two pieces of feedback are separable. Even with your preferred semantics,
your proposal would be much better if the attribute followed a `noexcept`-like
design where it can optionally take a boolean argument. In fact, adding that
is much more important with your semantics for the attribute since so many use
sites will be conditional.
Also, I don't think your semantics actually give rise to a simpler
specification. The specification has to formalize "naturally trivially
relocatable" in either case because it's important for rule-of-zero types to be
trivially relocatable if all their subobjects are. My suggested semantics are
basically just that the attribute doesn't override the natural relocatability
of the type; it only prevents the presence of non-trivial special members from
changing it. That's a short paragraph in the spec in exchange for an attribute
that's much less prone to errors in corner cases (e.g. ignoring the possibility
of non-trivial pointer types) and which rarely needs to be explicitly
conditionalized. But it does make the (unsafe) override case impossible,
although, again, I think the motivation for that is pretty weak.
Repository:
rC Clang
https://reviews.llvm.org/D50119
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
