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

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to