rjmccall 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());
----------------
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.
================
Comment at: include/clang/AST/VTableBuilder.h:169
+ return GlobalDecl(DtorDecl, CXXDtorType::Dtor_Deleting);
+ default:
+ llvm_unreachable("Only function pointers kinds");
----------------
By "exhaustive" I mean that you should list out all the cases instead of using
default. It means that someone who adds a new kind of v-table entry will get
alerted to fix this switch. There's only five other cases, it's not too bad.
https://reviews.llvm.org/D33437
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
