[PATCH] D66094: [CodeGen] Emit destructor calls for non-trivial C structs returned by function calls and loaded from volatile objects

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/CodeGen/CGCall.h:372
+      REQUIRES_DESTRUCTION = 0x4,
     };
 
----------------
Does `llvm::Value*` actually guarantee three bits?  Presumably on 64-bit platforms, but we do support 32-bit hosts.

Fortunately, I don't actually think there's any reason to be space-conscious in this class; all the values are short-lived.  Might as well pull all the fields out into a bit-field or something.


================
Comment at: clang/lib/CodeGen/CGCall.h:390
+      return Value.getInt() & REQUIRES_DESTRUCTION;
+    }
   };
----------------
I think the "is externally destructed" semantics make more sense — that is, the *default* should be that a destructor needs to get pushed, and maybe specific contexts should suppress that.  That will also let you avoid all the ugly code to compute whether destruction is required in a bunch of generic places.

Actually, "externally destructed" is a really problematic idea; we need to come up with a better solution for contexts that need to be careful about destructor ordering like this, and it should probably be based on passing around and deactivating destructors.  But that's for later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D66094