================
@@ -19109,7 +19119,17 @@ bool Sema::DefineUsedVTables() {
}
}
- if (IsExplicitInstantiationDeclaration)
+ if (IsExplicitInstantiationDeclaration &&
+ llvm::none_of(Class->decls(), [](Decl *decl) {
+ // If the class has a virtual member function declared with
+ // `__attribute__((exclude_from_explicit_instantiation))`, the
+ // explicit instantiation declaration shouldn't suppress emitting
+ // the vtable to ensure that the excluded member function is
+ // accessible through the vtable.
+ auto *Method = dyn_cast<CXXMethodDecl>(decl);
+ return Method && Method->isVirtual() &&
+ Method->hasAttr<ExcludeFromExplicitInstantiationAttr>();
+ }))
----------------
kikairoya wrote:
> As a style issue, I think it would be easier to read if we put this in a
> variable
Sure.
> Also, I'm not sure that it matters whether the method with the attribute is
> virtual or not. I think it would be enough to check that _any_ method has the
> attribute -- because then the motivation about "suppress the vtable; it will
> live with the explicit instantiation definition" may not hold.
I don't understand why non-virtual functions matter here. If no virtual methods
are excluded, then no special handling is needed to enforce emitting the
definition. The vtable itself is emitted by another procedure anyway (I still
can't find the procedure, though).
> I wonder if we could break this out into a separate PR as well, since it
> seems somewhat separate from the other issue.
No, this should be a part of the PR because this is the cause of reverting
#65961.
https://github.com/llvm/llvm-project/pull/168171
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
