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

[Douglas Chen via Phabricator via llvm-commits]

dougpuob marked 82 inline comments as done.
dougpuob added a comment.

Hi @aaron.ballman, thank you for your feedback. I will update my change later. Unrelated change were mixed with other commits when I against git master. I did it again then the problem was gone. I found the command, `arc diff master --preview`, knowing exactly changes before uploading diff by arc.



================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:254
+  static constexpr std::pair<StringRef, StringRef> CStrings[] = {
+      {"char*", "sz"}, {"char", "sz"}, {"wchar_t*", "wsz"}, {"wchar_t", "wsz"}};
+  for (const auto &CStr : CStrings) {
----------------
aaron.ballman wrote:
> One thing that confused me was the plain `char` and `wchar_t` entries -- those are for arrays of `char` and `wchar_t`, aren't they? Can we use `char[]` to make that more clear? If not, can you add a comment to clarify?
I improved it. It will look like the following:
```
static constexpr std::pair<StringRef, StringRef> CStrings[] = {
      {"char*", "sz"}, {"char[]", "sz"}, {"wchar_t*", "wsz"}, {"wchar_t[]", "wsz"}};
```


================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:376-380
+  static constexpr std::pair<StringRef, StringRef> HNCStrings[] = {
+      {"CharPrinter", "char*"},
+      {"CharArray", "char"},
+      {"WideCharPrinter", "wchar_t*"},
+      {"WideCharArray", "wchar_t"}};
----------------
aaron.ballman wrote:
> Similar question here as above, but less pressing because we at least have the word "array" nearby.
Improved here too. It will change to:

```
  static constexpr std::pair<StringRef, StringRef> HNCStrings[] = {
      {"CharPrinter", "char*"},
      {"CharArray", "char[]"},
      {"WideCharPrinter", "wchar_t*"},
      {"WideCharArray", "wchar_t[]"}};
```


================
Comment at: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:431
 static IdentifierNamingCheck::FileStyle
-getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options) {
+getFileStyleFromOptions(const ClangTidyCheck::OptionsView &Options,
+                IdentifierNamingCheck::HungarianNotationOption &HNOption) {
----------------
aaron.ballman wrote:
> Formatting
OK. I checked all the range including single-line if statements, and passing StringRef directly(not its reference).




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671