rsmith accepted this revision.
This revision is now accepted and ready to land.
================
Comment at: include/clang/AST/VTableBuilder.h:222-225
@@ -221,6 +221,6 @@
typedef const VTableComponent *vtable_component_iterator;
typedef const VTableThunkTy *vtable_thunk_iterator;
typedef llvm::iterator_range<vtable_component_iterator>
vtable_component_range;
----------------
Can you remove these? It looks like they should be unused now.
================
Comment at: lib/CodeGen/CGVTables.cpp:527-528
@@ +526,4 @@
+ auto OffsetConstant = [&](CharUnits Offset) {
+ llvm::Type *PtrDiffTy =
+ CGM.getTypes().ConvertType(CGM.getContext().getPointerDiffType());
+ return llvm::ConstantExpr::getIntToPtr(
----------------
This is just `CGM.PtrDiffTy`.
================
Comment at: lib/CodeGen/CGVTables.cpp:588
@@ +587,3 @@
+ if (auto *F = dyn_cast<llvm::Function>(Cache))
+ F->setUnnamedAddr(llvm::GlobalValue::UnnamedAddr::Global);
+ Cache = llvm::ConstantExpr::getBitCast(Cache, CGM.Int8PtrTy);
----------------
Do you have any idea why we're doing this? It looks wrong to me. These ABI
entry points are exposed and could certainly have their addresses taken and
used in this translation unit.
================
Comment at: lib/CodeGen/CGVTables.cpp:598-615
@@ -638,5 +597,20 @@
+ CGM.getCXXABI().GetPureVirtualCallName());
+ } else if (cast<CXXMethodDecl>(GD.getDecl())->isDeleted()) {
+ return SpecialVirtualFn(DeletedVirtualFn,
+ CGM.getCXXABI().GetDeletedVirtualCallName());
+ } else {
+ // Check if we should use a thunk.
+ if (NextVTableThunkIndex < VTLayout.vtable_thunks().size() &&
+ VTLayout.vtable_thunks()[NextVTableThunkIndex].first == Idx) {
+ const ThunkInfo &Thunk =
+ VTLayout.vtable_thunks()[NextVTableThunkIndex].second;
+
+ maybeEmitThunkForVTable(GD, Thunk);
+ NextVTableThunkIndex++;
+ return CGM.GetAddrOfThunk(GD, Thunk);
+ } else {
+ llvm::Type *Ty = CGM.getTypes().GetFunctionTypeForVTable(GD);
- Init = llvm::ConstantExpr::getBitCast(Init, Int8PtrTy);
+ return CGM.GetAddrOfFunction(GD, Ty, /*ForVTable=*/true);
}
}
----------------
Please remove the `else`-after-`return` throughout here. That'll make it much
more obvious to a reader of the code that fallthrough to the next `case` is not
possible.
================
Comment at: lib/CodeGen/CGVTables.h:52
@@ -51,1 +51,3 @@
+ llvm::Constant *PureVirtualFn = nullptr, *DeletedVirtualFn = nullptr;
+
----------------
I'd prefer these to be two separate declarations, with doc comments.
https://reviews.llvm.org/D22642
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
