[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

[Yaxun Liu via Phabricator via cfe-commits]

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