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

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:17127
+  template<class Derived>
+  class UsedDeclVisitor : public EvaluatedExprVisitor<UsedDeclVisitor<Derived>>{
+  protected:
----------------
This should inherit from `EvaluatedExprVisitor<Derived>`, or else calls from `EvaluatedExprVisitor` and above won't dispatch all the way down to the subclass.  This will allow subclasses to do node-specific logic, like your subclass's handling of `InOMPDeviceContext` or `EvaluatedExprMarker`'s need to do custom things with local variables, DREs, and MEs.

Please also define this in a header; it doesn't need to be file-specific.  I guess it needs a `Sema &` because of the call to `LookupDestructor`, so `lib/Sema` is probably the right place for that header.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:17152
+      asImpl().visitDeclRefExpr(E);
     }
 
----------------
Let's not have both a `visitDeclRefExpr` and a `VisitDeclRefExpr`, distinguished only by capitalization.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:17158
+      visitUsedDecl(E->getBeginLoc(), const_cast<CXXDestructorDecl *>(
+                                          E->getTemporary()->getDestructor()));
+      this->Visit(E->getSubExpr());
----------------
Please have all these call sites call `asImpl().visitUsedDecl` directly, and then don't define it in this class.


================
Comment at: clang/lib/Sema/SemaExpr.cpp:17195
+      --InOMPDeviceContext;
+    }
+
----------------
This should be in your OMP-specific subclass.


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

https://reviews.llvm.org/D70172