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
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
