[PATCH] D96536: Make sure some types are indeed trivially_copyable per llvm::is_trivially_copyable

Danila Malyutin via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Feb 24 09:34:00 PST 2021


danilaml added a comment.

In D96536#2585263 <https://reviews.llvm.org/D96536#2585263>, @serge-sans-paille wrote:

>> It's better, I just still not completely get why that assert was necessary in the first place. I.e. what exact scenario is it trying to prevent? If we just want all `trivially_copyable` uses to be consistent across compilers, then that assert won't help (at the very least since std:: is not consistent across compiler/std versions).
>
> Yeah, that was the original attempt: make sure that `llvm::is_trivially_copyable` was consistent with `std::is_trivially_copyable` when compiler implement it (at that time, some compiler were not supporting it all). As you know, experience proved that this was a naive approach :-/

That's a bit different and seems like a non-issue. We can't guarantee consistency across compilers without compiling with several compilers. And assert by itself gives us nothing, like I've explained. We should only care that llvm:: => std:: at most.
I don't think that checks added in this patch solve this in any way.

Also not sure why the attempt to replace llvm:: with std:: has failed (probably some weird mix of obsolete STL with a newer patched GCC), although if it's the ABI we want, it doesn't matter.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D96536/new/

https://reviews.llvm.org/D96536



More information about the llvm-commits mailing list