[PATCH] D139087: [include-cleaner] Handle base class member access from derived class.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:68
+    RecordDecl *RD = getDeclFromType(Type);
+    report(E->getMemberLoc(), RD);
     return true;
----------------
nit: just `report(E->getMemberLoc(), getDeclFromTye(Type));`.


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:73
+  bool VisitCXXDependentScopeMemberExpr(const CXXDependentScopeMemberExpr *E) {
+    // FIXME: implement this
+    return true;
----------------
Be more specific about the comment, something like `// FIMXE: support the dependent type case like "std::vector<T>().size();"`.

I will move this FIXME to `VisitMemberExpr`, and remove this function. 


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:77
+
+  RecordDecl *getDeclFromType(QualType Type) {
+    if (Type->isPointerType()) {
----------------
- I'd use `NamedDecl*` as a return type (which is used in `report`)
- move this method to private, it is only used internally inside class, no need to make it public
-  what do you think about naming it `resolveType`?


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:78
+  RecordDecl *getDeclFromType(QualType Type) {
+    if (Type->isPointerType()) {
+      Type = Type->getPointeeType();
----------------
nit: remove the surrounding {}


================
Comment at: clang-tools-extra/include-cleaner/lib/WalkAST.cpp:81
+    }
+    RecordDecl *RecordDecl = Type->getAsRecordDecl();
+    return RecordDecl;
----------------
nit: just `return Type->getAsRecordDecl();`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139087