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:
> > 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?
> For the second comment, we need to visit a function again for each use-path
> because we need to report each use-path that triggers a diagnostic, otherwise
> users will see a new error after they fix one error, instead of seeing all
> the errors at once.
>
> For the first comment, I will change the count to two flags: one for the case
> where the function is not in device context, the other is for the case where
> the function is in device context. This will allow us to avoid redundant
> visits whether or not we are in a device context.
> For the second comment, we need to visit a function again for each use-path
> because we need to report each use-path that triggers a diagnostic, otherwise
> users will see a new error after they fix one error, instead of seeing all
> the errors at once.
This is not what we do in analogous cases where errors are triggered by a use,
like in template instantiation. The bug might be that the device program is
using a function that it shouldn't be using, or the bug might be that a
function that's supposed to be usable from the device is written incorrectly.
In the former case, yes, not reporting the errors for each use-path may force
the programmer to build multiple times to find all the problematic uses.
However, in the latter case you can easily end up emitting a massive number of
errors that completely drowns out everything else. It's also non-linear: the
number of different use-paths of a particular function can be combinatoric.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D77028/new/
https://reviews.llvm.org/D77028
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
