[PATCH] D138554: [clang][Interp] Use placement new to construct opcode arguments into bytecode vector

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added inline comments.


================
Comment at: clang/lib/AST/Interp/ByteCodeEmitter.cpp:166
   if constexpr (!std::is_pointer_v<T>) {
-    const char *Data = reinterpret_cast<const char *>(&Val);
-    Code.insert(Code.end(), Data, Data + Size);
+    if constexpr (std::is_trivially_copyable_v<T>) {
+      const char *Data = reinterpret_cast<const char *>(&Val);
----------------
sepavloff wrote:
> tbaeder wrote:
> > sepavloff wrote:
> > > Do we really need a separate case for trivially copyable types?
> > It's not necessary, placement new'ing all everything works. I was just trying to keep the old behavior as much as possible.
> Another thing: when copy constructor for APFloat works, it can allocate data in heap. So the serialization is not perfect in this case. Would it be suitable for the interpretator?
That heap allocation is exactly why this change exists. I know it's not serialized here, but that's not a problem (right now at leat), since we don't write the byte code to disk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D138554