rjmccall added inline comments.
================ Comment at: include/clang/CodeGen/SwiftCallingConv.h:190 + llvm::FunctionType *type, + llvm::IRBuilderBase *builder); + ---------------- This isn't the right header for this. More thoughts on this point below. The documentation should describe the expected relationship between `this` and the method. I imagine that `thisPtr` is expected to already be a pointer to the type which declares the method (i.e. `MD->getParent()`), and in the appropriate address space. Please name this something like `emitLoadOfCXXVirtualMethod`. This particular case will probably never need control flow or temporary memory, but I wonder if we should go ahead and introduce an abstraction that bundles the CGM and the builder so that we can teach it to handle those in the future. If you do, you could just define it in its own header and then make this a method on that. I think it would be totally reasonable to pull Clang's `Address.h` up to `include/clang/CodeGen` instead of breaking it down here. Also, this really needs to return a `CGCallee`, which might require pulling that up as well. The reason why should be clear in a few weeks. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D66360/new/ https://reviews.llvm.org/D66360 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
