rjmccall added inline comments.
================
Comment at: clang/lib/Sema/Sema.cpp:1508
void checkFunc(SourceLocation Loc, FunctionDecl *FD) {
+ auto DiagsCountIt = DiagsCount.find(FD);
FunctionDecl *Caller = UseStack.empty() ? nullptr : UseStack.back();
----------------
yaxunl wrote:
> rjmccall wrote:
> > It makes me a little uncomfortable to be holding an iterator this long
> > while calling a fair amount of other stuff in the meantime.
> >
> > Your use of DiagsCount is subtle enough that it really needs to be
> > explained in some comments. You're doing stuff conditionally based on both
> > whether the entry exists but also whether it's non-zero.
> added comments
Okay, thanks for that. Two points, then. First, it looks like the count is
really just a boolean for whether the function recursively triggers any
diagnostics. And second, can't it just be as simple as whether we've visited
that function at all in a context that's forcing diagnostics to be emitted?
The logic seems to be to try to emit the diagnostics for each use-path, but why
would we want that?
================
Comment at: clang/lib/Sema/Sema.cpp:1553
if (!Caller)
ShouldEmit = IsKnownEmitted;
if ((!ShouldEmit && !S.getLangOpts().OpenMP && !Caller) ||
----------------
This becomes global state for the visitor; that doesn't seem like it can be
right.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77028/new/
https://reviews.llvm.org/D77028
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
