[PATCH] D11859: Generating vptr assume loads

[Piotr Padlewski via cfe-commits]

Prazek added inline comments.

================
Comment at: lib/CodeGen/CGClass.cpp:1842
@@ +1841,3 @@
+      CGM.getCXXABI().getVTableAddressPoint(vptr.Base, vptr.VTableClass);
+  if (!VTableGlobal) return;
+
----------------
majnemer wrote:
> The return should be on it's own line, please clang-format this.
> Also, under which conditions is this case possible?
Unfortunatelly clang-format puts short instructions in the same line

================
Comment at: lib/CodeGen/CGClass.cpp:2098
@@ -2063,3 +2097,3 @@
     // Initialize the vtable pointer for this base.
-    InitializeVTablePointer(Base, NearestVBase, OffsetFromNearestVBase,
-                            VTableClass);
+    VPtr vptr{Base, NearestVBase, OffsetFromNearestVBase, VTableClass};
+    vptrs.push_back(vptr);
----------------
majnemer wrote:
> This kind of initialization is not very common in LLVM.
is It worth adding constructor just to change { with ( ? 

================
Comment at: lib/CodeGen/MicrosoftCXXABI.cpp:223-228
@@ +222,8 @@
+  // with the 'novtable' attribute.
+  bool canInitilizeVPtr(const CXXRecordDecl *VTableClass,
+                        const CXXRecordDecl *Base,
+                        const CXXRecordDecl *NearestVBase) override {
+    return !VTableClass->hasAttr<MSNoVTableAttr>() ||
+           (Base != VTableClass && Base != NearestVBase);
+  }
+
----------------
majnemer wrote:
> In the MS ABI, derived classes never share vtables with bases.  Why do you need to do anything other than check that the most derived class doesn't have the `__declspec(novtable)` ?
I don't know, I just took previous code assuming it is correct.


http://reviews.llvm.org/D11859