Prazek marked 4 inline comments as done. Prazek added inline comments.
================ Comment at: include/clang/AST/VTableBuilder.h:160 + "GlobalDecl can be created only from virtual function"); + if (getKind() == CK_FunctionPointer) + return GlobalDecl(getFunctionDecl()); ---------------- rjmccall wrote: > Prazek wrote: > > rjmccall wrote: > > > Please use an exhaustive switch. You can put llvm_unreachable in the > > > other cases. > > Should I implement this for RTTI fields? Or maybe leave a fixme comment > > that it could also work for some other fields in vtable, but is not > > currently used? > I think your precondition of isUsedFunctionPointerKind() is fine, you don't > need to handle RTTI in this function. > > This does raise the interesting question, though, of whether this approach is > safe for lazily-emitted RTTI. I guess it currently works because IRGen > doesn't use the normal deferred-emission mechanism for RTTI objects, so if > the RTTI object is lazily-emitted, we'll just eagerly emit it instead of > potentially ending up with an illegal reference to external RTTI. You should > add a comment to the appropriate place in CGRTTI (i.e. where we fill in the > global definition) saying that this optimization may need adjustment if we > ever switch to using deferred emission for some reason. I couldn't find CGRTTI, so I added comment in Itanium and Microsoft ABI where we create RTTI fields, but maybe the CodeGenModule::GetAddrOfRTTIDescriptor would be a better place instead? I also added small note to the EmitVTablesOpportunistically about this. https://reviews.llvm.org/D33437 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
