[PATCH] D86671: [clang-tidy] Add new case type to check variables with Hungarian notation

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

Hi @aaron.ballman,

Thank you for the suggestion.



================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:237
+static void getHungarianNotationDefaultConfig(
+    std::shared_ptr<IdentifierNamingCheck::HungarianNotationOption> HNOption) {
+
----------------
aaron.ballman wrote:
> It seems like this function should take `HNOption` as a reference rather than a `shared_ptr`.
OK!


================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h:87
+    HungarianPrefixOption HungarianPrefixOpt;
+    std::shared_ptr<IdentifierNamingCheck::HungarianNotationOption>
+        HungarianNotationOption;
----------------
aaron.ballman wrote:
> I'd like to avoid using a `shared_ptr` here if we can avoid it -- do we expect this to be super expensive to copy by value (I would imagine it'll be an expensive copy but we don't make copies all that often, but maybe my intuition is wrong)?
Good idea. A reference object is good enough. I will change it at next commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671