[cfe-dev] std::pair not trivially copyable?

Mon Aug 31 09:45:17 PDT 2020

On Mon, Aug 31, 2020 at 8:09 AM Robinson, Paul <paul.robinson at sony.com>
wrote:

> Hopefully without diverting the quite relevant discussion of std::pair’s
> triviality, and looking at this Blaikie/Smith exchange again:
>
>
>
> (the original issue with the warning could be fixed by narrowing the
> warning to only worry about trivial copy construction, not trivial
> copyability in general)
>
>
>
> That sounds right to me. Is it worth also considering the triviality of
> the destructor?
>
>
>
> Looking at the description of range-for here
> https://en.cppreference.com/w/cpp/language/range-for it has
>
>                *range_declaration *= *__begin;
>
> …which is an initialization, not an assignment; so for the diagnostic (and
> codegen), Clang should be looking at copy-initialization/construction not
> copy-assignment anyway.  And std::pair’s copy constructors **are**
> trivial.
>
> Right?
>

Right - hence the suggestion. The question about whether non-trivial dtor
should matter to the warning is a fair one & I don't have a good sense of
the right answer - would be OK with that causing the warning too (though
that'd add (a probably small number - probably aren't many things with a
non-trivial dtor, but a trivial copy ctor) of new warnings).

> --paulr
>
>
>
> *From:* cfe-dev <cfe-dev-bounces at lists.llvm.org> *On Behalf Of *Richard
> Smith via cfe-dev
> *Sent:* Wednesday, August 26, 2020 9:24 PM
> *To:* David Blaikie <dblaikie at gmail.com>
> *Cc:* cfe-dev at lists.llvm.org
> *Subject:* Re: [cfe-dev] std::pair not trivially copyable?
>
>
>
> On Wed, 26 Aug 2020 at 11:42, David Blaikie <dblaikie at gmail.com> wrote:
>
> On Wed, Aug 26, 2020 at 11:34 AM Richard Smith via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
> On Wed, 26 Aug 2020 at 10:18, Nevin Liber via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
> On Tue, Aug 25, 2020 at 4:37 PM David Blaikie via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>
>
> From what I can tell, reading this (
> https://stackoverflow.com/questions/58283694/why-is-pair-of-const-trivially-copyable-but-pair-is-not
> <https://urldefense.com/v3/__https:/stackoverflow.com/questions/58283694/why-is-pair-of-const-trivially-copyable-but-pair-is-not__;!!JmoZiZGBv3RvKRSx!rMuXOBoosPAUEb_L7l9qA3KHSuisIvxQH9YC3nw5t0zEC7c5K6WEpAqMrB47qTd29A$> )
> and the C++17 spec, it just doesn't specify the copy and move assignment
> operators as defaulted, or say anything about their triviality, so they
> aren't trivial even for trivial types. Perhaps an oversight when thinking
> about the other complexities of when they shuold be deleted.
>
>
>
> In general, move/copy assignment cannot be defaulted for pair, because
> assignment of reference types has meaning:
>
>
>
> int i = 2;
>
> int j = 3;
>
> std::pair<int&, int> p1(i, 5);
>
> std::pair<int&, int> p2(j, 7);
>
> p2 = p1;  // j now has the value 2
>
>
>
>
>
> struct A
>
> {
>
>     A(int& r_, int i_) : r(r_), i(i_) {}
>
>     int& r;
>
>     int i;
>
> };
>
>
>
> int i = 2;
>
> int j = 3;
>
> A a1(i, 5);
>
> A a2(j, 7);
>
> a2 = a1;  // Compile time error - deleted copy assignment operator
>
>
>
>
>
> Now,  this doesn't that pair couldn't have trivial copy/move assignment
> operators when it holds trivially copy/move assignable types, but I don't
> know how much of an ABI break this would be.
>
>
>
> Making std::pair be trivially-copyable whenever possible would affect
> whether std::pair is POD for the purpose of layout, which could result in
> an ABI break for at least any code that derives from std::pair or that
> contains a [[no_unique_address]] std::pair member:
> https://godbolt.org/z/GTvo4r
> <https://urldefense.com/v3/__https:/godbolt.org/z/GTvo4r__;!!JmoZiZGBv3RvKRSx!rMuXOBoosPAUEb_L7l9qA3KHSuisIvxQH9YC3nw5t0zEC7c5K6WEpAqMrB6_XykF_w$>
>
>
>
> This is certainly not something we could do for the libc++ stable ABI.
> Maybe for the unstable ABI, but given the above change only makes layout
> worse, it's not clear that there would be a strong motivation. We already
> give std::pair trivial copy/move construction and trivial destruction
> whenever possible, so we can pass and return it efficiently.
>
>
> Interesting - so POD for the purpose of layout is... not good? or more
> restrictive on the layout than if not. (probably an impractical idea, but
> would it be reasonable then to have an attribute that says "not POD for the
> purposes of layout" but I guess that'd have to be cross-compiler/etc/etc...
> probably too much work)
>
>
>
> Once upon a time, C++ said that POD types could be memcpy'd, even if they
> were used as base classes. That rule got changed retroactively by core
> language defect reports, and you're no longer allowed to memcpy base class
> subobjects (nor [[no_unique_address]] members), but too late for the ABI,
> so as a consequence of a rule that we're now pretending never existed, the
> Itanium ABI is careful not to reuse the tail padding of a base class for
> members of the derived class if the base class is "POD for the purpose of
> layout" (which means POD according to some specific old C++ standard
> version).
>
>
>
> An attribute to turn this off might make sense, for classes that really
> care. But so would an ABI-affecting flag to request that we always permit
> tail padding reuse. I do wonder if Clang should have a flag for "give me
> the best ABI you can, I don't care if it's non-standard; I'm building the
> whole world this way with this version of this compiler", just like libc++
> does.
>
>
>
> What's the cost of pair having non-trivial copy assignment? More expensive
> vector of pair resizing (though I guess the copy ctor calls would be
> readily optimized away & LLVM could get that back to a memcpy anyway?)?
>
>
>
> Yes, this is probably only significant for code that's detecting the
> triviality and taking different action based on it. Clang has code in IR
> generation that tries to emit the actual copy assignment operator as a
> single memcpy regardless.
>
>
>
> (the original issue with the warning could be fixed by narrowing the
> warning to only worry about trivial copy construction, not trivial
> copyability in general)
>
>
>
> That sounds right to me. Is it worth also considering the triviality of
> the destructor?
>
>
>
> - Dave
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20200831/f9db27cd/attachment-0001.html>