yaxunl added a comment. In D70172#1812749 <https://reviews.llvm.org/D70172#1812749>, @rjmccall wrote:
> In D70172#1812664 <https://reviews.llvm.org/D70172#1812664>, @yaxunl wrote: > > > In D70172#1812631 <https://reviews.llvm.org/D70172#1812631>, @rjmccall > > wrote: > > > > > Most uses of the destructor do not use the delete operator, though, and > > > therefore should not trigger the diagnostics in `f` to be emitted. And > > > this really doesn't require a fully-realized use graph; you could very > > > easily track the current use stack when making a later pass over the > > > entities used. > > > > > > The call graph is not for this specific situation. A call graph is needed > > because of the transitive nature of the deferred diagnostic message. That > > is, if any direct or indirect caller is emitted, the diagnostic msg needs > > to be emitted. > > > One of the points that Richard and I have been trying to make is that this > really isn't specifically about *calls*, it's about *uses*. You only want to > emit diagnostics associated with an entity if you actually have to emit that > entity, and whether you emit an entity has nothing to do with what places > might *call* it, but rather what places *use* it and therefore force it to be > emitted. This is fortunate because call graphs are inherently imperfect > because of indirect calls, but use graphs are totally reliable. It's also > fortunate because it means you can piggy-back on all of the existing logic > that Sema has for tracking ODR uses. > > Richard and I are also pointing out that Sema has to treat the v-table as its > own separate thing when tracking ODR uses, and so you need to as well. You > want to emit diagnostics associated with a virtual function if you're > emitting code that either (1) directly uses the function (e.g. by calling > `x->A::foo()`) or (2) directly uses a v-table containing the function. You > can't rely on Sema's normal ODR-use tracking for *either* of these, because > Sema might have observed a use in code that you don't actually have to emit, > e.g. host code if you're compiling for the device. That is, a v-table is > only a "root" for virtual functions if you actually have to emit that > v-table, and you can't know that without tracking v-tables in your use graph. > > > The deferred diagnostic msg is recorded when parsing a function body. At > > that time we do not know which function will directly or indirectly call > > it. How do we keep a use stack? > > The "use stack" idea would apply if you switched from eagerly creating the > entire use graph to instead just making a late pass that walked function > bodies. If you walk function bodies depth-first, starting from a true root > and gathering all the ODR-used entities to be recursively walked, then you > can maintain a stack of what entities you're currently walking, and that > stack is a use-path that explains why you need to emit the current function. > > It should be straightforward to build a function that walks over the entities > used by a function body and calls a callback by just extracting it out of the > code in `MarkDeclarationsUsedInExpr`. I updated the patch to remove the explicit call graph and use an AST traverse instead. Since this patch is big, is it OK to leave the tracking of vtable to some future patch? This patch is sufficient to fix the assertion seen on Windows. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70172/new/ https://reviews.llvm.org/D70172 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
