[PATCH] D40218: [Clang] Add __builtin_launder

[Richard Smith - zygoloid via Phabricator via cfe-commits]

rsmith added inline comments.


================
Comment at: lib/CodeGen/CGBuiltin.cpp:1947-1948
+    const auto *Record = ArgTy->getAsCXXRecordDecl();
+    if (CGM.getCodeGenOpts().StrictVTablePointers && Record &&
+        Record->isDynamicClass())
+      Ptr = Builder.CreateInvariantGroupBarrier(Ptr);
----------------
I think you also need to catch class types that contain dynamic classes as subobjects.


================
Comment at: lib/Sema/SemaChecking.cpp:860
+
+  int DiagSelect = [&]() {
+    if (!ArgTy->isPointerType())
----------------
Might be a bit clearer to use `llvm::Optional<unsigned>` as the type of this.


================
Comment at: test/CodeGenCXX/builtin-launder.cpp:93-96
+/// The test cases in this namespace technically need to be laundered according
+/// to the language in the standard (ie they have const or reference subobjects)
+/// but LLVM doesn't currently optimize on these cases -- so Clang emits
+/// __builtin_launder as a nop.
----------------
I would note that this means adding optimizations for those cases later is an LTO ABI break. That's probably OK, but just something we're going to need to remember.


https://reviews.llvm.org/D40218