Disable invalid isPodLike<> specialization

[serge-sans-paille]

Summary

As noted in https://bugs.llvm.org/show_bug.cgi?id=36651, the specialization for isPodLike<std::pair<...>> did not match the expectation of std::is_trivially_copyable which makes the memcpy optimization invalid.

This patch renames the llvm::isPodLike trait into llvm::is_trivially_copyable. Unfortunately std::is_trivially_copyable is not portable across compiler / STL versions. So a portable version is provided too.

Note that the following specialization were invalid:

• std::pair<T0, T1>
• llvm::Optional<T>

Tests have been added to assert that former specialization are respected by the standard usage of llvm::is_trivially_copyable, and that in debug build, and when a decent version of std::is_trivially_copyable is available, llvm::is_trivially_copyable is compared to std::is_trivially_copyable

As of this patch, llvm::Optional is no longer considered trivially copyable, even if T is. this is to be fixed in a later patch, as it has impact on a long-running bug (see r347004)

Note that GCC warns about this UB, but this got silented by https://reviews.llvm.org/D50296.

[dblaikie]

include/llvm/Support/type_traits.h

40–44 ↗

(On Diff #173838)

Does the presence of this macro mean "no one should ever specialize isPodLike"? & if they don't, then its certain that isPodLike == is_trivially_copyable, by definition?

If so, then I'm not sure of the importance of the macro - we should just say "don't specialize isPodLike, ever & leave it at that, maybe?

[serge-sans-paille]

include/llvm/Support/type_traits.h

40–44 ↗

(On Diff #173838)

Almost. It means « if you specialize `isPodLike`, then make sure that for modern compiler, it does not conflict with std::is_trivially_copyable ».

Basically for modern compiler, we should have

`template<class T> using isPodLike = std::is_trivially_copiable<T>;`

However, some compilers supported by LLVM don't implement that feature. In my opinion, it is still valid to speicalize `isPodLike` to ensure decent performance in these case. But not at the cost of UB.

[dblaikie]

include/llvm/Support/type_traits.h

40–44 ↗

(On Diff #173838)

OK, fair enough.

Though does the performance matter on platforms that don't support the real trait? Which compilers/versions is that compared to the set of supported compilers/versions for building Clang/LLVM. (also, in part, the performance of Clang/LLVM is most important on a self-host build - correctness more broadly, but those interest in performance should likely be bootstrapping)

In any case, /if/ we're going to keep that performance feature (whereby LLVM developers can annotate types they know would pass is_trivially_copyable if it were available), probably validating its usage should be kept at the same place as the trait specializations, rather than in the usage?

Any chance of improving/changing Optional/SuccIterator to be trivially copyable when their parameters are trivially copyable?

(& is libc++'s std::pair trivially copyable when T1 and T2 are trivially copyable?)

[serge-sans-paille]

@dblaikie I made a step forward and just relied on std::is_trivially_copyable and disabled specialization of isPodLike (using a type alias). That way the trait is always valid, and the logic is not split among different files.

[serge-sans-paille]

include/llvm/Support/type_traits.h

40–44 ↗

(On Diff #173838)

probably validating its usage should be kept at the same place as the trait specializations, rather than in the usage?

The only solutipon I have for that is to define struct isPodLike : details::isPodLike<T>, perform the check in isPodLike and have user specialize details::isPodLike but nothing would prevent them from directly specializing isPodLike so I'm not a big fan of this soltion.

My best catch would be to define isPodLike as a template alias, that would make it non-speicalizable, no need for extra check and (slightly?) degraded performance for legacy compilers. I have the patch ready for that.

Any chance of improving/changing Optional/SuccIterator to be trivially copyable when their parameters are trivially copyable?

Nop, they overload the assign operator, which need to be defaulted :-/
In the case of Optionnal, I may give it a try, but probably in another commit.

Can't do anything for std::pair :-)

[chandlerc]

FWIW, I really like the direction you and Dave are pursuing. My random thoughts about how to make progress below:

include/llvm/Support/type_traits.h

40–44 ↗

(On Diff #173838)

I think, long-term, we should just be checking for std::is_trivially_copyable as that's what we actually want. And we *should* be fixing the types to actually be trivially copyable as it will also make those types fast in standard library containers.

What I'm having trouble extracting from a quick read here is:

1. What types in LLVM do we want the memcpy optimization for which fail a modern compiler's std::is_trivially_copyable? We should fix those first IMO.

2. What compilers / standard libraries actually cannot implement std::is_trivially_copyable? I suspect we could simulate it very closely (without the specialization hacks) on most if not all of them.

I suspect we should move this to be an llvm::is_trivially_copyable that is implemented via simulation rather than specialization. I suspect we have enough compiler smarts available now, but maybe not. Even if we have to do some limited specialization, I would suggest renaming it to make the intent super clear.

[serge-sans-paille]

What types in LLVM do we want the memcpy optimization for which fail a modern compiler's std::is_trivially_copyable? We should fix those first IMO.

Based on the one that had a isPodLike specialization, and that don't match the std::is_trivially_copyable trait

• std::pair<T0, T1> << this one is out of control
• llvm::Optional<T> << I'll check that one and report
• llvm::SuccIterator<T, U> << itou

What compilers / standard libraries actually cannot implement std::is_trivially_copyable? I suspect we could simulate it very closely (without the specialization hacks) on most if not all of them.

I'll investigate that, but wonder if it's worth the effort.

[serge-sans-paille]

@chandlerc , @dblaikie

clang implements is_trivially_copyable since at least 3.4. gcc implements it since 5.x, and it implements std::_is_trivial which is slighlty stronger but ok since 4.X, as illustrated in https://godbolt.org/z/iBDvYp

I'm currently validating a slight update of llvm::Optional<T> that's trivially copyable when T is, so that point should be okay too.

So I'm going that way, renaming isPodLike into llvm::is_trivially_copyable, slightly improving the support for older GCC version and that should be all fine.

[serge-sans-paille]

With `llvm::Optional` improvement

[serge-sans-paille]

@chandlerc , @dblaikie this should be ready for review.

[efriedma]

include/llvm/ADT/Optional.h

134 ↗

(On Diff #174927)

This looks almost identical to the specialization we had before r347004; does this avoid the miscompiles somehow?

[serge-sans-paille]

include/llvm/ADT/Optional.h

134 ↗

(On Diff #174927)

I double checked this commit, and yes it is important because the generic implementation is not trivially copyable: it has a non default destructor. Knowing that T is trivially copyable makes it possible to have a trivial destructor.

[serge-sans-paille]

Ensure that (almost) all types that used to be marked as `isPodLike` are trivially copyable by inserting the appropriate static_assert in the test suite.

[riccibruno]

In D54472#1306372, @serge-sans-paille wrote:

Ensure that (almost) all types that used to be marked as `isPodLike` are trivially copyable by inserting the appropriate static_assert in the test suite.

What about the types which are marked as isPodLike in clang ? (eg: SourceLocation, QualType, ...)

[serge-sans-paille]

@chandlerc, @efriedma et al. I've done some extra research and tests concerning the specialization of Optional for trivially copyable types.

There has been two set of patches:

1. first set ( r346985, r342966, r322862 ) tries to find a workaround GCC bugs and introduces an ABI issue by specializing only for clang
2. second set (r322859) tries to fix the gcc bug by introducing extra memove / copy instead of placement new.

1. is definitively not a concern for this patch, has we don't have any compiler -specific specialization.
2. is more difficult to accurately reproduce. I've tried the cited version of gcc on this godbolt instance https://godbolt.org/z/Pqh5R1 without issue, and as far as I understand, the only difference between the specialization and the default (*correct*) implementation is the removal of destructor handling. So What I'd like to try, if this review get accepeted, is just commit it and carefully track the bot's result.

[efriedma]

Maybe using the correct definition of "trivially copyable" will fix the gcc issues with Optional, if we're lucky. I guess we can try.

[serge-sans-paille]

Maybe using the correct definition of "trivially copyable" will fix the gcc issues with Optional, if we're lucky. I guess we can try.

I've tested building LLVM + clang and running tests using clang built on top of this patch with gcc 4.8 and I don't have any failure, so I'd say it should work. I will likely split this commit in two, so that the optional stuff can be reverted without affecting the whole `isPodLike` rewrite.

[serge-sans-paille]

Include clang changes + prune out the Optional stuff, so that it can be validated in a separate commit.

[dblaikie]

(honestly, I think I've lost the thread on this - between the reviews to define a trivially copyable attribute (not your work, but it's happened around the same time/recently - Arther O'Dwyer's D50119 phab review), these changes, the different aspects for different compilers, the different design choices/iterations this has gone through, the different classes that need to be handled or functionality dropped, etc)

Might benefit from a current summary?

and/or might be better to separate these in some way - eg: for the obvious/easy cases where removing the isPodLike specialization is benign because is_trivially_copyable already does the right thing, those should be easy - they don't regress functionality, they work with the existing implementation, or any newer/narrower implementation, etc?

Is the patch description up to date anymore? It doesn't look like it describes the Optional changes as they are now (doesn't look like they revert r347004 currently - and if they did that'd require some care/attention to address the reasons r347004 was committed in the first place), but I'm not sure.

[• Quuxplusone]

include/llvm/ADT/SmallVector.h

185 ↗

(On Diff #175453)

Incidentally, unless there's a portability issue I'm not aware of, I would prefer to write this as

template <typename T, bool = is_trivially_copyable<T>::value>
class SmallVectorTemplateBase : public SmallVectorTemplateCommon<T> {

so that on line ~322 you could write simply

template <typename T>
class SmallVectorImpl : public SmallVectorTemplateBase<T> {
  using SuperClass = SmallVectorTemplateBase<T>;

without repeating your is_trivially_copyable<T>::value all over the place.

The more times you have to manually type out is_trivially_copyable<T>::value, the greater the chance you'll slip up and write std::is_trivially_copyable<T>::value on autopilot.

include/llvm/ADT/bit.h

45 ↗

(On Diff #175453)

Should this say "is_trivially_copyable"?

[serge-sans-paille]

Might benefit from a current summary?

@dblaikie current state is a simplification of the original patch : no implementation change of llvm::Optional, nothing on SmallVector either, in order to keep it relatively simple while fixing the original UB bug. I've updated the summary accordingly.

I'll deal with Optional update in another patch.

[serge-sans-paille]

include/llvm/ADT/bit.h

45 ↗

(On Diff #175453)

Yep, fixed

[chandlerc]

While I like the update (keeping any potential changes to Optional and SmallVector separate seems good) I have a serious concern that this has the same fundamental problem as the prior mentioned ABI issue.

Specifically, if we end up mixing .o files from GCC and Clang, they may have subtly different ABIs due to different types being considered "trivially copyable". That can in turn cause these .o files to expect different specializations of SmallVector (or any other type utilizing this trait). And that in turn can lead to horrible ABI issues.

So, at the very least, I do not think we can mix std::is_trivial and std::is_trivially_copyable in this way.

If we want to do this, we have to use predicates that will be *ABI stable* between compilers on the same platform where we might reasonably mix .o files. This is... challenging.

I believe the most portable and reliably implementations I'm aware of don't *technically* implement std::is_trivially_copyable. Instead, they separately implement std::is_trivially_copy_constructible and std::is_trivially_copy_assignable. I believe we could change LLVM's existing usag of isPodLike to use these traits and then implement them using the same core technique used by Abseil here:
https://github.com/abseil/abseil-cpp/blob/master/absl/meta/type_traits.h#L291-L295
https://github.com/abseil/abseil-cpp/blob/master/absl/meta/type_traits.h#L325-L329

I'm still not 100% sure this is going to be reliable from an ABI perspective. I'm asking a group of Abseil and C++ ABI experts to see if they see any problems here.

include/llvm/ADT/SmallVector.h

185 ↗

(On Diff #175453)

While this might be an independently nice improvement, I agree w/ the current mechanics of the patch to make the diff more isolated.

[serge-sans-paille]

Simple ABI breakage reproducer:

compile the following codes and run nm on the generated objects, with clang 6 and gcc 4.8

#include <llvm/ADT/StringRef.h>
#include <llvm/ADT/SmallVector.h>

llvm::SmallVector<llvm::StringRef, 4> ff;

the list of symbol differ because a different SmallVector specialization is used...

[dexonsmith]

include/llvm/ADT/SmallVector.h

185 ↗

(On Diff #175453)

It doesn't seem completely independent to me, because this patch introduces repeated instances of (reader's) ambiguity between is_trivially_copyable and std::is_trivially_copyable.

Can the mechanical change suggested by @Quuxplusone be implemented as an initial NFC commit on isPodLike (and then this patch rebased on top)? If so, that would avoid cluttering this diff.

[serge-sans-paille]

include/llvm/ADT/SmallVector.h

185 ↗

(On Diff #175453)

Patch proposed in https://reviews.llvm.org/D55005

[serge-sans-paille]

https://godbolt.org/z/_gK-9m provides a first draft of a compiler-agnostic implementation of `std::is_trivially_copyable`. It uses no builtin and compiles fine with gcc 4.9. I'll do more testing soon.

[serge-sans-paille]

Lighter version, using bits of std:: but still works for gcc 4.9: https://godbolt.org/z/Dq9pMk

[• Quuxplusone]

include/llvm/ADT/SmallVector.h

333 ↗

(On Diff #176110)

The , is_trivially_copyable<T>::value can safely be removed here. Sorry I missed it on the other review.

include/llvm/Analysis/BlockFrequencyInfoImpl.h

188 ↗

(On Diff #176110)

Unintended diff? Also, surely noexcept(true) a.k.a. noexcept would be the default here anyway. If anything wants noexcept, it'd be line 189, not line 188. :)

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

This last clause — (has_trivial_copy_constructor || has_trivial_copy_assign || has_trivial_move_constructor || has_trivial_move_assign) — does not correspond to anything in the Standard. So:

https://godbolt.org/z/R5hCff

struct S {
    S(S&&) = delete;
};
static_assert(is_trivially_copyable<S>::value);

error: static_assert failed "consistent behavior"
  static_assert(value == std::is_trivially_copyable<T>::value, "consistent behavior");
  ^             ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I'm not sure but I would suggest just getting rid of this last clause.

tools/clang/lib/Sema/SemaChecking.cpp

11553 ↗

(On Diff #176110)

Unintended diff? But harmless.

unittests/ADT/ArrayRefTest.cpp

252 ↗

(On Diff #176110)

Why does this need the guard? You're not using the std:: version here.
(And likewise on the next 7-or-so diffs.)

[serge-sans-paille]

include/llvm/Analysis/BlockFrequencyInfoImpl.h

188 ↗

(On Diff #176110)

That's unfortunate but intended: for some reason, and for some clang version, the type trait `is_trivially_copyable` leads to this being required. There's no such issue with the gcc version I compiled clang with.

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

From https://en.cppreference.com/w/cpp/named_req/TriviallyCopyable (yeah that's not the standard, but I do hope it's derived from the standard)

at least one copy constructor, move constructor, copy assignment operator, or move assignment operator is non-deleted

That's the intent I was trying to capture.

I'm okay with getting rid of this capture, especially when looking at https://godbolt.org/z/p3-Cy4.

tools/clang/lib/Sema/SemaChecking.cpp

11553 ↗

(On Diff #176110)

Same as above, omitting this change leads to error in type traits instantiation for some compiler version.

[serge-sans-paille]

Rebase against master + minor fixes + take into account reviews

[chandlerc]

include/llvm/ADT/PointerIntPair.h

133–134 ↗

(On Diff #176577)

The compiler may pass __has_feature(is_trivially_copyable) and yet the standard library not provide std::is_trivially_copyable. And I don't see any feature test spelled is_trivially_copyable? Do you mean __has_trivially_copy, which is the Clang and GCC builtin?

Also, the message is printed on an *error*.

[serge-sans-paille]

include/llvm/ADT/PointerIntPair.h

133–134 ↗

(On Diff #176577)

The legacy code for isPodLike was using the very same guard without issue, so I tend to think it's okay. +1 for the error stuff, I'll update that.

[• Quuxplusone]

include/llvm/ADT/PointerIntPair.h

133–134 ↗

(On Diff #176577)

I see Clang providing both __has_feature(has_trivial_copy) and __has_feature(is_trivially_copyable) (using exactly the spellings I just wrote).
GCC doesn't support __has_feature at all, so a bunch of places in LLVM do #define __has_feature(x) 0.
I think this snippet is OK as written.

include/llvm/Analysis/BlockFrequencyInfoImpl.h

188 ↗

(On Diff #176110)

That's extremely weird and scary. Is it worth digging further into why that happens?

[serge-sans-paille]

include/llvm/Analysis/BlockFrequencyInfoImpl.h

188 ↗

(On Diff #176110)

I reduced the test case to that: https://godbolt.org/z/briQXa
This is gcc-specific, and adding the noexcept clause fixes the bug.

[serge-sans-paille]

include/llvm/Analysis/BlockFrequencyInfoImpl.h

188 ↗

(On Diff #176110)

Bug confirmed and reported as https://gcc.gnu.org/bugzilla/show_bug.cgi?id=88368

[serge-sans-paille]

@chandlerc concerning union as a way to detect non-trivial operations, according to https://en.cppreference.com/w/cpp/language/union

If a union contains a non-static data member with a non-trivial special member function (copy/move constructor, copy/move assignment, or destructor), that function is deleted by default in the union and needs to be defined explicitly by the programmer.

So I guess it's a decent way to implement these checks.

[serge-sans-paille]

@Quuxplusone + @chandlerc : looks good now!

[• Quuxplusone]

LGTM at this point modulo my two comments (one important but trivial, the other completely trivial).
I don't know whether Chandler's concern about __has_feature in "include/llvm/ADT/PointerIntPair.h" has been 100% resolved yet — @chandlerc?

include/llvm/ADT/PointerIntPair.h

133–134 ↗

(On Diff #176577)

Wait, you definitely need something like

using T = PointerIntPair<PointerTy, IntBits, IntType, PtrTraits, Info>;

before this line, or else it won't know what T is here! (Probably means you should find a way to compile without NDEBUG before merging. :))

tools/llvm-xray/trie-node.h

45 ↗

(On Diff #176638)

Extraneous diff.

[serge-sans-paille]

@Quuxplusone this patch has been rebuilt without NDEBUG, which triggered a few minor changes I have commented above. If @chandlerc is fine with current state, we should be able to merge it. Then move on to the llvm::Optional special case.

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

@Quuxplusone I had to reintroduce the (has_trivial_move_assign || has_trivial_move_constructor || has_trivial_copy_assign || has_trivial_copy_constructor) condition, which is indeed a standard requirement. It basically states « if the class is not meant to be copied, don't allow to copy it ».

[• Quuxplusone]

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

The standard trait "trivially copyable" is not related to copy-{constructibility,assignability}; it's its own thing.

I insist that if you're naming your thing "trivially copyable," it should exactly match the Standard trait's behavior, i.e., it should succeed at this test case:

struct S {
    S(S&&) = delete;
};
static_assert(is_trivially_copyable<S>::value);

Your current code doesn't pass that test. https://godbolt.org/z/j6pkaS

Any users of is_trivially_copyable should use it only to enable perf optimizations. They shouldn't be using it to ask, "Should I make a copy of this?" First of all, "make a copy" is underspecified. What they should have asked is "Should I copy-construct this?" or "Should I copy-assign this?" And we already have traits for that. Then, once they've already decided to make a copy (or maybe not to make any copies, but just to move the value without copying), then they should look at is_trivially_copyable to decide how to make that copy.

[serge-sans-paille]

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

I totally agree on your point. Unfortunately, I don't think it's possible to achieve a portable implementation of is_trivially_copyable without compiler support for all supported compiler version (including gcc 4.9). And that's exactly why this patch exist: to cope with this limitation, while ensuring the memcpy are valid, which is not the case with current implementation (which is both invalid for some type/compiler combination, and not conservative with respect to ABI because of the impact of isPodLike on ADT like SmallVector. So I would advocate for renaming this trait is_memc(o)pyable and ensuring that forall T instanciated in LLVM where std::is_trivially_copyable matters, is_memcopyable<T>::value == true implies std::is_trivially_copyable<T> == true, which is currently the case (as we actually even verify that is_memcopyable<T>::value == std::is_trivially_copyable<T>::value).

Does that sound OK to you?

[• Quuxplusone]

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

I think that is_memcpyable sounds underdetermined: it ought to be is_move_assignable_via_memcpy, is_copy_constructible_via_memcpy, or whatever operation we're asserting is tantamount to memcpy. At which point you've just reinvented is_trivially_move_assignable, is_trivially_copy_constructible etc. (and here I'll insert a shameless plug for D50119 is_trivially_relocatable, because quite often the operation we want to replace is "relocating", not merely "copy-constructing" or "move-assigning").

The situation is complicated by the fact that right now the C++17 standard specifies that memcpying is technically permitted only for trivially copyable types. So if by is_memcpyable you mean "is memcpying technically permitted for this type under any circumstances — never mind whether it's semantically correct —" then is_memcpyable would be just a synonym for std::is_trivially_copyable. (But then specializing it would be UB.)

I think this patch has been evolving toward "Provide an llvm::is_trivially_copyable trait that is exactly the same as std::is_trivially_copyable, but portable." I think it should keep going in that direction.

If we say "Maybe the trait shouldn't be is_trivially_copyable at all, but something else," then that reopens the can of worms. (At that point, why not just keep the name isPodLike?)

[• Quuxplusone]

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

But on the other hand, I see that MSVC's standard library agrees with you that an immovable type is not is_trivially_copyable. (GCC/libstdc++, Clang/libc++, and ICC agree with me that it's unrelated.)
https://godbolt.org/z/Gv_hcj
So we'll never get value == std::is_trivially_copyable<T>::value to pass on all platforms.

So I won't block this review over the (has_trivial_move_assign || has_trivial_move_constructor || has_trivial_copy_assign || has_trivial_copy_constructor) condition. If you want it, you can keep it. But I hope that that line isn't necessary for correctness! I mean, I hope that if someone removes that line in the future, or switches from llvm::is_trivially_copyable to std::is_trivially_copyable, it won't cause silent breakage of any of LLVM's data structures. Is that reasonable?

[serge-sans-paille]

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

So I won't block this review

Cool.

switches from llvm::is_trivially_copyable to std::is_trivially_copyable

In current state, thanks to the assert, llvm::is_trivially_copyable and std::is_trivially_copyable are equivalent *with respect to LLVM codebase*.

[chandlerc]

Given the ... very subtle ABI implications here, I really want Richard to look at this and make sure we're not barking up the wrong tree.

Also I think the example raised in https://godbolt.org/z/59_R-J is a bug in all but MSVC, although I'm not the expert here and Richard may have DRs or issues to cite. My reading of the standard supports the implementation of is_trivially_copyable in this patch and in MSVC. Specifically, [class.prop]p1.2 in the latest draft has the *exact* requirement that this code enforces: one of the copy or move constructors or assignment operators must be non-deleted. And the example given has all of those deleted (all but one because they are defined as deleted implicitly IIRC). But again, happy to defer to Richard here, this is a subtle and complex part of the standard.

include/llvm/ADT/PointerIntPair.h

128–129 ↗

(On Diff #178865)

This comment doesn't really make sense here...

I would say something more about the *effect* of this, not the symptom that made you add it. So, talk about the fact that this allows even an incomplete PointerIntPair type (or some such) to be known to be trivially copyable.

include/llvm/Support/type_traits.h

123–124 ↗

(On Diff #178865)

Can you re-flow this comment to fit in 80-columns?

128–154 ↗

(On Diff #178865)

All of this should be private, no?

169–175 ↗

(On Diff #178865)

Why is this needed? Due to incomplete types? (answer in a comment please!)

[serge-sans-paille]

@chandlerc: thanks for the final review! everything should be okay right now, waiting for @rsmith for the final call.

[serge-sans-paille]

@rsmith: up

[rsmith]

include/llvm/ADT/PointerIntPair.h

132 ↗

(On Diff #179822)

Why is this guarded by NDEBUG?

133 ↗

(On Diff #179822)

I am confused by this. If the type in question is incomplete, how can this static_assert work? (And if it's complete, why do you need the specialization at all?) Have you made sure this actually compiles in a Debug build?

If you need this, I think what you should probably do instead is to put this static_assert inside PointerIntPair, in some member function that is commonly instantiated, such as the default constructor. (It can't go in the body of the class itself because the class isn't complete there.)

include/llvm/ADT/SmallVector.h

240 ↗

(On Diff #179822)

Please give this a suitable, LLVM-style-conforming name that doesn't shadow the trait. Maybe TriviallyCopyable?

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

The problem here is a bug in GCC and Clang's __is_trivially_copyable, not a bug in this patch. Example: https://godbolt.org/z/XKaLOa (The specific issue is that neither compiler implements DR1734 yet.)

168 ↗

(On Diff #179822)

Why check NDEBUG here?

169–175 ↗

(On Diff #178865)

The comment still doesn't explain why you need this. Why would the above implementation not work for pointers to incomplete types? (As far as I can see, the above implementation should work fine in that case, and works fine in practice: https://godbolt.org/z/FJ09dy)

[serge-sans-paille]

Take into account @rsmith review.

include/llvm/ADT/PointerIntPair.h

132 ↗

(On Diff #179822)

To limit compile-time overhead.

include/llvm/Support/type_traits.h

168 ↗

(On Diff #179822)

It's debug code, to ensure the consistency of llvm::is_trivially_copyable with std:: version. I assume ignoring this on release build make them faster.

169–175 ↗

(On Diff #178865)

That definitively was a left over. I removed it in the updated patch.

[serge-sans-paille]

include/llvm/ADT/PointerIntPair.h

133 ↗

(On Diff #179822)

If the type in question is incomplete, how can this static_assert work?

My *guess* is that std::is_trivially_copyable

Have you made sure this actually compiles in a Debug build?

yes. Double checked right now!

[rsmith]

include/llvm/ADT/PointerIntPair.h

132 ↗

(On Diff #179822)

I don't think that's worth the loss of safety, particularly for folks who aren't clang developers and just try to make a release build with an untested toolchain. I would prefer to take the (likely tiny) compile time cost and always have the check enabled.

133 ↗

(On Diff #179822)

Some of your comment seems to have gotten lost here. I still don't see how this can be right (if the type is complete, then this specialization should be unnecessary, and if it's incomplete, then this use of std::is_trivially_copyable should be ill-formed). Have you tried removing this specialization entirely to see if it's actually necessary? If so, what compile error do you get?

include/llvm/Support/type_traits.h

168 ↗

(On Diff #179822)

(As for the other case, I would prefer that we always static_assert this, even in release builds.)

[serge-sans-paille]

For some reason, all the specialization were not required, so we end up with a very clean implementation. Thanks a lot @rsmith for pointing that out!

[• Quuxplusone]

LGTM!

docs/ProgrammersManual.rst

1459 ↗

(On Diff #182284)

Bikeshed: This isn't technically an advantage over std::vector, since std::vector also does this. :)

I do think you should explicitly say llvm::is_trivially_copyable<Type> here, to distinguish it from std::is_trivially_copyable<Type> (which IIUC should not be used anywhere in the LLVM codebase). But maybe the llvm:: qualification would be implicitly assumed by people who work with LLVM frequently?

[chandlerc]

docs/ProgrammersManual.rst

1459 ↗

(On Diff #182284)

Not all implementations of vector reliably implement this optimization is I think what this is trying to say.

[serge-sans-paille]

@rsmith thanks a lot for the final review. I rebased the patch, did a final check with both GCC and Clang, and everything looks fine now. Waiting for a green light!

docs/ProgrammersManual.rst

1459 ↗

(On Diff #182284)

I totally agreee on std::vector using the same technique. That's independent from that commit though :-)

And yeah, better be explicit about llvm::is_trivially_copyable.

include/llvm/Analysis/BlockFrequencyInfoImpl.h

188 ↗

(On Diff #182506)

I still needed to rewrite it that way. Clang was happy with the original version bug gcc wasn't (i.e. it made std:: and llvm:: version behave differently).

include/llvm/CodeGen/DIE.h

803 ↗

(On Diff #182506)

This class has a virtual member, so it needs a virtual destructor. And without that qualifier, std:: and llvm:: behave differently.

include/llvm/Support/type_traits.h

163 ↗

(On Diff #176110)

The standard trait "trivially copyable" is not related to copy-{constructibility,assignability}; it's its own thing.

Again, when recompiling with both GCC and Clang, I realized that they both agree with you; i double checked the clang source code and it also agrees with you, so I removed the final condition.

[rsmith]

Thanks, looks great!

[fedor.sergeev]

I'm sorry to bring bad news, but this fails for our gcc 4.9.2 build, when building llvm/tools/llvm-xray/xray-converter.cpp.
Failures looks like this:

] g++   -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Itools/llvm-xray -I/llvm/tools/llvm-xray -Iinclude -I/home/fsergeev/work/llvm-upstre
am/llvm/include -fPIC -fvisibility-inlines-hidden -Werror=date-time -std=c++11  -fdiagnostics-color -ffunction-sections -fdata-sections -O2 -g    -UNDEBUG  -fno-exceptions -fno-rtti  -c /llvm/tools/llvm-xr
ay/xray-converter.cpp 
In file included from /llvm/tools/llvm-xray/xray-converter.cpp:14:0:
/llvm/tools/llvm-xray/trie-node.h: In instantiation of ‘struct TrieNode<{anonymous}::StackIdData>’:
gcc/include/c++/4.9.2/type_traits:1183:45:   required by substitution of ‘template<class _Tp1, class _Up1, class> static std::true_type std::__is_assignable_helper<_Tp, _Up>::__test
(int) [with _Tp1 = llvm::detail::trivial_helper<TrieNode<{anonymous}::StackIdData>*>&; _Up1 = const llvm::detail::trivial_helper<TrieNode<{anonymous}::StackIdData>*>&; <template-parameter-1-3> = <missing>]’
gcc/include/c++/4.9.2/type_traits:1192:30:   required from ‘class std::__is_assignable_helper<llvm::detail::trivial_helper<TrieNode<{anonymous}::StackIdData>*>&, const llvm::detail:
:trivial_helper<TrieNode<{anonymous}::StackIdData>*>&>’
gcc/include/c++/4.9.2/type_traits:1197:12:   required from ‘struct std::is_assignable<llvm::detail::trivial_helper<TrieNode<{anonymous}::StackIdData>*>&, const llvm::detail::trivial
_helper<TrieNode<{anonymous}::StackIdData>*>&>’
gcc/include/c++/4.9.2/type_traits:1209:12:   required from ‘struct std::__is_copy_assignable_impl<llvm::detail::trivial_helper<TrieNode<{anonymous}::StackIdData>*>, true>’
gcc/include/c++/4.9.2/type_traits:1215:12:   required from ‘struct std::is_copy_assignable<llvm::detail::trivial_helper<TrieNode<{anonymous}::StackIdData>*> >’
/llvm/include/llvm/Support/type_traits.h:142:25:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<{anonymous}::StackIdData>*>::has_trivial_copy_assign’
/llvm/include/llvm/Support/type_traits.h:163:32:   required from ‘constexpr const bool llvm::is_trivially_copyable<TrieNode<{anonymous}::StackIdData>*>::value’
/llvm/include/llvm/ADT/SmallVector.h:321:7:   required from ‘class llvm::SmallVectorImpl<TrieNode<{anonymous}::StackIdData>*>’
/llvm/include/llvm/ADT/SmallVector.h:845:7:   required from ‘class llvm::SmallVector<TrieNode<{anonymous}::StackIdData>*, 4u>’
/llvm/tools/llvm-xray/xray-converter.cpp:174:43:   required from here
/llvm/tools/llvm-xray/trie-node.h:38:52: error: ‘TrieNode<AssociatedData>::Callees’ has incomplete type
   llvm::SmallVector<TrieNode<AssociatedData> *, 4> Callees;
                                                    ^
In file included from /llvm/include/llvm/ADT/STLExtras.h:20:0,
                 from /llvm/include/llvm/ADT/StringRef.h:12,
                 from /llvm/include/llvm/ADT/StringMap.h:16,
                 from /llvm/include/llvm/Support/Host.h:16,
                 from /llvm/include/llvm/ADT/Hashing.h:48,
                 from /llvm/include/llvm/ADT/ArrayRef.h:12,
                 from /llvm/include/llvm/ADT/DenseMapInfo.h:16,
                 from /llvm/include/llvm/ADT/DenseMap.h:16,
                 from /llvm/tools/llvm-xray/func-id-helper.h:15,
                 from /llvm/tools/llvm-xray/xray-converter.h:16,
                 from /llvm/tools/llvm-xray/xray-converter.cpp:12:

Here I have a reproducer that shows the problem on godbolt:

https://gcc.godbolt.org/z/Tgk7M0

[Anastasia]

The same here! I don't think we should break builds of compiler versions that are still officially supported.

[fedor.sergeev]

And the smallest testcase I'v been able to come up with:

#include <type_traits>
template<class T>
union trivial_helper {
    T t;
};
template <typename AssociatedData> struct TrieNode {
  AssociatedData ExtraData;
};
struct StackIdData {
  int x [std::is_copy_assignable<trivial_helper<TrieNode<StackIdData>*>>::value + 1];
};
...
copyable_test_small.cpp: In instantiation of ‘struct TrieNode<StackIdData>’:
gcc/include/c++/4.9.2/type_traits:1183:45:   required by substitution of ‘template<class _Tp1, class _Up1, class> static std::true_type std::__is_assignable_helper<_Tp, _Up>::__test(int) [with _Tp1 = trivial_helper<TrieNode<StackIdData>*>&; _Up1 = const trivial_helper<TrieNode<StackIdData>*>&; <template-parameter-1-3> = <missing>]’
gcc/include/c++/4.9.2/type_traits:1192:30:   required from ‘class std::__is_assignable_helper<trivial_helper<TrieNode<StackIdData>*>&, const trivial_helper<TrieNode<StackIdData>*>&>’
gcc/include/c++/4.9.2/type_traits:1197:12:   required from ‘struct std::is_assignable<trivial_helper<TrieNode<StackIdData>*>&, const trivial_helper<TrieNode<StackIdData>*>&>’
gcc/include/c++/4.9.2/type_traits:1209:12:   required from ‘struct std::__is_copy_assignable_impl<trivial_helper<TrieNode<StackIdData>*>, true>’
gcc/include/c++/4.9.2/type_traits:1215:12:   required from ‘struct std::is_copy_assignable<trivial_helper<TrieNode<StackIdData>*> >’
copyable_test_small.cpp:10:73:   required from here
copyable_test_small.cpp:7:18: error: ‘TrieNode<AssociatedData>::ExtraData’ has incomplete type
   AssociatedData ExtraData;
                  ^
copyable_test_small.cpp:9:8: error: forward declaration of ‘struct StackIdData’
 struct StackIdData {
        ^

It does not fail if trivial_helper wrapper is removed from the construct (just passing TrieNode to is_copy_assignable).
So, llvm::detail::trivial_helper triggers something inside the gcc4.9.2 traits implementation.
I'm not sure what the workaround could be...

[sylvestre.ledru]

@serge-sans-paille
This (probably) broke LLVM on Debian jessie with gcc 4.9. It fails with:

/build/llvm-toolchain-snapshot-9~svn351724/include/llvm/ADT/PointerIntPair.h: In instantiation of 'struct llvm::PointerIntPairInfo<clang::Module*, 2u, llvm::PointerLikeTypeTraits<clang::Module*> >':
/build/llvm-toolchain-snapshot-9~svn351724/tools/clang/include/clang/Lex/ModuleMap.h:158:29: required from here
/build/llvm-toolchain-snapshot-9~svn351724/include/llvm/ADT/PointerIntPair.h:133:3: error: static assertion failed: PointerIntPair with integer size too large for pointer
static_assert(IntBits <= PtrTraits::NumLowBitsAvailable,
^

[serge-sans-paille]

Yeah, I knew from the start that compiling with gcc 8 and clang 6 was not enough for my tests. I've already fixed a few issues with gcc 6, let me try to move forward.

@Anastasia @fedor.sergeev : That's ultra painful :-/ I'm looking for a way to fix that for gcc 4.9, if I can't find a good solution I'm afraid we'll have to roll back. Thanks for providing a minimal test case.

@sylvestre.ledru I'll have a look at you problem later on.

[serge-sans-paille]

https://godbolt.org/z/KaJ2is seems to be aportable implementaiton of `std::is_copy_assignable`. I'm investigating that.

[serge-sans-paille]

Patch to be tested available on https://reviews.llvm.org/D57018

[Anastasia]

In D54472#1365186, @sylvestre.ledru wrote:

@serge-sans-paille
This (probably) broke LLVM on Debian jessie with gcc 4.9. It fails with:

/build/llvm-toolchain-snapshot-9~svn351724/include/llvm/ADT/PointerIntPair.h: In instantiation of 'struct llvm::PointerIntPairInfo<clang::Module*, 2u, llvm::PointerLikeTypeTraits<clang::Module*> >':
/build/llvm-toolchain-snapshot-9~svn351724/tools/clang/include/clang/Lex/ModuleMap.h:158:29: required from here
/build/llvm-toolchain-snapshot-9~svn351724/include/llvm/ADT/PointerIntPair.h:133:3: error: static assertion failed: PointerIntPair with integer size too large for pointer
static_assert(IntBits <= PtrTraits::NumLowBitsAvailable,
^

Actually the issue I am seeing it this too, but I don't feel it belongs to this commit... after reverting it the build still seem to fail in the same way. Perhaps we have some other problem too.

[efriedma]

FYI, this is failing on the polly-arm-linux builder (http://lab.llvm.org:8011/builders/polly-arm-linux). I guess that builder isn't sending emails to people who break it at the moment?

It looks like https://reviews.llvm.org/D57018 will fix it.

cfe/trunk/include/clang/AST/BaseSubobject.h

template<> struct DenseMapInfo<clang::BaseSubobject> {
   }
 
   static bool isEqual(const clang::BaseSubobject &LHS,
                       const clang::BaseSubobject &RHS) {
     return LHS == RHS;
   }
 };
 
-// It's OK to treat BaseSubobject as a POD type.
-template <> struct isPodLike<clang::BaseSubobject> {
-  static const bool value = true;
-};
-
 } // namespace llvm
 
 #endif // LLVM_CLANG_AST_BASESUBOBJECT_H

cfe/trunk/include/clang/AST/CharUnits.h

template<> struct DenseMapInfo<clang::CharUnits> {
   }
 
   static bool isEqual(const clang::CharUnits &LHS,
                       const clang::CharUnits &RHS) {
     return LHS == RHS;
   }
 };
 
-template <> struct isPodLike<clang::CharUnits> {
-  static const bool value = true;
-};
-
 } // end namespace llvm
 
 #endif // LLVM_CLANG_AST_CHARUNITS_H

cfe/trunk/include/clang/AST/DeclAccessPair.h

void set(NamedDecl *D, AccessSpecifier AS) {
     Ptr = uintptr_t(AS) | reinterpret_cast<uintptr_t>(D);
   }
 
   operator NamedDecl*() const { return getDecl(); }
   NamedDecl *operator->() const { return getDecl(); }
 };
 }
 
-// Take a moment to tell SmallVector that DeclAccessPair is POD.
-namespace llvm {
-template<typename> struct isPodLike;
-template<> struct isPodLike<clang::DeclAccessPair> {
-   static const bool value = true;
-};
-}
-
 #endif

cfe/trunk/include/clang/AST/DeclarationName.h

struct DenseMapInfo<clang::DeclarationName> {
   }
 
   static inline bool
   isEqual(clang::DeclarationName LHS, clang::DeclarationName RHS) {
     return LHS == RHS;
   }
 };
 
-template <>
-struct isPodLike<clang::DeclarationName> { static const bool value = true; };
-
 } // namespace llvm
 
 #endif // LLVM_CLANG_AST_DECLARATIONNAME_H

cfe/trunk/include/clang/AST/ExprObjC.h

struct ObjCDictionaryElement {
   Optional<unsigned> NumExpansions;
 
   /// Determines whether this dictionary element is a pack expansion.
   bool isPackExpansion() const { return EllipsisLoc.isValid(); }
 };
 
 } // namespace clang
 
-namespace llvm {
-
-template <> struct isPodLike<clang::ObjCDictionaryElement> : std::true_type {};
-
-} // namespace llvm
-
 namespace clang {
 
 /// Internal struct for storing Key/value pair.
 struct ObjCDictionaryLiteral_KeyValuePair {
   Expr *Key;
   Expr *Value;
 };
 

cfe/trunk/include/clang/AST/GlobalDecl.h

template<> struct DenseMapInfo<clang::GlobalDecl> {
     }
 
     static bool isEqual(clang::GlobalDecl LHS,
                         clang::GlobalDecl RHS) {
       return LHS == RHS;
     }
   };
 
-  // GlobalDecl isn't *technically* a POD type. However, its copy constructor,
-  // copy assignment operator, and destructor are all trivial.
-  template <>
-  struct isPodLike<clang::GlobalDecl> {
-    static const bool value = true;
-  };
-
 } // namespace llvm
 
 #endif // LLVM_CLANG_AST_GLOBALDECL_H

cfe/trunk/include/clang/AST/Type.h

struct PointerLikeTypeTraits< ::clang::ExtQuals*> {
 
     static inline ::clang::ExtQuals *getFromVoidPointer(void *P) {
       return static_cast< ::clang::ExtQuals*>(P);
     }
 
     enum { NumLowBitsAvailable = clang::TypeAlignmentInBits };
   };
 
-  template <>
-  struct isPodLike<clang::QualType> { static const bool value = true; };
-
 } // namespace llvm
 
 namespace clang {
 
 class ASTContext;
 template <typename> class CanQual;
 class CXXRecordDecl;
 class DeclContext;

cfe/trunk/include/clang/Analysis/ProgramPoint.h

 
 static bool isEqual(const clang::ProgramPoint &L,
                     const clang::ProgramPoint &R) {
   return L == R;
 }
 
 };
 
-template <>
-struct isPodLike<clang::ProgramPoint> { static const bool value = true; };
-
 } // end namespace llvm
 
 #endif

cfe/trunk/include/clang/Basic/IdentifierTable.h

struct DenseMapInfo<clang::Selector> {
 
   static unsigned getHashValue(clang::Selector S);
 
   static bool isEqual(clang::Selector LHS, clang::Selector RHS) {
     return LHS == RHS;
   }
 };
 
-template <>
-struct isPodLike<clang::Selector> { static const bool value = true; };
-
-template<>
+template<>
 struct PointerLikeTypeTraits<clang::Selector> {
   static const void *getAsVoidPointer(clang::Selector P) {
     return P.getAsOpaquePtr();
   }
 
   static clang::Selector getFromVoidPointer(const void *P) {
     return clang::Selector(reinterpret_cast<uintptr_t>(P));
   }

cfe/trunk/include/clang/Basic/SourceLocation.h

 #include <cassert>
 #include <cstdint>
 #include <string>
 #include <utility>
 
 namespace llvm {
 
 template <typename T> struct DenseMapInfo;
-template <typename T> struct isPodLike;
 
 } // namespace llvm
 
 namespace clang {
 
 class SourceManager;
 
 /// An opaque identifier used by SourceManager which refers to a
static unsigned getHashValue(clang::FileID S) {
       return S.getHashValue();
     }
 
     static bool isEqual(clang::FileID LHS, clang::FileID RHS) {
       return LHS == RHS;
     }
   };
 
-  template <>
-  struct isPodLike<clang::SourceLocation> { static const bool value = true; };
-  template <>
-  struct isPodLike<clang::FileID> { static const bool value = true; };
-
   // Teach SmallPtrSet how to handle SourceLocation.
   template<>
   struct PointerLikeTypeTraits<clang::SourceLocation> {
     enum { NumLowBitsAvailable = 0 };
 
     static void *getAsVoidPointer(clang::SourceLocation L) {
       return L.getPtrEncoding();
     }

cfe/trunk/include/clang/Lex/Token.h

struct PPConditionalInfo {
 
   /// True if we've seen a \#else in this block.  If so,
   /// \#elif/\#else directives are not allowed.
   bool FoundElse;
 };
 
 } // end namespace clang
 
-namespace llvm {
-  template <>
-  struct isPodLike<clang::Token> { static const bool value = true; };
-} // end namespace llvm
-
 #endif // LLVM_CLANG_LEX_TOKEN_H

cfe/trunk/include/clang/Sema/CodeCompleteConsumer.h

CodeCompletionAllocator &getAllocator() const {
     return *AllocatorRef;
   }
 
   StringRef getParentName(const DeclContext *DC);
 };
 
 } // namespace clang
 
-namespace llvm {
-
-template <> struct isPodLike<clang::CodeCompletionString::Chunk> {
-  static const bool value = true;
-};
-
-} // namespace llvm
-
 namespace clang {
 
 /// A builder class used to construct new code-completion strings.
 class CodeCompletionBuilder {
 public:
   using Chunk = CodeCompletionString::Chunk;
 
 private:

cfe/trunk/include/clang/Sema/Ownership.h

static inline void *getAsVoidPointer(clang::OpaquePtr<T> P) {
       return P.getAsOpaquePtr();
     }
 
     static inline clang::OpaquePtr<T> getFromVoidPointer(void *P) {
       return clang::OpaquePtr<T>::getFromOpaquePtr(P);
     }
   };
 
-  template <class T>
-  struct isPodLike<clang::OpaquePtr<T>> { static const bool value = true; };
-
 } // namespace llvm
 
 namespace clang {
 
   // Basic
   class DiagnosticBuilder;
 
   // Determines whether the low bit of the result pointer for the

cfe/trunk/include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h

 };
 
 } // namespace loc
 
 } // namespace ento
 
 } // namespace clang
 
-namespace llvm {
-
-template <typename T> struct isPodLike;
-template <> struct isPodLike<clang::ento::SVal> {
-  static const bool value = true;
-};
-
-} // namespace llvm
-
 #endif // LLVM_CLANG_STATICANALYZER_CORE_PATHSENSITIVE_SVALS_H

cfe/trunk/lib/AST/VTableBuilder.cpp

MethodInfo(CharUnits BaseOffset, CharUnits BaseOffsetInLayoutClass,
       : BaseOffset(BaseOffset),
       BaseOffsetInLayoutClass(BaseOffsetInLayoutClass),
       VTableIndex(VTableIndex) { }
 
     MethodInfo()
       : BaseOffset(CharUnits::Zero()),
       BaseOffsetInLayoutClass(CharUnits::Zero()),
       VTableIndex(0) { }
+
+    MethodInfo(MethodInfo const&) = default;
   };
 
   typedef llvm::DenseMap<const CXXMethodDecl *, MethodInfo> MethodInfoMapTy;
 
   /// MethodInfoMap - The information for all methods in the vtable we're
   /// currently building.
   MethodInfoMapTy MethodInfoMap;
 

cfe/trunk/lib/Sema/SemaChecking.cpp

class SequenceTree {
     SmallVector<Value, 8> Values;
 
   public:
     /// A region within an expression which may be sequenced with respect
     /// to some other region.
     class Seq {
       friend class SequenceTree;
 
-      unsigned Index = 0;
+      unsigned Index;
 
       explicit Seq(unsigned N) : Index(N) {}
 
     public:
-      Seq() = default;
+      Seq() : Index(0) {}
     };
 
     SequenceTree() { Values.push_back(Value(0)); }
     Seq root() const { return Seq(0); }
 
     /// Create a new sequence of operations, which is an unsequenced
     /// subset of \p Parent. This sequence of operations is sequenced with
     /// respect to other children of \p Parent.

cfe/trunk/lib/StaticAnalyzer/Core/RegionStore.cpp

raw_ostream &operator<<(raw_ostream &os, BindingKey K) {
     os << '(' << K.getRegion();
     if (!K.hasSymbolicOffset())
       os << ',' << K.getOffset();
     os << ',' << (K.isDirect() ? "direct" : "default")
        << ')';
     return os;
   }
 
-  template <typename T> struct isPodLike;
-  template <> struct isPodLike<BindingKey> {
-    static const bool value = true;
-  };
 } // end llvm namespace
 
 #ifndef NDEBUG
 LLVM_DUMP_METHOD void BindingKey::dump() const { llvm::errs() << *this; }
 #endif
 
 //===----------------------------------------------------------------------===//
 // Actual Store type.

cfe/trunk/test/Analysis/llvm-conventions.cpp

 inline bool operator==(StringRef LHS, StringRef RHS);
 inline bool operator!=(StringRef LHS, StringRef RHS);
 inline bool operator<(StringRef LHS, StringRef RHS);
 inline bool operator<=(StringRef LHS, StringRef RHS);
 inline bool operator>(StringRef LHS, StringRef RHS);
 inline bool operator>=(StringRef LHS, StringRef RHS);
 inline std::string &operator+=(std::string &buffer, StringRef string);
 hash_code hash_value(StringRef S);
-template <typename T> struct isPodLike;
-template <> struct isPodLike<StringRef> { static const bool value = true; };
 
 } // end of namespace llvm
 
 //===----------------------------------------------------------------------===//
 // Tests for StringRef.
 //===----------------------------------------------------------------------===//
 
 void temporarayStringToStringRefAssignmentTest() {

cfe/trunk/tools/libclang/Indexing.cpp

public:
   }
 };
 
 typedef llvm::DenseSet<PPRegion> PPRegionSetTy;
 
 } // end anonymous namespace
 
 namespace llvm {
-  template <> struct isPodLike<PPRegion> {
-    static const bool value = true;
-  };
 
   template <>
   struct DenseMapInfo<PPRegion> {
     static inline PPRegion getEmptyKey() {
       return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-1), 0);
     }
     static inline PPRegion getTombstoneKey() {
       return PPRegion(llvm::sys::fs::UniqueID(0, 0), unsigned(-2), 0);

llvm/trunk/docs/ProgrammersManual.rst

 ``alloca``.
 
 SmallVector has grown a few other minor advantages over std::vector, causing
 ``SmallVector<Type, 0>`` to be preferred over ``std::vector<Type>``.
 
 #. std::vector is exception-safe, and some implementations have pessimizations
    that copy elements when SmallVector would move them.
 
-#. SmallVector understands ``isPodLike<Type>`` and uses realloc aggressively.
+#. SmallVector understands ``llvm::is_trivially_copyable<Type>`` and uses realloc aggressively.
 
 #. Many LLVM APIs take a SmallVectorImpl as an out parameter (see the note
    below).
 
 #. SmallVector with N equal to 0 is smaller than std::vector on 64-bit
    platforms, since it uses ``unsigned`` (instead of ``void*``) for its size
    and capacity.
 

llvm/trunk/include/llvm/ADT/ArrayRef.h

namespace llvm {
 
   template<typename T>
   inline bool operator!=(ArrayRef<T> LHS, ArrayRef<T> RHS) {
     return !(LHS == RHS);
   }
 
   /// @}
 
-  // ArrayRefs can be treated like a POD type.
-  template <typename T> struct isPodLike;
-  template <typename T> struct isPodLike<ArrayRef<T>> {
-    static const bool value = true;
-  };
-
   template <typename T> hash_code hash_value(ArrayRef<T> S) {
     return hash_combine_range(S.begin(), S.end());
   }
 
 } // end namespace llvm
 
 #endif // LLVM_ADT_ARRAYREF_H

llvm/trunk/include/llvm/ADT/DenseMap.h

void clear() {
     // If the capacity of the array is huge, and the # elements used is small,
     // shrink the array.
     if (getNumEntries() * 4 < getNumBuckets() && getNumBuckets() > 64) {
       shrink_and_clear();
       return;
     }
 
     const KeyT EmptyKey = getEmptyKey(), TombstoneKey = getTombstoneKey();
-    if (isPodLike<KeyT>::value && isPodLike<ValueT>::value) {
+    if (is_trivially_copyable<KeyT>::value &&
+        is_trivially_copyable<ValueT>::value) {
       // Use a simpler loop when these are trivial types.
       for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P)
         P->getFirst() = EmptyKey;
     } else {
       unsigned NumEntries = getNumEntries();
       for (BucketT *P = getBuckets(), *E = getBucketsEnd(); P != E; ++P) {
         if (!KeyInfoT::isEqual(P->getFirst(), EmptyKey)) {
           if (!KeyInfoT::isEqual(P->getFirst(), TombstoneKey)) {
protected:
   void copyFrom(
       const DenseMapBase<OtherBaseT, KeyT, ValueT, KeyInfoT, BucketT> &other) {
     assert(&other != this);
     assert(getNumBuckets() == other.getNumBuckets());
 
     setNumEntries(other.getNumEntries());
     setNumTombstones(other.getNumTombstones());
 
-    if (isPodLike<KeyT>::value && isPodLike<ValueT>::value)
+    if (is_trivially_copyable<KeyT>::value &&
+        is_trivially_copyable<ValueT>::value)
       memcpy(reinterpret_cast<void *>(getBuckets()), other.getBuckets(),
              getNumBuckets() * sizeof(BucketT));
     else
       for (size_t i = 0; i < getNumBuckets(); ++i) {
         ::new (&getBuckets()[i].getFirst())
             KeyT(other.getBuckets()[i].getFirst());
         if (!KeyInfoT::isEqual(getBuckets()[i].getFirst(), getEmptyKey()) &&
             !KeyInfoT::isEqual(getBuckets()[i].getFirst(), getTombstoneKey()))

llvm/trunk/include/llvm/ADT/ImmutableList.h

return (unsigned((uintptr_t)PtrVal) >> 4) ^
            (unsigned((uintptr_t)PtrVal) >> 9);
   }
 
   static bool isEqual(ImmutableList<T> X1, ImmutableList<T> X2) {
     return X1 == X2;
   }
 };
 
-template <typename T> struct isPodLike;
-template <typename T>
-struct isPodLike<ImmutableList<T>> { static const bool value = true; };
-
 } // end namespace llvm
 
 #endif // LLVM_ADT_IMMUTABLELIST_H

llvm/trunk/include/llvm/ADT/Optional.h

 #include <utility>
 
 namespace llvm {
 
 class raw_ostream;
 
 namespace optional_detail {
 /// Storage for any type.
-template <typename T, bool = isPodLike<T>::value> struct OptionalStorage {
+template <typename T, bool = is_trivially_copyable<T>::value> struct OptionalStorage {
   AlignedCharArrayUnion<T> storage;
   bool hasVal = false;
 
   OptionalStorage() = default;
 
   OptionalStorage(const T &y) : hasVal(true) { new (storage.buffer) T(y); }
   OptionalStorage(const OptionalStorage &O) : hasVal(O.hasVal) {
     if (hasVal)
#if LLVM_HAS_RVALUE_REFERENCE_THIS
 
   template <typename U>
   T getValueOr(U &&value) && {
     return hasValue() ? std::move(getValue()) : std::forward<U>(value);
   }
 #endif
 };
 
-template <typename T> struct isPodLike<Optional<T>> {
-  // An Optional<T> is pod-like if T is.
-  static const bool value = isPodLike<T>::value;
-};
-
 template <typename T, typename U>
 bool operator==(const Optional<T> &X, const Optional<U> &Y) {
   if (X && Y)
     return *X == *Y;
   return X.hasValue() == Y.hasValue();
 }
 
 template <typename T, typename U>

llvm/trunk/include/llvm/ADT/PointerIntPair.h

static intptr_t updateInt(intptr_t OrigValue, intptr_t Int) {
     intptr_t IntWord = static_cast<intptr_t>(Int);
     assert((IntWord & ~IntMask) == 0 && "Integer too large for field");
 
     // Preserve all bits other than the ones we are updating.
     return (OrigValue & ~ShiftedIntMask) | IntWord << IntShift;
   }
 };
 
-template <typename T> struct isPodLike;
-template <typename PointerTy, unsigned IntBits, typename IntType>
-struct isPodLike<PointerIntPair<PointerTy, IntBits, IntType>> {
-  static const bool value = true;
-};
-
 // Provide specialization of DenseMapInfo for PointerIntPair.
 template <typename PointerTy, unsigned IntBits, typename IntType>
 struct DenseMapInfo<PointerIntPair<PointerTy, IntBits, IntType>> {
   using Ty = PointerIntPair<PointerTy, IntBits, IntType>;
 
   static Ty getEmptyKey() {
     uintptr_t Val = static_cast<uintptr_t>(-1);
     Val <<= PointerLikeTypeTraits<Ty>::NumLowBitsAvailable;

llvm/trunk/include/llvm/ADT/SmallVector.h

reference back() {
     return end()[-1];
   }
   const_reference back() const {
     assert(!empty());
     return end()[-1];
   }
 };
 
-/// SmallVectorTemplateBase<isPodLike = false> - This is where we put method
+/// SmallVectorTemplateBase<TriviallyCopyable = false> - This is where we put method
 /// implementations that are designed to work with non-POD-like T's.
-template <typename T, bool = isPodLike<T>::value>
+template <typename T, bool = is_trivially_copyable<T>::value>
 class SmallVectorTemplateBase : public SmallVectorTemplateCommon<T> {
 protected:
   SmallVectorTemplateBase(size_t Size) : SmallVectorTemplateCommon<T>(Size) {}
 
   static void destroy_range(T *S, T *E) {
     while (S != E) {
       --E;
       E->~T();
public:
 
   void pop_back() {
     this->set_size(this->size() - 1);
     this->end()->~T();
   }
 };
 
 // Define this out-of-line to dissuade the C++ compiler from inlining it.
-template <typename T, bool isPodLike>
-void SmallVectorTemplateBase<T, isPodLike>::grow(size_t MinSize) {
+template <typename T, bool TriviallyCopyable>
+void SmallVectorTemplateBase<T, TriviallyCopyable>::grow(size_t MinSize) {
   if (MinSize > UINT32_MAX)
     report_bad_alloc_error("SmallVector capacity overflow during allocation");
 
   // Always grow, even from zero.
   size_t NewCapacity = size_t(NextPowerOf2(this->capacity() + 2));
   NewCapacity = std::min(std::max(NewCapacity, MinSize), size_t(UINT32_MAX));
   T *NewElts = static_cast<T*>(llvm::safe_malloc(NewCapacity*sizeof(T)));
 
   // Move the elements over.
   this->uninitialized_move(this->begin(), this->end(), NewElts);
 
   // Destroy the original elements.
   destroy_range(this->begin(), this->end());
 
   // If this wasn't grown from the inline copy, deallocate the old space.
   if (!this->isSmall())
     free(this->begin());
 
   this->BeginX = NewElts;
   this->Capacity = NewCapacity;
 }
 
-
-/// SmallVectorTemplateBase<isPodLike = true> - This is where we put method
-/// implementations that are designed to work with POD-like T's.
+/// SmallVectorTemplateBase<TriviallyCopyable = true> - This is where we put
+/// method implementations that are designed to work with POD-like T's.
 template <typename T>
 class SmallVectorTemplateBase<T, true> : public SmallVectorTemplateCommon<T> {
 protected:
   SmallVectorTemplateBase(size_t Size) : SmallVectorTemplateCommon<T>(Size) {}
 
   // No need to do a destroy loop for POD's.
   static void destroy_range(T *, T *) {}
 

 public:
   using iterator = typename SuperClass::iterator;
   using const_iterator = typename SuperClass::const_iterator;
   using size_type = typename SuperClass::size_type;
 
 protected:
   // Default ctor - Initialize to empty.
   explicit SmallVectorImpl(unsigned N)
-      : SmallVectorTemplateBase<T, isPodLike<T>::value>(N) {}
+      : SmallVectorTemplateBase<T>(N) {}
 
 public:
   SmallVectorImpl(const SmallVectorImpl &) = delete;
 
   ~SmallVectorImpl() {
     // Subclass has already destructed this vector's elements.
     // If this wasn't grown from the inline copy, deallocate the old space.
     if (!this->isSmall())

llvm/trunk/include/llvm/ADT/StringRef.h

#endif
   }
 
   /// @}
 
   /// Compute a hash_code for a StringRef.
   LLVM_NODISCARD
   hash_code hash_value(StringRef S);
 
-  // StringRefs can be treated like a POD type.
-  template <typename T> struct isPodLike;
-  template <> struct isPodLike<StringRef> { static const bool value = true; };
-
 } // end namespace llvm
 
 #endif // LLVM_ADT_STRINGREF_H

llvm/trunk/include/llvm/ADT/bit.h

 #if (__has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION)) || \
     (defined(__GNUC__) && __GNUC__ >= 5)
           , typename = typename std::enable_if<std::is_trivially_copyable<To>::value>::type
           , typename = typename std::enable_if<std::is_trivially_copyable<From>::value>::type
 #elif __has_feature(is_trivially_copyable)
           , typename = typename std::enable_if<__is_trivially_copyable(To)>::type
           , typename = typename std::enable_if<__is_trivially_copyable(From)>::type
 #else
-  // This case is GCC 4.x. clang with libc++ or libstdc++ never get here. Unlike
-  // llvm/Support/type_traits.h's isPodLike we don't want to provide a
-  // good-enough answer here: developers in that configuration will hit
-  // compilation failures on the bots instead of locally. That's acceptable
-  // because it's very few developers, and only until we move past C++11.
+// llvm/Support/type_traits.h's is_trivially_copyable we don't want to
+// provide a good-enough answer here: developers in that configuration will hit
 #endif
 >
 inline To bit_cast(const From &from) noexcept {
   To to;
   std::memcpy(&to, &from, sizeof(To));
   return to;
 }
 
 } // namespace llvm
 
 #endif

llvm/trunk/include/llvm/Analysis/BlockFrequencyInfoImpl.h

 }
 
 inline raw_ostream &operator<<(raw_ostream &OS, BlockMass X) {
   return X.print(OS);
 }
 
 } // end namespace bfi_detail
 
-template <> struct isPodLike<bfi_detail::BlockMass> {
-  static const bool value = true;
-};
-
 /// Base class for BlockFrequencyInfoImpl
 ///
 /// BlockFrequencyInfoImplBase has supporting data structures and some
 /// algorithms for BlockFrequencyInfoImplBase.  Only algorithms that depend on
 /// the block type (or that call such algorithms) are skipped here.
 ///
 /// Nevertheless, the majority of the overall algorithm documention lives with
 /// BlockFrequencyInfoImpl.  See there for details.
 class BlockFrequencyInfoImplBase {
 public:
   using Scaled64 = ScaledNumber<uint64_t>;
   using BlockMass = bfi_detail::BlockMass;
 
   /// Representative of a block.
   ///
   /// This is a simple wrapper around an index into the reverse-post-order
   /// traversal of the blocks.
   ///
   /// Unlike a block pointer, its order has meaning (location in the
   /// topological sort) and it's class is the same regardless of block type.
   struct BlockNode {
     using IndexType = uint32_t;
 
-    IndexType Index = std::numeric_limits<uint32_t>::max();
+    IndexType Index;
 
-    BlockNode() = default;
+    BlockNode() : Index(std::numeric_limits<uint32_t>::max()) {}
     BlockNode(IndexType Index) : Index(Index) {}
 
     bool operator==(const BlockNode &X) const { return Index == X.Index; }
     bool operator!=(const BlockNode &X) const { return Index != X.Index; }
     bool operator<=(const BlockNode &X) const { return Index <= X.Index; }
     bool operator>=(const BlockNode &X) const { return Index >= X.Index; }
     bool operator<(const BlockNode &X) const { return Index < X.Index; }
     bool operator>(const BlockNode &X) const { return Index > X.Index; }

llvm/trunk/include/llvm/Bitcode/BitCodes.h

public:
   static char DecodeChar6(unsigned V) {
     assert((V & ~63) == 0 && "Not a Char6 encoded character!");
     return "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789._"
         [V];
   }
 
 };
 
-template <> struct isPodLike<BitCodeAbbrevOp> { static const bool value=true; };
-
 /// BitCodeAbbrev - This class represents an abbreviation record.  An
 /// abbreviation allows a complex record that has redundancy to be stored in a
 /// specialized format instead of the fully-general, fully-vbr, format.
 class BitCodeAbbrev {
   SmallVector<BitCodeAbbrevOp, 32> OperandList;
 
 public:
   unsigned getNumOperandInfos() const {

llvm/trunk/include/llvm/CodeGen/DIE.h

class DIEUnit {
   /// The section this unit will be emitted in. This may or may not be set to
   /// a valid section depending on the client that is emitting DWARF.
   MCSection *Section;
   uint64_t Offset; /// .debug_info or .debug_types absolute section offset.
   uint32_t Length; /// The length in bytes of all of the DIEs in this unit.
   const uint16_t Version; /// The Dwarf version number for this unit.
   const uint8_t AddrSize; /// The size in bytes of an address for this unit.
 protected:
-  ~DIEUnit() = default;
+  virtual ~DIEUnit() = default;
 
 public:
   DIEUnit(uint16_t Version, uint8_t AddrSize, dwarf::Tag UnitTag);
   DIEUnit(const DIEUnit &RHS) = delete;
   DIEUnit(DIEUnit &&RHS) = delete;
   void operator=(const DIEUnit &RHS) = delete;
   void operator=(const DIEUnit &&RHS) = delete;
   /// Set the section that this DIEUnit will be emitted into.

llvm/trunk/include/llvm/CodeGen/RegisterPressure.h

public:
 
   void setUnitInc(int Inc) { UnitInc = Inc; }
 
   bool operator==(const PressureChange &RHS) const {
     return PSetID == RHS.PSetID && UnitInc == RHS.UnitInc;
   }
 };
 
-template <> struct isPodLike<PressureChange> {
-   static const bool value = true;
-};
-
 /// List of PressureChanges in order of increasing, unique PSetID.
 ///
 /// Use a small fixed number, because we can fit more PressureChanges in an
 /// empty SmallVector than ever need to be tracked per register class. If more
 /// PSets are affected, then we only track the most constrained.
 class PressureDiff {
   // The initial design was for MaxPSets=4, but that requires PSet partitions,
   // which are not yet implemented. (PSet partitions are equivalent PSets given

llvm/trunk/include/llvm/CodeGen/ScheduleDAG.h

void setReg(unsigned Reg) {
       assert((getKind() != Output || Reg != 0) &&
              "SDep::Output edge cannot use the zero register!");
       Contents.Reg = Reg;
     }
 
     void dump(const TargetRegisterInfo *TRI = nullptr) const;
   };
 
-  template <>
-  struct isPodLike<SDep> { static const bool value = true; };
-
   /// Scheduling unit. This is a node in the scheduling DAG.
   class SUnit {
   private:
     enum : unsigned { BoundaryID = ~0u };
 
     SDNode *Node = nullptr;        ///< Representative node.
     MachineInstr *Instr = nullptr; ///< Alternatively, a MachineInstr.
 

llvm/trunk/include/llvm/CodeGen/SelectionDAGNodes.h

static unsigned getHashValue(const SDValue &Val) {
     return ((unsigned)((uintptr_t)Val.getNode() >> 4) ^
             (unsigned)((uintptr_t)Val.getNode() >> 9)) + Val.getResNo();
   }
 
   static bool isEqual(const SDValue &LHS, const SDValue &RHS) {
     return LHS == RHS;
   }
 };
-template <> struct isPodLike<SDValue> { static const bool value = true; };
 
 /// Allow casting operators to work directly on
 /// SDValues as if they were SDNode*'s.
 template<> struct simplify_type<SDValue> {
   using SimpleType = SDNode *;
 
   static SimpleType getSimplifiedValue(SDValue &Val) {
     return Val.getNode();

llvm/trunk/include/llvm/CodeGen/SlotIndexes.h

public:
 
     /// Returns the previous index. This is the index corresponding to this
     /// index's slot, but for the previous instruction.
     SlotIndex getPrevIndex() const {
       return SlotIndex(&*--listEntry()->getIterator(), getSlot());
     }
   };
 
-  template <> struct isPodLike<SlotIndex> { static const bool value = true; };
-
   inline raw_ostream& operator<<(raw_ostream &os, SlotIndex li) {
     li.print(os);
     return os;
   }
 
   using IdxMBBPair = std::pair<SlotIndex, MachineBasicBlock *>;
 
   inline bool operator<(SlotIndex V, const IdxMBBPair &IM) {

llvm/trunk/include/llvm/CodeGen/TargetPassConfig.h

public:
   }
 
   Pass *getInstance() const {
     assert(IsInstance && "Not a Pass Instance");
     return P;
   }
 };
 
-template <> struct isPodLike<IdentifyingPassPtr> {
-  static const bool value = true;
-};
 
 /// Target-Independent Code Generator Pass Configuration Options.
 ///
 /// This is an ImmutablePass solely for the purpose of exposing CodeGen options
 /// to the internals of other CodeGen passes.
 class TargetPassConfig : public ImmutablePass {
 private:
   PassManagerBase *PM = nullptr;

llvm/trunk/include/llvm/IR/CFG.h

public:
 
   /// Get the source BlockT of this iterator.
   inline BlockT *getSource() {
     assert(Inst && "Source not available, if basic block was malformed");
     return Inst->getParent();
   }
 };
 
-template <typename T, typename U> struct isPodLike<SuccIterator<T, U>> {
-  static const bool value = isPodLike<T>::value;
-};
-
 using succ_iterator = SuccIterator<Instruction, BasicBlock>;
 using succ_const_iterator = SuccIterator<const Instruction, const BasicBlock>;
 using succ_range = iterator_range<succ_iterator>;
 using succ_const_range = iterator_range<succ_const_iterator>;
 
 inline succ_iterator succ_begin(Instruction *I) { return succ_iterator(I); }
 inline succ_const_iterator succ_begin(const Instruction *I) {
   return succ_const_iterator(I);

llvm/trunk/include/llvm/IR/ValueHandle.h

struct DenseMapInfo<AssertingVH<T>> {
   }
 
   static bool isEqual(const AssertingVH<T> &LHS, const AssertingVH<T> &RHS) {
     return DenseMapInfo<Value *>::isEqual(LHS.getRawValPtr(),
                                           RHS.getRawValPtr());
   }
 };
 
-template <typename T>
-struct isPodLike<AssertingVH<T>> {
-#ifdef NDEBUG
-  static const bool value = true;
-#else
-  static const bool value = false;
-#endif
-};
-
 /// Value handle that tracks a Value across RAUW.
 ///
 /// TrackingVH is designed for situations where a client needs to hold a handle
 /// to a Value (or subclass) across some operations which may move that value,
 /// but should never destroy it or replace it with some unacceptable type.
 ///
 /// It is an error to attempt to replace a value with one of a type which is
 /// incompatible with any of its outstanding TrackingVHs.
template <typename T> struct DenseMapInfo<PoisoningVH<T>> {
   }
 
   static bool isEqual(const PoisoningVH<T> &LHS, const PoisoningVH<T> &RHS) {
     return DenseMapInfo<Value *>::isEqual(LHS.getRawValPtr(),
                                           RHS.getRawValPtr());
   }
 };
 
-template <typename T> struct isPodLike<PoisoningVH<T>> {
-#ifdef NDEBUG
-  static const bool value = true;
-#else
-  static const bool value = false;
-#endif
-};
-
 } // end namespace llvm
 
 #endif // LLVM_IR_VALUEHANDLE_H

llvm/trunk/include/llvm/MC/MCInst.h

public:
   }
 
   void print(raw_ostream &OS) const;
   void dump() const;
   bool isBareSymbolRef() const;
   bool evaluateAsConstantImm(int64_t &Imm) const;
 };
 
-template <> struct isPodLike<MCOperand> { static const bool value = true; };
-
 /// Instances of this class represent a single low-level machine
 /// instruction.
 class MCInst {
   unsigned Opcode = 0;
   SMLoc Loc;
   SmallVector<MCOperand, 8> Operands;
   // These flags could be used to pass some info from one target subcomponent
   // to another, for example, from disassembler to asm printer. The values of

llvm/trunk/include/llvm/Support/ScaledNumber.h

if (Shift >= Width) {
     // Saturate.
     *this = getZero();
     return;
   }
 
   Digits >>= Shift;
 }
 
-template <typename T> struct isPodLike;
-template <typename T> struct isPodLike<ScaledNumber<T>> {
-  static const bool value = true;
-};
 
 } // end namespace llvm
 
 #endif // LLVM_SUPPORT_SCALEDNUMBER_H

llvm/trunk/include/llvm/Support/type_traits.h

 
 #ifndef __has_feature
 #define LLVM_DEFINED_HAS_FEATURE
 #define __has_feature(x) 0
 #endif
 
 namespace llvm {
 
-/// isPodLike - This is a type trait that is used to determine whether a given
-/// type can be copied around with memcpy instead of running ctors etc.
-template <typename T>
-struct isPodLike {
-  // std::is_trivially_copyable is available in libc++ with clang, libstdc++
-  // that comes with GCC 5.  MSVC 2015 and newer also have
-  // std::is_trivially_copyable.
-#if (__has_feature(is_trivially_copyable) && defined(_LIBCPP_VERSION)) ||      \
-    (defined(__GNUC__) && __GNUC__ >= 5) || defined(_MSC_VER)
-  // If the compiler supports the is_trivially_copyable trait use it, as it
-  // matches the definition of isPodLike closely.
-  static const bool value = std::is_trivially_copyable<T>::value;
-#elif __has_feature(is_trivially_copyable)
-  // Use the internal name if the compiler supports is_trivially_copyable but we
-  // don't know if the standard library does. This is the case for clang in
-  // conjunction with libstdc++ from GCC 4.x.
-  static const bool value = __is_trivially_copyable(T);
-#else
-  // If we don't know anything else, we can (at least) assume that all non-class
-  // types are PODs.
-  static const bool value = !std::is_class<T>::value;
-#endif
-};
-
-// std::pair's are pod-like if their elements are.
-template<typename T, typename U>
-struct isPodLike<std::pair<T, U>> {
-  static const bool value = isPodLike<T>::value && isPodLike<U>::value;
-};
 
 /// Metafunction that determines whether the given type is either an
 /// integral type or an enumeration type, including enum classes.
 ///
 /// Note that this accepts potentially more integral types than is_integral
 /// because it is based on being implicitly convertible to an integral type.
 /// Also note that enum classes aren't implicitly convertible to integral types,
 /// the value may therefore need to be explicitly converted before being used.

 };
 /// Internal utility to detect trivial move construction.
 template<typename T> union move_construction_triviality_helper {
     T t;
     move_construction_triviality_helper() = default;
     move_construction_triviality_helper(move_construction_triviality_helper&&) = default;
     ~move_construction_triviality_helper() = default;
 };
+
+template<class T>
+union trivial_helper {
+    T t;
+};
 } // end namespace detail
 
 /// An implementation of `std::is_trivially_copy_constructible` since we have
 /// users with STLs that don't yet include it.
 template <typename T>
 struct is_trivially_copy_constructible
     : std::is_copy_constructible<
           ::llvm::detail::copy_construction_triviality_helper<T>> {};
 template <typename T>
 struct is_trivially_copy_constructible<T &> : std::true_type {};
 template <typename T>
 struct is_trivially_copy_constructible<T &&> : std::false_type {};
 
 /// An implementation of `std::is_trivially_move_constructible` since we have
 /// users with STLs that don't yet include it.
 template <typename T>
 struct is_trivially_move_constructible
     : std::is_move_constructible<
           ::llvm::detail::move_construction_triviality_helper<T>> {};
 template <typename T>
 struct is_trivially_move_constructible<T &> : std::true_type {};
 template <typename T>
 struct is_trivially_move_constructible<T &&> : std::true_type {};
 
+// An implementation of `std::is_trivially_copyable` since STL version
+// is not equally supported by all compilers, especially GCC 4.9.
+// Uniform implementation of this trait is important for ABI compatibility
+// as it has an impact on SmallVector's ABI (among others).
+template <typename T>
+class is_trivially_copyable {
+
+  // copy constructors
+  static constexpr bool has_trivial_copy_constructor =
+      std::is_copy_constructible<detail::trivial_helper<T>>::value;
+  static constexpr bool has_deleted_copy_constructor =
+      !std::is_copy_constructible<T>::value;
+
+  // move constructors
+  static constexpr bool has_trivial_move_constructor =
+      std::is_move_constructible<detail::trivial_helper<T>>::value;
+  static constexpr bool has_deleted_move_constructor =
+      !std::is_move_constructible<T>::value;
+
+  // copy assign
+  static constexpr bool has_trivial_copy_assign =
+      std::is_copy_assignable<detail::trivial_helper<T>>::value;
+  static constexpr bool has_deleted_copy_assign =
+      !std::is_copy_assignable<T>::value;
+
+  // move assign
+  static constexpr bool has_trivial_move_assign =
+      std::is_move_assignable<detail::trivial_helper<T>>::value;
+  static constexpr bool has_deleted_move_assign =
+      !std::is_move_assignable<T>::value;
+
+  // destructor
+  static constexpr bool has_trivial_destructor =
+      std::is_destructible<detail::trivial_helper<T>>::value;
+
+  public:
+
+  static constexpr bool value =
+      has_trivial_destructor &&
+      (has_deleted_move_assign || has_trivial_move_assign) &&
+      (has_deleted_move_constructor || has_trivial_move_constructor) &&
+      (has_deleted_copy_assign || has_trivial_copy_assign) &&
+      (has_deleted_copy_constructor || has_trivial_copy_constructor);
+
+#if (__has_feature(is_trivially_copyable) || (defined(__GNUC__) && __GNUC__ >= 5))
+  static_assert(value == std::is_trivially_copyable<T>::value, "inconsistent behavior between llvm:: and std:: implementation of is_trivially_copyable");
+#endif
+};
+
+
 } // end namespace llvm
 
 // If the compiler supports detecting whether a class is final, define
 // an LLVM_IS_FINAL macro. If it cannot be defined properly, this
 // macro will be left undefined.
 #if __cplusplus >= 201402L || defined(_MSC_VER)
 #define LLVM_IS_FINAL(Ty) std::is_final<Ty>()
 #elif __has_feature(is_final) || LLVM_GNUC_PREREQ(4, 7, 0)
 #define LLVM_IS_FINAL(Ty) __is_final(Ty)
 #endif
 
 #ifdef LLVM_DEFINED_HAS_FEATURE
 #undef __has_feature
 #endif
 
 #endif // LLVM_SUPPORT_TYPE_TRAITS_H

llvm/trunk/lib/Transforms/Scalar/SROA.cpp

bool operator==(const Slice &RHS) const {
     return isSplittable() == RHS.isSplittable() &&
            beginOffset() == RHS.beginOffset() && endOffset() == RHS.endOffset();
   }
   bool operator!=(const Slice &RHS) const { return !operator==(RHS); }
 };
 
 } // end anonymous namespace
 
-namespace llvm {
-
-template <typename T> struct isPodLike;
-template <> struct isPodLike<Slice> { static const bool value = true; };
-
-} // end namespace llvm
-
 /// Representation of the alloca slices.
 ///
 /// This class represents the slices of an alloca which are formed by its
 /// various uses. If a pointer escapes, we can't fully build a representation
 /// for the slices used and we reflect that in this structure. The uses are
 /// stored, sorted by increasing beginning offset and with unsplittable slices
 /// starting at a particular offset before splittable slices.
 class llvm::sroa::AllocaSlices {

llvm/trunk/tools/llvm-diff/DifferenceEngine.cpp

public:
   /// Remove the minimum value in the heap.  Only valid on a non-empty heap.
   T remove_min() {
     assert(!empty());
     T tmp = Storage[0];
     
     unsigned NewSize = Storage.size() - 1;
     if (NewSize) {
       // Move the slot at the end to the beginning.
-      if (isPodLike<T>::value)
+      if (is_trivially_copyable<T>::value)
         Storage[0] = Storage[NewSize];
       else
         std::swap(Storage[0], Storage[NewSize]);
 
       // Bubble the root up as necessary.
       unsigned Index = 0;
       while (true) {
         // With a 1-based index, the children would be Index*2 and Index*2+1.

llvm/trunk/unittests/ADT/ArrayRefTest.cpp

TEST(ArrayRefTest, makeArrayRef) {
 
   // A copy is expected for non-const ArrayRef (thin copy)
   const ArrayRef<int> AR2(A1);
   const ArrayRef<int> &AR2Ref = makeArrayRef(AR2);
   EXPECT_NE(&AR2Ref, &AR2);
   EXPECT_TRUE(AR2.equals(AR2Ref));
 }
 
+static_assert(is_trivially_copyable<ArrayRef<int>>::value,
+              "trivially copyable");
+
 } // end anonymous namespace

llvm/trunk/unittests/ADT/ImmutableListTest.cpp

TEST_F(ImmutableListTest, LongListOrderingTest) {
   for (ImmutableList<Wrapper<long>>::iterator I = L3.begin(), E = L3.end();
        I != E; ++I) {
     ASSERT_EQ(i, *I);
     i++;
   }
   ASSERT_EQ(6, i);
 }
 
+static_assert(is_trivially_copyable<ImmutableList<Wrapper<long>>>::value,
+              "trivially copyable");
+
 } // namespace

llvm/trunk/unittests/ADT/PointerIntPairTest.cpp

TEST(PointerIntPairTest, GetSet) {
 
   Pair2.setPointer(nullptr);
   EXPECT_EQ(nullptr, Pair2.getPointer());
   EXPECT_EQ(E::Case2, Pair2.getInt());
 
   Pair2.setPointerAndInt(&s, E::Case3);
   EXPECT_EQ(&s, Pair2.getPointer());
   EXPECT_EQ(E::Case3, Pair2.getInt());
+
+  static_assert(is_trivially_copyable<PointerIntPair<S *, 2, E>>::value,
+                "trivially copyable");
 }
 
 TEST(PointerIntPairTest, DefaultInitialize) {
   PointerIntPair<float *, 2> Pair;
   EXPECT_EQ(nullptr, Pair.getPointer());
   EXPECT_EQ(0U, Pair.getInt());
 }
 
TEST(PointerIntPairTest, ManyUnusedBits) {
   EXPECT_FALSE(pair.getInt());
 
   pair.setPointerAndInt({ 0x7FFFFFFF }, true );
   EXPECT_EQ((uintptr_t)0x7FFFFFFF, pair.getPointer().Value);
   EXPECT_TRUE(pair.getInt());
 
   EXPECT_EQ(FixnumPointerTraits::NumLowBitsAvailable - 1,
             PointerLikeTypeTraits<decltype(pair)>::NumLowBitsAvailable);
+
+  static_assert(
+      is_trivially_copyable<
+          PointerIntPair<Fixnum31, 1, bool, FixnumPointerTraits>>::value,
+      "trivially copyable");
 }
 
 } // end anonymous namespace

llvm/trunk/unittests/ADT/StringRefTest.cpp

 }
 
 TEST(StringRefTest, StringLiteral) {
   constexpr StringLiteral Strings[] = {"Foo", "Bar"};
   EXPECT_EQ(StringRef("Foo"), Strings[0]);
   EXPECT_EQ(StringRef("Bar"), Strings[1]);
 }
 
+static_assert(is_trivially_copyable<StringRef>::value, "trivially copyable");
+
 } // end anonymous namespace

llvm/trunk/unittests/Analysis/BlockFrequencyInfoTest.cpp

 //===- BlockFrequencyInfoTest.cpp - BlockFrequencyInfo unit tests ---------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "llvm/Analysis/BlockFrequencyInfo.h"
+#include "llvm/Analysis/BlockFrequencyInfoImpl.h"
 #include "llvm/Analysis/BranchProbabilityInfo.h"
 #include "llvm/Analysis/LoopInfo.h"
 #include "llvm/AsmParser/Parser.h"
 #include "llvm/IR/BasicBlock.h"
 #include "llvm/IR/Dominators.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/Module.h"
TEST_F(BlockFrequencyInfoTest, Basic) {
   SmallPtrSet<BasicBlock *, 4> BlocksToScale({BB1, BB2});
   BFI.setBlockFreqAndScale(&BB0, BB0Freq * 2, BlocksToScale);
   EXPECT_EQ(BFI.getBlockFreq(&BB0).getFrequency(), 2 * BB0Freq);
   EXPECT_EQ(BFI.getBlockFreq(BB1).getFrequency(), 2 * BB1Freq);
   EXPECT_EQ(BFI.getBlockFreq(BB2).getFrequency(), 2 * BB2Freq);
   EXPECT_EQ(BFI.getBlockFreq(BB3).getFrequency(), BB3Freq);
 }
 
+static_assert(is_trivially_copyable<bfi_detail::BlockMass>::value,
+              "trivially copyable");
+
 } // end anonymous namespace
 } // end namespace llvm

llvm/trunk/unittests/Bitcode/BitstreamReaderTest.cpp

 TEST(BitstreamReaderTest, shortRead) {
   uint8_t Bytes[] = {8, 7, 6, 5, 4, 3, 2, 1};
   for (unsigned I = 1; I != 8; ++I) {
     SimpleBitstreamCursor Cursor(ArrayRef<uint8_t>(Bytes, I));
     EXPECT_EQ(8ull, Cursor.Read(8));
   }
 }
 
+static_assert(is_trivially_copyable<BitCodeAbbrevOp>::value,
+              "trivially copyable");
+
 } // end anonymous namespace

llvm/trunk/unittests/CodeGen/CMakeLists.txt

 add_llvm_unittest(CodeGenTests
   AArch64SelectionDAGTest.cpp
   DIEHashTest.cpp
   LowLevelTypeTest.cpp
   MachineInstrBundleIteratorTest.cpp
   MachineInstrTest.cpp
   MachineOperandTest.cpp
   ScalableVectorMVTsTest.cpp
+  TypeTraitsTest.cpp
   )
 
 add_subdirectory(GlobalISel)

llvm/trunk/unittests/CodeGen/MachineInstrTest.cpp

TEST(MachineInstrPrintingTest, DebugLocPrinting) {
   raw_string_ostream OS(str);
   MI->print(OS);
   ASSERT_TRUE(
       StringRef(OS.str()).startswith("$noreg = UNKNOWN debug-location "));
   ASSERT_TRUE(
       StringRef(OS.str()).endswith("filename:1:5"));
 }
 
+static_assert(is_trivially_copyable<MCOperand>::value, "trivially copyable");
+
 } // end namespace

llvm/trunk/unittests/IR/CFGBuilder.cpp

 //===- llvm/Testing/Support/CFGBuilder.cpp --------------------------------===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
 // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
 //
 //===----------------------------------------------------------------------===//
 
 #include "CFGBuilder.h"
 
+#include "llvm/IR/CFG.h"
 #include "llvm/IR/IRBuilder.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include "gtest/gtest.h"
 
 #define DEBUG_TYPE "cfg-builder"
 
while (B.applyUpdate())
     ++i;
   EXPECT_EQ(i, NumUpdates);
 
   EXPECT_TRUE(isa<SwitchInst>(B.getOrAddBlock("entry")->getTerminator()));
   EXPECT_TRUE(isa<SwitchInst>(B.getOrAddBlock("a")->getTerminator()));
   EXPECT_TRUE(isa<SwitchInst>(B.getOrAddBlock("c")->getTerminator()));
   EXPECT_TRUE(isa<SwitchInst>(B.getOrAddBlock("d")->getTerminator()));
 }
+
+static_assert(is_trivially_copyable<succ_iterator>::value,
+              "trivially copyable");
+static_assert(is_trivially_copyable<succ_const_iterator>::value,
+              "trivially copyable");
+static_assert(is_trivially_copyable<succ_range>::value, "trivially copyable");
+static_assert(is_trivially_copyable<succ_const_range>::value,
+              "trivially copyable");

llvm/trunk/unittests/Support/ScaledNumberTest.cpp

TEST(ScaledNumberHelpersTest, arithmeticOperators) {
   EXPECT_EQ(ScaledNumber<uint64_t>(1, 4), ScaledNumber<uint64_t>(1, 3) << 1);
 }
 
 TEST(ScaledNumberHelpersTest, toIntBug) {
   ScaledNumber<uint32_t> n(1, 0);
   EXPECT_EQ(1u, (n * n).toInt<uint32_t>());
 }
 
+static_assert(is_trivially_copyable<ScaledNumber<uint32_t>>::value,
+              "trivially copyable");
+
 } // end namespace