[PATCH] D73052: [clang-tidy] RenamerClangTidy now renames dependent member expr when the member can be resolved

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp:282
+                        : DepMemberRef->getBaseType();
+    CXXRecordDecl *Base = BaseType.getTypePtr()->getAsCXXRecordDecl();
+    if (!Base)
----------------
JonasToth wrote:
> can `Base` be a `const` pointer?
> 
> In which case will this pointer be a `nullptr`? Your test-cases seem to exercise only cases where `Base` is not-null.
Base can be a const pointer. However I don't think it can be a nullptr, but I put the check in there just in case it ever is, maybe an assert would be the correct solution.


================
Comment at: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-member-decl-usage.cpp:187
+};
+
+Derived<int> AInt{};
----------------
JonasToth wrote:
> Could you please provide a test-case with non-type template parameters as well?
> 
> Another question but maybe/most likely off topic: are how are member-pointers handled? Both for data and member-functions.
> -> Maybe a test-case for such a situation would be good, as well? :)
> 
> Crème de la Crème would be if templated member-pointers are treated properly, too.
The test for non-type isn't strictly needed. in these test cases the template type isn't used but they still show up as dependent scope exprs. 

MemberPointers is something that this doesn't handle and probably something I wont add as you can have issues with specialisation meaning the member may not exist in a specialization 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73052