[PATCH] D76937: Fix infinite recursion in deferred diagnostic emitter

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D76937#1950764 <https://reviews.llvm.org/D76937#1950764>, @yaxunl wrote:

> In D76937#1950077 <https://reviews.llvm.org/D76937#1950077>, @rjmccall wrote:
>
> > Can you explain what exactly the emission/semantic model is for variables?  Normal code-generation absolutely triggers the emission of many variables lazily (e.g. internal-linkage globals, C++ inline variables); and any variable that's *not* being emitted lazily actually needs to be treated as a potential root into the delayed-diagnostic graph.
>
>
> Currently only in the following case a global variable can change the emission state of a function:
>
>   int foobar3() { return 1; }
>   #pragma omp declare target
>   int (*C)() = &foobar3;
>   #pragma omp end declare target
>
>
> The global variable needs to be in a omp declare target directive. Its initializer references a host function.
>
> This will cause the function emitted in device compilation.
>
> This is not transitive for variable declaration/references, e.g. the following case will not cause foobar3 to be emitted in device compilation, unless variable C itself is enclosed in omp declare target directive.
>
>   int foobar3() { return 1; }
>   int (*C)() = &foobar3;
>   #pragma omp declare target
>   int (*D)() = C;
>   #pragma omp end declare target
>
>
> Since we logged all such variable declarations in DeclsToCheckForDeferredDiags, we only need to check variables decls in DeclsToCheckForDeferredDiags and do not need to check variable decls in DeclRefExpr.


Okay.  There are other AST nodes besides `DeclRefExpr` that can cause an ODR-use of a `VarDecl`; you could add special cases for them, too, but part of the point of this design is so that you can stop worrying about that kind of thing.  I think I've said this before, but you're making things harder for yourself by re-using `visitUsedDecl` as both the "callback" of the `UsedDeclVisitor` and the original method that you call for things from `DeclsToCheckForDeferredDiags`.  Just use `visitUsedDecl` for the former, make a new method for the latter, and extract any common logic you want (e.g. for `FunctionDecl`s) into helper methods that both of them can use.  That way you can just unconditionally ignore non-functions in `visitUsedDecl`, assert that `DeclsToCheckForDeferredDiags` only contains the kinds of declarations you think it should, and so on.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D76937/new/

https://reviews.llvm.org/D76937