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

Thu May 7 05:38:02 PDT 2020

njames93 marked an inline comment as done.
njames93 added inline comments.

================
Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:44

+  virtual void storeCheckOptions(ClangTidyOptions::OptionMap &Opts) {}
+
----------------
aaron.ballman wrote:
> I'd appreciate some comments here explaining when this should be overridden. I'd also like to understand why we need `storeOptions` and `storeCheckOptions` because the two names are so similar to one another.
I'm still unsure what the best approach here is. `RenamerClangTidyCheck::storeOptions` needs to be called to store the `AggressiveDependentMemberLookup` Option (as well as any other renaming specific option that may be added down the line). However derived classes need to also store their respective options at the same time. Due to how c++ works there isn't a nice way to ensure base methods are called at the same time as derived methods.
Anyway that's the reason the names are similar they do a very similar job only one is meant for the `RenamerClangTidyCheck` class and the other for derived classes.
I agree a comment is needed to explain why so I'll add that in for now

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D73052