rjmccall added inline comments.
================
Comment at: clang/include/clang/Sema/Sema.h:1470
+ /// diagnostics should be emitted.
+ SmallVector<Decl *, 4> DeclsToCheckForDeferredDiags;
+
----------------
This needs to be saved and restored in modules / PCH.
================
Comment at: clang/lib/Sema/Sema.cpp:1472
if (HasWarningOrError && ShowCallStack)
- emitCallStackNotes(S, FD);
+ emitCallStackNotes(*this, FD);
+}
----------------
Hmm. I know this is existing code, but I just realized something. I think
it's okay to not emit the notes on every diagnostic, but you might want to emit
them on the first diagnostic from a function instead of after the last. If the
real bug is that the program is using something it's not supposed to use, and
there are enough errors in that function to reach the error limit, then the
diagnostics emitter will associate these notes with a diagnostic it's
suppressing and so presumably suppress them as well, leaving the user with no
way to find this information.
================
Comment at: clang/lib/Sema/Sema.cpp:1494
+ visitUsedDecl(E->getLocation(), FD);
+ }
+ }
----------------
This needs to trigger if you use a variable with delayed diagnostics, too,
right?
When you add these methods to `UsedDeclVisitor`, you'll be able to remove them
here.
================
Comment at: clang/lib/Sema/Sema.cpp:1512
+ Inherited::VisitCapturedStmt(Node);
+ }
+
----------------
Should this also go in the base `UsedDeclVisitor`? I'm less sure about that
because the captured statement is really always a part of the enclosing
function, right? Should the delay mechanism just be looking through local
sub-contexts instead?
================
Comment at: clang/lib/Sema/UsedDeclVisitor.h:21
+template <class Derived>
+class UsedDeclVisitor : public EvaluatedExprVisitor<Derived> {
+protected:
----------------
Could you add this in a separate patch?
================
Comment at: clang/lib/Sema/UsedDeclVisitor.h:30
+
+ Derived &asImpl() { return *static_cast<Derived *>(this); }
+
----------------
There should definitely be cases in here for every expression that uses a
declaration, including both `DeclRefExpr` and `MemberExpr`. Those might be
overridden in subclasses, but that's their business; the default behavior
should be to visit every used decl.
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
