[PATCH] D148314: [clang-tidy][NFC] Improved hungarian notation regression test at post-commit review

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D148314#4268989 , @PiotrZSL wrote:

> I will push into repository this in a moment

@PiotrZSL, Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148314

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

[PATCH] D148314: [clang-tidy][NFC] Improved hungarian notation regression test at post-commit review

[Douglas Chen via Phabricator via cfe-commits]

dougpuob marked an inline comment as done.
dougpuob added a comment.

In D148314#4268460 , @PiotrZSL wrote:

> Change in tests is ok, change in release notes not needed (please remove).
> Change title into [clang-tidy][NFC] Improved ...
>
> NFC - Non Functional Change

Thank you for the review :)
I removed the release notes and changed the title.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148314

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

[PATCH] D148314: [clang-tidy] Improved hungarian notation regression test at post-commit review

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 513648.
dougpuob added a comment.

- Removed unnecessary information from ReleaseNotes.rst


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148314

Files:
  
clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -11,159 +11,159 @@
 public:
   static int ClassMemberCase;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for class member 'ClassMemberCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  static int custiClassMemberCase;
+  // CHECK-FIXES: {{^}}  static int myiClassMemberCase;
 
   char const ConstantMemberCase = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'ConstantMemberCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  char const custcConstantMemberCase = 0;
+  // CHECK-FIXES: {{^}}  char const mycConstantMemberCase = 0;
 
   void MyFunc1(const int ConstantParameterCase);
   // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for constant parameter 'ConstantParameterCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  void MyFunc1(const int custiConstantParameterCase);
+  // CHECK-FIXES: {{^}}  void MyFunc1(const int myiConstantParameterCase);
 
   void MyFunc2(const int* ConstantPointerParameterCase);
   // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: invalid case style for pointer parameter 'ConstantPointerParameterCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  void MyFunc2(const int* custpcustiConstantPointerParameterCase);
+  // CHECK-FIXES: {{^}}  void MyFunc2(const int* mypmyiConstantPointerParameterCase);
 
   static constexpr int ConstexprVariableCase = 123;
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: invalid case style for constexpr variable 'ConstexprVariableCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  static constexpr int custiConstexprVariableCase = 123;
+  // CHECK-FIXES: {{^}}  static constexpr int myiConstexprVariableCase = 123;
 };
 
 const int GlobalConstantCase = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global constant 'GlobalConstantCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}const int custiGlobalConstantCase = 0;
+// CHECK-FIXES: {{^}}const int myiGlobalConstantCase = 0;
 
 const int* GlobalConstantPointerCase = nullptr;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for global pointer 'GlobalConstantPointerCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}const int* custpcustiGlobalConstantPointerCase = nullptr;
+// CHECK-FIXES: {{^}}const int* mypmyiGlobalConstantPointerCase = nullptr;
 
 int* GlobalPointerCase = nullptr;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global pointer 'GlobalPointerCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}int* custpcustiGlobalPointerCase = nullptr;
+// CHECK-FIXES: {{^}}int* mypmyiGlobalPointerCase = nullptr;
 
 int GlobalVariableCase = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'GlobalVariableCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}int custiGlobalVariableCase = 0;
+// CHECK-FIXES: {{^}}int myiGlobalVariableCase = 0;
 
 void Func1(){
   int const LocalConstantCase = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for local constant 'LocalConstantCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  int const custiLocalConstantCase = 3;
+  // CHECK-FIXES: {{^}}  int const myiLocalConstantCase = 3;
 
   unsigned const ConstantCase = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'ConstantCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  unsigned const custuConstantCase = 1;
+  // CHECK-FIXES: {{^}}  unsigned const myuConstantCase = 1;
 
   int* const LocalConstantPointerCase = nullptr;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for local constant pointer 'LocalConstantPointerCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  int* const custpcustiLocalConstantPointerCase = nullptr;
+  // CHECK-FIXES: {{^}}  int* const mypmyiLocalConstantPointerCase = nullptr;
 
   int *LocalPointerCase = nullptr;
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local pointer 'LocalPointerCase' 

[PATCH] D148314: [clang-tidy] Improved hungarian notation regression test at post-commit review

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D148314#4267567 , @PiotrZSL wrote:

> And you forget to attach changes in unit tests.

My bad, I against to a wrong one.




Comment at: clang-tools-extra/docs/ReleaseNotes.rst:264-267
+- Improved readability for hungarian notation in
+  :doc:`readability-identifier-naming
+  ` by changing the prefix
+  from `cust` to `my` in regression test.

PiotrZSL wrote:
> Don't put this into release notes, in release notes we put only information 
> that impact users.
Make sense, I will remove it in next commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148314

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

[PATCH] D148314: [clang-tidy] Improved hungarian notation regression test at post-commit review

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 513586.
dougpuob added a comment.
Herald added a subscriber: aheejin.

Against trunk.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D148314

Files:
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/Inputs/identifier-naming/hungarian-notation2/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -11,159 +11,159 @@
 public:
   static int ClassMemberCase;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for class member 'ClassMemberCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  static int custiClassMemberCase;
+  // CHECK-FIXES: {{^}}  static int myiClassMemberCase;
 
   char const ConstantMemberCase = 0;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for constant member 'ConstantMemberCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  char const custcConstantMemberCase = 0;
+  // CHECK-FIXES: {{^}}  char const mycConstantMemberCase = 0;
 
   void MyFunc1(const int ConstantParameterCase);
   // CHECK-MESSAGES: :[[@LINE-1]]:26: warning: invalid case style for constant parameter 'ConstantParameterCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  void MyFunc1(const int custiConstantParameterCase);
+  // CHECK-FIXES: {{^}}  void MyFunc1(const int myiConstantParameterCase);
 
   void MyFunc2(const int* ConstantPointerParameterCase);
   // CHECK-MESSAGES: :[[@LINE-1]]:27: warning: invalid case style for pointer parameter 'ConstantPointerParameterCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  void MyFunc2(const int* custpcustiConstantPointerParameterCase);
+  // CHECK-FIXES: {{^}}  void MyFunc2(const int* mypmyiConstantPointerParameterCase);
 
   static constexpr int ConstexprVariableCase = 123;
   // CHECK-MESSAGES: :[[@LINE-1]]:24: warning: invalid case style for constexpr variable 'ConstexprVariableCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  static constexpr int custiConstexprVariableCase = 123;
+  // CHECK-FIXES: {{^}}  static constexpr int myiConstexprVariableCase = 123;
 };
 
 const int GlobalConstantCase = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global constant 'GlobalConstantCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}const int custiGlobalConstantCase = 0;
+// CHECK-FIXES: {{^}}const int myiGlobalConstantCase = 0;
 
 const int* GlobalConstantPointerCase = nullptr;
 // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for global pointer 'GlobalConstantPointerCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}const int* custpcustiGlobalConstantPointerCase = nullptr;
+// CHECK-FIXES: {{^}}const int* mypmyiGlobalConstantPointerCase = nullptr;
 
 int* GlobalPointerCase = nullptr;
 // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global pointer 'GlobalPointerCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}int* custpcustiGlobalPointerCase = nullptr;
+// CHECK-FIXES: {{^}}int* mypmyiGlobalPointerCase = nullptr;
 
 int GlobalVariableCase = 0;
 // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for global variable 'GlobalVariableCase' [readability-identifier-naming]
-// CHECK-FIXES: {{^}}int custiGlobalVariableCase = 0;
+// CHECK-FIXES: {{^}}int myiGlobalVariableCase = 0;
 
 void Func1(){
   int const LocalConstantCase = 3;
   // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for local constant 'LocalConstantCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  int const custiLocalConstantCase = 3;
+  // CHECK-FIXES: {{^}}  int const myiLocalConstantCase = 3;
 
   unsigned const ConstantCase = 1;
   // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: invalid case style for local constant 'ConstantCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  unsigned const custuConstantCase = 1;
+  // CHECK-FIXES: {{^}}  unsigned const myuConstantCase = 1;
 
   int* const LocalConstantPointerCase = nullptr;
   // CHECK-MESSAGES: :[[@LINE-1]]:14: warning: invalid case style for local constant pointer 'LocalConstantPointerCase' [readability-identifier-naming]
-  // CHECK-FIXES: {{^}}  int* const custpcustiLocalConstantPointerCase = nullptr;
+  // CHECK-FIXES: {{^}}  int* const mypmyiLocalConstantPointerCase = nullptr;
 
   int *LocalPointerCase = nullptr;
   // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for local pointer 

[PATCH] D148314: [clang-tidy] Improved hungarian notation regression test at post-commit review

[Douglas Chen via Phabricator via cfe-commits]

dougpuob created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
dougpuob requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Improve the (`D144510`)[https://reviews.llvm.org/D144510] patch with prefix 
string. Using "my" instead of "cust" would increase 
readability.

Take some examples:

- const char* `custszNamePtr` = "Name"; --> `myszNamePtr`
- uint8_t `custu8ValueU8` = 0; --> `myu8ValueU8`
- DWORD `custdwMsDword` = 0; --> `mydwMsDword`


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D148314

Files:
  clang-tools-extra/docs/ReleaseNotes.rst


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -261,6 +261,11 @@
   ` which failed to indicate
   the number of asterisks.
 
+- Improved readability for hungarian notation in
+  :doc:`readability-identifier-naming
+  ` by changing the prefix
+  from `cust` to `my` in regression test.
+
 - Fixed a false positive in :doc:`readability-implicit-bool-conversion
   ` check warning would
   be unnecessarily emitted for explicit cast using direct list initialization.


Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -261,6 +261,11 @@
   ` which failed to indicate
   the number of asterisks.
 
+- Improved readability for hungarian notation in
+  :doc:`readability-identifier-naming
+  ` by changing the prefix
+  from `cust` to `my` in regression test.
+
 - Fixed a false positive in :doc:`readability-implicit-bool-conversion
   ` check warning would
   be unnecessarily emitted for explicit cast using direct list initialization.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D147779#4257212 , @PiotrZSL wrote:

> arc land should work, I usually use arc patch --nobranch to check things 
> before committing and then git push.

Hi @PiotrZSL, thank you for the help :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

Hi @PiotrZSL
I do not have permission to land the change. Can you please help me to land 
this change?

Another related question is that can I know how you land diffs? Take this diff 
as an example. Is it correct with `arc land --onto main --revision D147779`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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

[PATCH] D144510: [clang-tidy] improve readability-identifier-naming hungarian options test

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D144510#4256258 , @amurzeau wrote:

> In D144510#4254959 , @dougpuob 
> wrote:
>
>> Hi @amurzeau
>>
>> I missed the review before landing. I have a suggestion regarding the 
>> customized prefix "cust" because it confused me when I found the Hungarian 
>> notation patterns in the output message. Perhaps using "my" instead of 
>> "cust" would improve readability.
>>
>> Take some examples:
>>
>> - const char *`custszNamePtr` = "Name"; --> `myszNamePtr`
>> - uint8_t `custu8ValueU8` = 0; --> `myu8ValueU8`
>> - DWORD `custdwMsDword` = 0; --> `mydwMsDword`
>
> Hi,
>
> Yes this is a good idea, m and y are less used as type prefixes.
>
> Do you want to make the change ? I can do it if you want.

Thank you. I will send a patch for the change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144510

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

[PATCH] D144510: [clang-tidy] improve readability-identifier-naming hungarian options test

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.
Herald added a subscriber: PiotrZSL.

Hi @amurzeau

I missed the review before landing. I have a suggestion regarding the 
customized prefix "cust" because it confused me when I found the Hungarian 
notation patterns in the output message. Perhaps using "my" instead of "cust" 
would improve readability.

Take some examples:

- const char *`custszNamePtr` = "Name"; --> `myszNamePtr`
- uint8_t `custu8ValueU8` = 0; --> `myu8ValueU8`
- DWORD `custdwMsDword` = 0; --> `mydwMsDword`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D144510

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

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 512021.
dougpuob added a comment.

- Improved suggestions in code review


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *custpcustu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *custpcustucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **custpcustpcustucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* custpcustvVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
@@ -283,6 +283,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -240,6 +240,10 @@
   behavior of using `i` as the prefix for enum tags, set the `EnumConstantPrefix`
   option to `i` instead of using `EnumConstantHungarianPrefix`.
 
+- Fixed a hungarian notation issue in :doc:`readability-identifier-naming
+  ` which failed to indicate
+  the number of asterisks.
+
 - Fixed a false positive in 

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 511969.
dougpuob marked 2 inline comments as done.
dougpuob added a comment.

- Extract method
- Remove template parameters in the getDeclTypeName() function


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *custpcustu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *custpcustucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **custpcustpcustucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* custpcustvVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
@@ -283,6 +283,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -291,6 +291,10 @@
   ` when warning would be
   emitted for a const local variable to which NRVO is applied.
 
+- Fix hungarian notation issue in :doc:`readability-identifier-naming
+  ` which failed to indicate
+  the number of asterisks.
+
 Removed checks
 

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

[Douglas Chen via Phabricator via cfe-commits]

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

In D147779#4251081 , @PiotrZSL wrote:

> Missing release notes.

Thank you for reminding me.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:367-379
+if (const auto *TD = dyn_cast(ND)) {
+  QualType QT = TD->getType();
+
+  size_t PtrCount = 0;
+  if (QT->isPointerType()) {
+PtrCount = getAsteriskCount(Type);
+  }

PiotrZSL wrote:
> this function is already big, extract this to have something like:
> ```
> if (removeReudnantAsterisks(Type, ND))
> {
> RedundantRemoved = true;
> break;
> }
> ```
Good idea, thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:696-702
+const std::string ) const {
+  size_t Pos = TypeName.find('*');
+  size_t Count = 0;
+  for (; Pos < TypeName.length(); Pos++, Count++) {
+if ('*' != TypeName[Pos])
+  break;
+  }

PiotrZSL wrote:
> what if type will be like std::optional, will this function 
> also be executed ?
Yes, this function is also executed in this case, but the 
`std::optional` is not in the supported list so the case will 
be ignored. Even though it is still weird, I will add some code to remove 
template parameters in the `getDeclTypeName()` function, making it clean.

- OLD: `std::optional`  --> `std::optional`  --> `std::optional` (removed template 
parameters)

Thank you for reminding me.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D147779

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

[PATCH] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

[Douglas Chen via Phabricator via cfe-commits]

dougpuob created this revision.
Herald added subscribers: PiotrZSL, carlosgalvezp, xazax.hun.
Herald added a reviewer: njames93.
Herald added a project: All.
dougpuob requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Fix hungarian notation failed to indicate the number of asterisks for the 
pointers of multiple word types.

- WRONG: `unsigned char* value`  : `value` --> `ucValue`
- RIGHT: `unsigned cahr* value`  : `value' --> `pucValue`
- RIGHT: `unsigned char** value` : `value' --> 'ppucValue'


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D147779

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-cfgfile.cpp
@@ -355,6 +355,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *custpcustu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *custpcustucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **custpcustpcustucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* custpcustvVal){}
Index: clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
===
--- clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
+++ clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming-hungarian-notation-c-language.c
@@ -283,6 +283,14 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for global pointer 'ValueU8Ptr' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}uint8_t *pu8ValueU8Ptr;
 
+unsigned char *ValueUcPtr;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'ValueUcPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char *pucValueUcPtr;
+
+unsigned char **ValueUcPtr2;
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: invalid case style for global pointer 'ValueUcPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}unsigned char **ppucValueUcPtr2;
+
 void MyFunc2(void* Val){}
 // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: invalid case style for pointer parameter 'Val' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}void MyFunc2(void* pVal){}
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ 

[PATCH] D107325: [clang-tidy] Fix command line is too long issue which breaks test on Windows

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D107325#2923518 , @thakis wrote:

> The bot is happy again, thanks.

Nice, thank you for your help :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107325

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

[PATCH] D107325: [clang-tidy] Fix command line is too long issue which breaks test on Windows

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

Hi @thakis:
I can't access the repo, could you please help me to land?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107325

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

[PATCH] D107325: [clang-tidy] Fix command line is too long issue which breaks test on Windows

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 363717.
dougpuob marked 3 inline comments as done.
dougpuob added a comment.

- Improved code review suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107325

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation2/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -1,62 +1,5 @@
-// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
-// RUN:   -config='{ CheckOptions: [ \
-// RUN: { key: readability-identifier-naming.AbstractClassCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassConstantCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassMemberCase , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantMemberCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantParameterCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantPointerParameterCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstexprVariableCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.EnumConstantCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalConstantPointerCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalPointerCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalVariableCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalConstantCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalConstantPointerCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalPointerCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalVariableCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.MemberCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ParameterCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PointerParameterCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PrivateMemberCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ProtectedMemberCase , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PublicMemberCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ScopedEnumConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.StaticConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.StaticVariableCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.VariableCase, value: CamelCase }, \
-// RUN: { key: 

[PATCH] D107325: [clang-tidy] Fix command line is too long issue which breaks test on Windows

[Douglas Chen via Phabricator via cfe-commits]

dougpuob marked 3 inline comments as done.
dougpuob added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy:1
+Checks: readability-identifier-naming
+CheckOptions:

whisperity wrote:
> I do not think this file should have the //execute// bit set...
I will change it to 664.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy:3
+CheckOptions:
+  - { key: readability-identifier-naming.AbstractClassCase 
  , value: CamelCase }
+  - { key: readability-identifier-naming.ClassCase 
  , value: CamelCase }

whisperity wrote:
> thakis wrote:
> > can we drop all the spaces here? :)
> And maybe even the object brackets.
Sure.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy:3
+CheckOptions:
+  - { key: readability-identifier-naming.AbstractClassCase 
  , value: CamelCase }
+  - { key: readability-identifier-naming.ClassCase 
  , value: CamelCase }

dougpuob wrote:
> whisperity wrote:
> > thakis wrote:
> > > can we drop all the spaces here? :)
> > And maybe even the object brackets.
> Sure.
OK


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107325

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

[PATCH] D107325: [clang-tidy] Fix command line is too long issue which breaks test on Windows

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

In D107325#2921844 , @thakis wrote:

> Thanks!
>
> Since the config file contents are fixed as far as I can tell, maybe we could 
> instead just add a file with the right contents to 
> `clang-tools-extra/test/clang-tidy/checkers/Inputs` and refer to that? (Use 
> `%S/Inputs/myfile.txt`) (Alternatively you could use the `split-file` 
> utility, but putting the file in Inputs is probably easier if you're new, and 
> it's not any worse :) )

Hi @thakis:
It's a really good idea, thank you for the suggestion. I just upload the latest 
diff.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107325

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

[PATCH] D107325: [clang-tidy] Fix command line is too long issue which breaks test on Windows

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 363698.
dougpuob added a comment.
Herald added a subscriber: aheejin.

- Moved config content of regression test to 
`clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation*/.clang-tidy`,
 then refer to those files in tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107325

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation1/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/Inputs/readability-identifier-naming/hungarian-notation2/.clang-tidy
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -1,62 +1,5 @@
-// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
-// RUN:   -config='{ CheckOptions: [ \
-// RUN: { key: readability-identifier-naming.AbstractClassCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassConstantCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassMemberCase , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantMemberCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantParameterCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantPointerParameterCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstexprVariableCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.EnumConstantCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalConstantPointerCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalPointerCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalVariableCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalConstantCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalConstantPointerCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalPointerCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalVariableCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.MemberCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ParameterCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PointerParameterCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PrivateMemberCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ProtectedMemberCase , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PublicMemberCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ScopedEnumConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.StaticConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.StaticVariableCase  , value: CamelCase }, \
-// RUN: { key: 

[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.

In D86671#2921424 , @thakis wrote:

> Any news here? This has been broken for over a day now.

Hi @thakis, 
YES. I just create a new differential diff for it, 
https://reviews.llvm.org/D107325, could you help me to review it?


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

[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.

In D86671#2920570 , @aeubanks wrote:

> you can reopen the revision via "Add Action..."

Hi @aeubanks , thank you for the suggestion.


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

[PATCH] D107325: [clang-tidy] Fix command line is too long issue which breaks test on Windows

[Douglas Chen via Phabricator via cfe-commits]

dougpuob created this revision.
Herald added a subscriber: xazax.hun.
dougpuob requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Command line is too long with check_clang_tidy.py program on Windows, because 
the configuration is too long for regression test. Fix this issue by creating a 
temporary response file then pass to target program. (error log: 
http://45.33.8.238/win/43180/step_8.txt)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D107325

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
@@ -1,141 +1,142 @@
-// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
-// RUN:   -config='{ CheckOptions: [ \
-// RUN: { key: readability-identifier-naming.AbstractClassCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassConstantCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ClassMemberCase , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantMemberCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantParameterCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstantPointerParameterCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ConstexprVariableCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.EnumConstantCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalConstantPointerCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalPointerCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.GlobalVariableCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalConstantCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalConstantPointerCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalPointerCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.LocalVariableCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.MemberCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ParameterCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PointerParameterCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PrivateMemberCase   , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ProtectedMemberCase , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.PublicMemberCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.ScopedEnumConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.StaticConstantCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.StaticVariableCase  , value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.VariableCase, value: CamelCase }, \
-// RUN: { key: readability-identifier-naming.AbstractClassHungarianPrefix, 

[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.

In D86671#2919077 , @thakis wrote:

> This breaks tests on windows: http://45.33.8.238/win/43180/step_8.txt
>
> "The command line is too long" Maybe some arts could be passed via a response 
> file instead?

Hi @thakis :
Thank you.

I'm a newbie. I tried to arc diff but arc shows `ERR_CLOSED: This revision has 
already been closed.`. Should I create a new differential diff for it?


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

[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.

In D86671#2837818 , @gnossier wrote:

> Is this ready to merge soon?

Hi  @gnossier:
I'm waiting for feedbacks from reviewer.

Hi @aaron.ballman:
Nathan is helping me to review this patch, but seems he is not here recently. 
Do you have any suggestion about how to keep the ball rolling in this 
situation? If it was done, can I apply the right to land it by self?


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

[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.

@njames93, Ping


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

[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.

In D86671#2754542 , @njames93 wrote:

> In D86671#2750957 , @dougpuob wrote:
>
>> Hi @njames93:
>> Could you do me a favor? Because it is my first patch, something I'm not 
>> sure. I'm confused about can I land this patch now? I read the "LLVM 
>> Code-Review Policy and Practices" document, it said patches can be landed if 
>> received a LGTM, but seems you are still reviewing.
>
> If you have made significant changes (excluding what a reviewer asks when 
> giving an LGTM) Its best to get those further changes also reviewed.

Thank you for your reply and suggestion in code, I will try it.


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

[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 @njames93:
Could you do me a favor? Because it is my first patch, something I'm not sure. 
I'm confused about can I land this patch now? I read the "LLVM Code-Review 
Policy and Practices" document, it said patches can be landed if received a 
LGTM, but seems you are still reviewing.


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

[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 @njames93

I tried to create a class, `HungarianNotation`, and put all related functions 
into the class. Then I can call `configurationDiag()` in 
`HungarianNotation::checkOptionValid()` function.
How about this way?

The new class.

  struct HungarianNotation {
  public:
HungarianNotation(ClangTidyContext *Context = nullptr);
  
bool checkOptionValid(int StyleKindIndex, StringRef StyleString,
  bool HasValue);
...

Use the configurationDiag() in checkOptionValid() function

  bool HungarianNotation::checkOptionValid(int StyleKindIndex,
   StringRef StyleString, bool 
HasValue) {
..
if (HasValue)
  Context->configurationDiag("invalid identifier naming option '%0'")
  << StyleString;
  
return false;
  }

Call the checkOptionValid() here in getFileStyleFromOptions().

  static IdentifierNamingCheck::FileStyle
  getFileStyleFromOptions(const ClangTidyCheck::OptionsView ,
  ClangTidyContext ) {
  ...
  StyleString.append("HungarianPrefix");
  auto HPTOpt =
  Options.get(StyleString);
  HN.checkOptionValid(I, StyleString, HPTOpt.hasValue());
  ...


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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:124-126
   DiagnosticBuilder
   configurationDiag(StringRef Description,
 DiagnosticIDs::Level Level = DiagnosticIDs::Warning);

njames93 wrote:
> You don't need a ClangTidyContext, this function is all that's needed.
But the `getFileStyleFromOptions()` is a static function in 
`IdentifierNamingCheck.cpp` file. It can't access `configurationDiag()` 
directly.


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

[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 @njames93,

That's ok. You have helped me a lots. Thank you.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:450
+if (!isHungarianNotationSupportedStyle(I) && HPTOpt.hasValue())
+  Options.diagnoseInvalidConfigOption(StyleString);
+StyleString.resize(StyleSize);

njames93 wrote:
> 
To use configurationDiag() function in getFileStyleFromOptions(), seems it 
needs to pass a ClangTidyContext object to getFileStyleFromOptions(). How about 
changing like the following?

```
getFileStyleFromOptions(const ClangTidyCheck::OptionsView ,
ClangTidyContext ) {
...

if (!isHungarianNotationSupportedStyle(I) && HPTOpt.hasValue())
  Context.configurationDiag("invalid identifier naming option '%0'")
  << StyleString;

...
}
```


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

[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.

Ping @njames93


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

[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.

Addressed comments by @njames93. Including adding warning message for 
unsupported options in config file, refining code in getFileStyleFromOptions(), 
and for consistent reason to use llvm::yaml::parseBool() function instead of 
checking On/Off string.


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

[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.

In D86671#2456161 , @njames93 wrote:

> One last point, is there a way to validate that these options are only set 
> where it makes sense. If someone tries to use say 
> `MacroDefinitionHungarianPrefix` That could be warning worthy?

Hi @njames93, it's a good idea. I am trying to do it, thank you.


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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob marked an inline comment as done.
dougpuob added a comment.

Hi @njames93, thank you for your review suggestions, I have improved them and 
against my change to the main branch.

I encounter a problem on the Buildbot for Windows only. Several people 
encounter also to the same problem that their build failed at an unrelated 
regression test (llvm\test\CodeGen\XCore\threads.ll).


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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 309767.
dougpuob added a comment.

- Improved code review suggestions from @njames93. Including move the 
IdentifierNamingCheck::HNOption variable to 
IdentifierNamingCheck::FileStyle::HNOption, use try_emplace() instead of 
insert() and lookup().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h

Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
@@ -84,19 +84,26 @@
   struct FileStyle {
 FileStyle() : IsActive(false), IgnoreMainLikeFunctions(false) {}
 FileStyle(SmallVectorImpl> &,
-  bool IgnoreMainLike)
-: Styles(std::move(Styles)), IsActive(true),
-  IgnoreMainLikeFunctions(IgnoreMainLike) {}
+  HungarianNotationOption HNOption, bool IgnoreMainLike)
+: Styles(std::move(Styles)), HNOption(std::move(HNOption)),
+  IsActive(true), IgnoreMainLikeFunctions(IgnoreMainLike) {}
 
 ArrayRef> getStyles() const {
   assert(IsActive);
   return Styles;
 }
+
+const HungarianNotationOption () const {
+  assert(IsActive);
+  return HNOption;
+}
+
 bool isActive() const { return IsActive; }
 bool isIgnoringMainLikeFunction() const { return IgnoreMainLikeFunctions; }
 
   private:
 SmallVector, 0> Styles;
+HungarianNotationOption HNOption;
 bool IsActive;
 bool IgnoreMainLikeFunctions;
   };
@@ -121,8 +128,6 @@
   const std::string CheckName;
   const bool GetConfigPerFile;
   const bool IgnoreFailedSplit;
-
-  IdentifierNamingCheck::HungarianNotationOption HNOption;
 };
 
 } // namespace readability
Index: clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
@@ -230,22 +230,16 @@
 IdentifierNamingCheck::HungarianNotationOption ) {
 
   // Options
-  static constexpr std::pair Options[] = {
+  static constexpr std::pair General[] = {
   {"TreatStructAsClass", "false"}};
-  for (const auto  : Options) {
-std::string Val = HNOption.General.lookup(Opt.first);
-if (Val.empty())
-  HNOption.General.insert({Opt.first, Opt.second.str()});
-  }
+  for (const auto  : General)
+HNOption.General.try_emplace(G.first, G.second);
 
   // Derived types
   static constexpr std::pair DerivedTypes[] = {
   {"Array", "a"}, {"Pointer", "p"}, {"FunctionPointer", "fn"}};
-  for (const auto  : DerivedTypes) {
-std::string Val = HNOption.DerivedType.lookup(Other.first);
-if (Val.empty())
-  HNOption.DerivedType.insert({Other.first, Other.second.str()});
-  }
+  for (const auto  : DerivedTypes)
+HNOption.DerivedType.try_emplace(DT.first, DT.second);
 
   // C strings
   static constexpr std::pair CStrings[] = {
@@ -253,11 +247,8 @@
   {"char[]", "sz"},
   {"wchar_t*", "wsz"},
   {"wchar_t[]", "wsz"}};
-  for (const auto  : CStrings) {
-std::string Val = HNOption.CString.lookup(CStr.first);
-if (Val.empty())
-  HNOption.CString.insert({CStr.first, CStr.second.str()});
-  }
+  for (const auto  : CStrings)
+HNOption.CString.try_emplace(CStr.first, CStr.second);
 
   // clang-format off
   static constexpr std::pair PrimitiveTypes[] = {
@@ -305,11 +296,8 @@
 {"long","l"   },
 {"ptrdiff_t",   "p"   }};
   // clang-format on
-  for (const auto  : PrimitiveTypes) {
-std::string Val = HNOption.PrimitiveType.lookup(Type.first);
-if (Val.empty())
-  HNOption.PrimitiveType.insert({Type.first, Type.second.str()});
-  }
+  for (const auto  : PrimitiveTypes)
+HNOption.PrimitiveType.try_emplace(PT.first, PT.second);
 
   // clang-format off
   static constexpr std::pair UserDefinedTypes[] = {
@@ -343,11 +331,8 @@
   {"UINT64",  "u64" },
   {"PVOID",   "p"   } };
   // clang-format on
-  for (const auto  : UserDefinedTypes) {
-std::string Val = HNOption.UserDefinedType.lookup(Type.first);
-if (Val.empty())
-  HNOption.UserDefinedType.insert({Type.first, Type.second.str()});
-  }
+  for (const auto  : UserDefinedTypes)
+HNOption.UserDefinedType.try_emplace(UDT.first, UDT.second);
 }
 
 static constexpr StringRef HNOpts[] = {"TreatStructAsClass"};
@@ -363,14 +348,14 @@
 Buffer.assign({Section, "General.", Opt});
 std::string Val = Options.get(Buffer, "");
 if (!Val.empty())
-  HNOption.General[Opt] = Val;
+  

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

[Douglas Chen via Phabricator via cfe-commits]

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

Hi @aaron.ballman and @Eugene.Zelenko, thank you for your suggestions. I will 
improve them and upload my diff later.


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

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

[Douglas Chen via Phabricator via cfe-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 CStrings[] = {
+  {"char*", "sz"}, {"char", "sz"}, {"wchar_t*", "wsz"}, {"wchar_t", 
"wsz"}};
+  for (const auto  : 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 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 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 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 ) {
+getFileStyleFromOptions(const ClangTidyCheck::OptionsView ,
+IdentifierNamingCheck::HungarianNotationOption ) {

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

[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.

ping?


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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:130
 m(ObjcIvar) \
+m(HungarianNotation) \
 

njames93 wrote:
> Is this line needed?
I will remove it. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:241
+  // Options
+  const static llvm::StringMap Options = {
+  {"TreatStructAsClass", "false"}};

njames93 wrote:
> As you never use map like access for this, shouldn't it be an array.
> The same goes for all the other StringMaps in this function
Thank you. I will change it.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:368
+
+  std::vector HNOpts = {"TreatStructAsClass"};
+  for (auto const  : HNOpts) {

njames93 wrote:
> However for this I can see that its mapping the same options as `Options` in 
> `getHungarianNotationDefaultConfig()`.
> Maybe `HNOpts` should be removed from here, `Option` from 
> `getHungarianNotationDefaultConfig()` taken out of function scope and iterate 
> over that array below. 
> 
> A similar approach could be made with HNDerviedTypes
I will move `HNOpts` and `HNDerviedTypes` outward to the top of the 
`getHungarianNotationDefaultConfig()` function. If the arrays are defined as 
static, is there any difference btw inside or outside of function, or did I 
misunderstand your meaning?  



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:369-370
+  std::vector HNOpts = {"TreatStructAsClass"};
+  for (auto const  : HNOpts) {
+std::string Key = Section + "General." + Opt;
+std::string Val = Options.get(Key, "");

njames93 wrote:
> Building these temporary strings is expensive. Better off having a 
> SmallString contsructed outside the loop and fill the string for each 
> iteration, saved on allocations.
> The same buffer can be reused below for the other loops
Good idea, thank you.


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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:441-456
+static IdentifierNamingCheck::HungarianPrefixOption
+parseHungarianPrefix(std::string OptionVal) {
+  for (auto  : OptionVal)
+C = toupper(C);
+
+  if (std::string::npos != OptionVal.find("LOWER_CASE"))
+return IdentifierNamingCheck::HungarianPrefixOption::HPO_LowerCase;

njames93 wrote:
> This isn't really needed if you have the mapping defined, `Options.get` works 
> with enums, just look at how CaseType is parsed and stored. If you want to 
> map multiple strings to a single enum constant that can also work by putting 
> both strings in the mapping.
> This method also validates inputs and will print out an error if a user 
> supplies a value that can't be converted.
Good idea. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:460
 getNamingStyles(const ClangTidyCheck::OptionsView ) {
+  static IdentifierNamingCheck::HungarianNotationOption HNOption;
+  HNOption.clearAll();

njames93 wrote:
> This function can be called multiple times per translation unit when looking 
> through header files if `GetConfigPerFile` is enabled. Making this static 
> will mean that each file thats read could potentially alter other files style 
> configuration.
> Maybe a smarter way about this is rather than this function returning a 
> vector of naming styles, it returns a struct which contains the Hungarian 
> options and a vector of the styles. Doing this would probably also mean you 
> don't need to store a reference to this in the `NamingStyle`.
I removed the static variable from the function and a reference in 
`NamingStyle`.


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

[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.

Reply code review suggestions. I will upload my change later.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:483
+  std::move(CaseOptional), std::move(Prefix), std::move(Postfix),
+  std::move(HPOpt), HNOption});
+} else {

aaron.ballman wrote:
> Would it make sense to declare `HNOption` as a typical local variable (no 
> need for the `clearAll()` API since you can default construct properly), and 
> pass an rvalue reference rather than an lvalue reference (so call 
> std::move()) here, as with the other arguments)?
OK~ I moved the `HNOption` to IdentifierNamingCheck class as its private data 
member. Then give value by constructor initializer list (instead of 
`clearAll()`).



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:549
+  for (const auto  : HNOption.CString) {
+auto Key = CStr.getKey().str();
+if (ModifiedTypeName.find(Key) == 0) {

aaron.ballman wrote:
> You shouldn't use `auto` when the type isn't spelled out in the 
> initialization. (Same below)
OK~ Thank you. I will check them.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:631
+
+  if (CRD->isUnion()) {
 return "";

aaron.ballman wrote:
> Elide braces around single-line if statements, per our usual coding 
> guidelines.
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:635-636
 
-  if (clang::Decl::Kind::EnumConstant == Decl->getKind() ||
-  clang::Decl::Kind::CXXMethod == Decl->getKind()||
-  clang::Decl::Kind::Function == Decl->getKind()) {
+  if (CRD->isStruct()) {
+if (!isHungarianNotationOptionEnabled("TreatStructAsClass",
+  HNOption.General)) {

aaron.ballman wrote:
> Combine into a single `if`?
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:644
+  bool IsAbstractClass = false;
+  for (const auto *Method : CRD->methods()) {
+if (Method->isPure()) {

aaron.ballman wrote:
> No need to do this manually, you can use `CXXRecordData::isAbstract()` since 
> it's already computed.
Nice, thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:651-658
+  std::string Prefix;
+  if (IsAbstractClass) {
+  Prefix = "I";
+  } else {
+  Prefix = "C";
+  }
+

aaron.ballman wrote:
> `return CRD->isAbstract() ? "I" : "C";`
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:662-666
+  std::string Name = ECD->getType().getAsString();
+  if (std::string::npos != Name.find("enum")) {
+Name = Name.substr(strlen("enum"), Name.length() - strlen("enum"));
+Name = Name.erase(0, Name.find_first_not_of(" "));
+  }

aaron.ballman wrote:
> This is a bit worrying -- can you not look at the `DeclContext` for the 
> `EnumConstantDecl` to find the parent `EnumDecl` and ask for its name 
> directly?
YES. I get the its `QualType` from `EnumConstantDecl::getType()`. Then get 
`EnumDecl` name via `QualType::getAsString()` (if enum tag name is "DataType", 
the string I get is "enum DataType").

```
static std::string getHungarianNotationEnumPrefix(const EnumConstantDecl *ECD) {
  std::string Name = ECD->getType().getAsString();
  // ...
}
```



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:703
+
+static std::string getDeclTypeName(const clang::NamedDecl *ND) {
+  const auto VD = dyn_cast(ND);

aaron.ballman wrote:
> You can drop the `clang::` bit.
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:704
+static std::string getDeclTypeName(const clang::NamedDecl *ND) {
+  const auto VD = dyn_cast(ND);
+  if (!VD) {

aaron.ballman wrote:
> When using type deduction, the coding style is to put all pointers and 
> qualifiers on the declaration as well (so we don't have to infer them from 
> context), so this should be `const auto *`.
OK, thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:806
+static std::string getHungarianNotationPrefix(
+const clang::Decl *D,
+IdentifierNamingCheck::HungarianNotationOption ) {

aaron.ballman wrote:
> Can drop the `clang::` bit.
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:808-809
+IdentifierNamingCheck::HungarianNotationOption ) {
+  const auto ND = dyn_cast(D);
+  if (!ND) {
+return "";

aaron.ballman wrote:
> `const auto *` and elide braces (I'll stop pointing these out, can you do a 
> pass over the whole check?)
Sure! it's what 

[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 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
+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

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

[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.

In D86671#2342016 , @njames93 wrote:

> In D86671#2341838 , @dougpuob wrote:
>
>> Hi @njames93,
>>
>> It's a smart idea, the rework for it is worth. There is a special case if 
>> lowercase name with Hungarian prefix, it possibly makes variable ambiguous, 
>> like the `Case1`. How about separating them and `aNy_CasE` with an 
>> underscore, like `Case2` ?
>>
>>   // Case1
>>   bool bRIGHT_LEVEL; // UPPER_CASE
>>   bool bRightLevel;  // CamelCase
>>   bool bRight_Level; // Camel_Snake_Case
>>   bool baNy_CasE;// aNy_CasE
>>   bool bright_level; // lower_case
>>   bool brightLevel;  // camelBack
>>   bool bright_Level; // camel_Snake_Back
>>   .^^ <-- right? bright?
>>   
>>   // Case2
>>   bool bRIGHT_LEVEL; // UPPER_CASE
>>   bool bRightLevel;  // CamelCase
>>   bool bRight_Level; // Camel_Snake_Case
>>   bool b_aNy_CasE;   // aNy_CasE
>>   bool b_right_level;// lower_case
>>   bool b_rightLevel; // camelBack
>>   bool b_right_Level;// camel_Snake_Back
>>   .^^^ <-- add an underscore
>
> That still has hidden surprises. Maybe instead of a bool, an enum is used for 
> controlling hungarian prefix (Off|On|...).
> Can't think of a good name for the third option but it would do the inserting 
> of '_' (bright_level ->b_right_level) or capitalising the first word of the 
> identifier (brightLevel -> bRightLevel).

Maybe it doesn't need a new name, how about (`Off|On|lower_case|camelBack`) or 
(`Off|On|sz_lower_case|szCamelBack`)?


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

[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.

In D86671#2330689 , @njames93 wrote:

> Is this diff been created incorrectly again?
>
> Taking a step back, Is Hungarian notation really a case style, Seems to me 
> its mainly about the prefix and a user may want `DWORD dwUPPER_CASE`, Right 
> now there is no way of adopting that.
> Maybe extend the options for hungarian type decls to
>
>   Case
>   Prefix
>   Suffix
>   HungarianPrefix
>
> `HungarianPrefix` would likely be a bool and if enabled, it would be 
> prepended to `Prefix`
> I could see a situation like this
>
>   [Options]
>   // VariableCase: UPPER_CASE
>   // VariablePrefix: PRE_
>   // VariableSuffix: _POST
>   // VariableHungarianPrefix: true
>   
>   DWORD MyVariable; -> DWORD dwPRE_MY_VARIABLE_POST;
>
> This would give users full control of exactly how they want their 
> declarations with no hidden surprises.
>
> Granted this approach would require a little rework but it would satisfy more 
> users.

Hi @njames93,

It's a smart idea, the rework for it is worth. There is a special case if 
lowercase name with Hungarian prefix, it possibly makes variable ambiguous, 
like the `Case1`. How about separating them and `aNy_CasE` with an underscore, 
like `Case2` ?

  // Case1
  bool bRIGHT_LEVEL; // UPPER_CASE
  bool bRightLevel;  // CamelCase
  bool bRight_Level; // Camel_Snake_Case
  bool baNy_CasE;// aNy_CasE
  bool bright_level; // lower_case
  bool brightLevel;  // camelBack
  bool bright_Level; // camel_Snake_Back
  .^^ <-- right? bright?
  
  // Case2
  bool bRIGHT_LEVEL; // UPPER_CASE
  bool bRightLevel;  // CamelCase
  bool bRight_Level; // Camel_Snake_Case
  bool b_aNy_CasE;   // aNy_CasE
  bool b_right_level;// lower_case
  bool b_rightLevel; // camelBack
  bool b_right_Level;// camel_Snake_Back
  .^^^ <-- add an underscore


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

[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 and @njames93,
I addressed your code review suggestions and supported Hungarian Notation 
prefix for decl of enum and class(option) at latest patch. Unfortunately, I 
encountered a problem that new patch failed on remote BuildBot, it showed the 
"No such file or directory" error message when it was trying to call 
apply_patch2.py!_apply_diff(). Do you have any idea what is going on? Do you 
suggest I create a new Diff(new diff id) for it ?


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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 297443.
dougpuob added a comment.

- Support to add Class prefix for Hungarian Notation.
- Support to add Enum prefix for Hungarian Notation.
- Support `unsigned long long`, `ULONG`, and `HANDLE` types and others.
- Support options for Hungarian Notation in config file.
- Added more test cases.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation-cfgfile.cpp
@@ -72,34 +72,34 @@
 // RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.long-int, value: li   }, \
 // RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.long, value: l}, \
 // RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.ptrdiff_t   , value: p}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.BOOL, value: b}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.BOOLEAN , value: b}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.BYTE, value: by   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.CHAR, value: c}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.UCHAR   , value: uc   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.SHORT   , value: s}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.USHORT  , value: us   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.WORD, value: w}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.DWORD   , value: dw   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.DWORD32 , value: dw32 }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.DWORD64 , value: dw64 }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.LONG, value: l}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.ULONG   , value: ul   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.ULONG32 , value: ul32 }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.ULONG64 , value: ul64 }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.ULONGLONG   , value: ull  }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.HANDLE  , value: h}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.INT , value: i}, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.INT8, value: i8   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.INT16   , value: i16  }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.INT32   , value: i32  }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.INT64   , value: i64  }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.UINT, value: ui   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.UINT8   , value: u8   }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.UINT16  , value: u16  }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.UINT32  , value: u32  }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.UINT64  , value: u64  }, \
-// RUN: { key: readability-identifier-naming.HungarianNotation.PrimitiveType.PVOID   , 

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp:25
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: 
CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: 
szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: 
szHungarianNotation }, \

aaron.ballman wrote:
> dougpuob wrote:
> > njames93 wrote:
> > > Class names shouldn't use hungarian notation.
> > OK~ I have classified CheckOptions, and all test cases one by one in the 
> > next diff.
> > 
> > ```
> > // RUN:   -config='{ CheckOptions: [ \
> > // RUN: { key: readability-identifier-naming.ClassMemberCase
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.ConstantCase   
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.ConstantMemberCase 
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.ConstantParameterCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: 
> > readability-identifier-naming.ConstantPointerParameterCase , value: 
> > szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.ConstexprVariableCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.GlobalConstantCase 
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.GlobalConstantPointerCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.GlobalVariableCase 
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.LocalConstantCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.LocalConstantPointerCase   
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.LocalPointerCase   
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.LocalVariableCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.MemberCase 
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.ParameterCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.PointerParameterCase   
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.PrivateMemberCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.StaticConstantCase 
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.StaticVariableCase 
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.StructCase 
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.UnionCase  
> >   , value: szHungarianNotation }, \
> > // RUN: { key: readability-identifier-naming.VariableCase   
> >   , value: szHungarianNotation }  \
> > // RUN:   ]}'
> > ```
> > Class names shouldn't use hungarian notation.
> 
> That may be debatable as I've definitely seen `C` used as a prefix for class 
> names and `I` used as a prefix for pure virtual class names (interfaces). 
> Doing a quick search on Google brings up evidence that this isn't uncommon.
Agree both of you because I saw them in different projects. I will add this 
feature as an option (default is off, user can enable it in .clang-tidy).


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

[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.

In D86671#2304337 , @njames93 wrote:

> Not strictly relevant here, but this does open up the idea of enforcing the 
> style where an enum constant is prefixed by the initials of the enum name.

I like this idea. There is a case when `EnumConstantPrefix` and 
`EnumConstantCase=szHungarianNotation` options are set, it may take similar 
affect(which will be the first) or be overwritten? I can have it a try later.

Showing my conception as the following:

  // [Before]
  typedef enum {
  RevValid = -1,
  RevNone  = 0, 
  RevCrlReason = 1, 
  RevHold  = 2, 
  RevKeyCompromise = 3, 
  RevCaCompromise  = 4  
  } REVINFO_TYPE;
  
  // [After]
  typedef enum {
  rtRevValid = -1
  rtRevNone  = 0,
  rtRevCrlReason = 1,
  rtRevHold  = 2,
  rtRevKeyCompromise = 3,
  rtRevCaCompromise  = 4 
  } REVINFO_TYPE;
  
  // [After] EnumConstantPrefix first case
  // EnumConstantCase=snHungarianNotation
  // EnumConstantPrefix=pre_
  typedef enum {
  pre_rtRevValid = -1
  pre_rtRevNone  = 0,
  pre_rtRevCrlReason = 1,
  pre_rtRevHold  = 2,
  pre_rtRevKeyCompromise = 3,
  pre_rtRevCaCompromise  = 4 
  } REVINFO_TYPE;




Comment at: 
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp:25
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: 
CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: 
szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: 
szHungarianNotation }, \

njames93 wrote:
> Class names shouldn't use hungarian notation.
OK~ I have classified CheckOptions, and all test cases one by one in the next 
diff.

```
// RUN:   -config='{ CheckOptions: [ \
// RUN: { key: readability-identifier-naming.ClassMemberCase  , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantMemberCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantParameterCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstantPointerParameterCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ConstexprVariableCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.GlobalConstantCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.GlobalConstantPointerCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.GlobalVariableCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalConstantCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalConstantPointerCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalPointerCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.LocalVariableCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.MemberCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.ParameterCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.PointerParameterCase , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.PrivateMemberCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.StaticConstantCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.StaticVariableCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.StructCase   , 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.UnionCase, 
value: szHungarianNotation }, \
// RUN: { key: readability-identifier-naming.VariableCase , 
value: szHungarianNotation }  \
// RUN:   ]}'
```


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

[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.

In D86671#2293128 , @aaron.ballman 
wrote:

> In D86671#2284078 , @dougpuob wrote:
>
>> Hi @aaron.ballman
>> About changing  `size_t nLength` to `cbLength`. I searched MSVC folders with 
>> `size_t`, many names of variable start with `n`, or `i` in MFC related 
>> files. So I prefer to keep it starts with `n`. Another side to name starts 
>> with `cb`, I found variables like cbXxx are defined with ULONG, DWORD, or 
>> UCHAR type.
>
> I think the important point is that `cb` is used for APIs that specify a 
> count of bytes, whereas `i`, `n`, and `l` are used for integers (there is no 
> specific prefix for `size_t` that I'm aware of or that MSDN documents, so the 
> lack of consistency there is not too surprising). That's why I mentioned that 
> using a heuristic approach may allow you to identify the parameters that are 
> intended to be a count of bytes so that you can use the more specific prefix 
> if there's more specific contextual information available.

Hi @aaron.ballman

1. **Making `sz` as a prefix to the `char*` parameters of `strncpy()` and 
`strncat()` functions.** I read the code of `strncpy` function, seems 
null-terminated to parameters is essential. I thought those parameters to 
strXxx() are also able to the parameter of `strlen` function knowing the length 
of a C string(it can be passed to be a parameter or assigned to be a variable, 
those point to the identical memory. strXxx functions tell the end by null 
character in the memory). Moreover, I searched source code of OpenSSL, I found 
the project uses most of `char*` as C string, and data buffer to retrieve data 
with `unsigned char*`.  `unsigned char` is a primitive type in C89 (is portable 
than `uint8_t`). Maybe current mechanism is a good way to hint users that 
`unsigned char*` is more explicit than `char*` if they want to treat it as a 
buffer to retrieve data.

2. **Adding heuristics to pick the correct prefix.** Do you mean that use the 
heuristics approach to give naming suggestion as warning message, or correct it 
with the `--fix` option? Is there any existing in this project can be a sample 
for me? If my thought about the heuristics is correct. The information of 
parameters to functions I can query its **type**, **name**, and **location**. 
As we have discussed that the declaration type is insufficient to tell `sz` for 
`char*`, or `cb` for count of bytes instead `i`, `n`, or `l`, so the **name** 
and **location** provide more information for comparing names with string 
mapping tables and location relationship between parameters/variables. Take 
`FILE * fopen ( const char * filename, const char * mode );` for example(C 
String). It will change `filename` to `szFilename` if the `name` string is in 
the mapping table, change `mode` to `pcMode` if `mode` string is not in the 
mapping table. Take `size_t fread ( void * ptr, size_t size, size_t count, FILE 
* stream );` for example(count of bytes). It will change `size` to `cbSize`, 
because its previous parameter is a void pointer.

  I can smell that there is always exceptional cases, and not good for 
maintainability.

3. **Changing `i`, `n`, and `l` to `cb` for APIs that specify a count of 
bytes.** I have no idea how to distinguish when should add the `cb` instead of 
integer types for heuristics. If I was the user, I wish clang-tidy don't change 
integer types from `cbXxx` to `iCbXxx` or `nCbXxx`. Keep `cb` with default or 
an option in config file, like `IgnoreParameterPrefix`, `IgnoreVariablePrefix`.

4. **Why not use Decl->getType()->getAsString()** I printed the differences as 
a log for your reference. 
(https://gist.github.com/dougpuob/e5a76db6e2c581ba003afec3236ab6ac#file-clang-tidy-readability-identifier-naming-hungarian-notation-log-L191)
 Previous I mentioned "if users haven't specific the include directories, the 
checking result may look messy". This concern will not happened because the 
`clang-diagnostic-error`, if users didn't specific header directories, 
clang-tidy will show the error (L414 
).




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

[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
About changing  `size_t nLength` to `cbLength`. I searched MSVC folders with 
`size_t`, many names of variable start with `n`, or `i` in MFC related files. 
So I prefer to keep it starts with `n`. Another side to name starts with `cb`, 
I found variables like cbXxx are defined with ULONG, DWORD, or UCHAR type.

Maybe make them as options in config (.clang-tidy) is more practical. The 
prefixes in the default table follows its abbreviation or acronym of data type.


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

[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.

Replied comments by @aaron.ballman




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:216
+{"long","l"},
+{"long long",   "ll"},
+{"unsigned long",   "ul"},

aaron.ballman wrote:
> `unsigned long long` -> `ull`
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:225
+{"WORD","w"},
+{"DWORD",   "dw"}};
+  // clang-format on

aaron.ballman wrote:
> `ULONG` -> `ul`
> `HANDLE` -> `h`
> 
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:236
+  PrefixStr = "fn"; // Function Pointer
+} else if (QT->isPointerType()) {
+  // clang-format off

aaron.ballman wrote:
> I'm not certain how valid it is to look at just the type and decide that it's 
> a null-terminated string. For instance, the following is not an uncommon 
> pattern: `void something(const char *buffer, size_t length);` and it would be 
> a bit strange to change that into `szBuffer` as that would give an indication 
> that the buffer is null terminated. You could look at surrounding code for 
> additional information though, like other parameters in a function 
> declaration. As an aside, this sort of heuristic search may also allow you to 
> change `length` into `cbLength` instead of `nLength` for conventions like the 
> Microsoft one.
> 
> However, for local variables, I don't see how you could even come up with a 
> heuristic like you could with parameters, so I'm not certain what the right 
> answer is here.
OK (size_t nLength --> cbLength)

About the `void something(const char *buffer, size_t length)` pattern. `char` 
is a special type which can express signed char and unsigned char.  One can 
express C string(ASCII), another expresses general data buffer(in hex). I think 
you are mentioning when it is a general data buffer. I agree with you if it 
changed the name from `buffer` to `szBuffer` for general data buffer is 
strange. I prefer to use `uint8_t` or `unsigned char` instead.

How about adding a new config option for it? like, `  - { key: 
readability-identifier-naming.HungarainNotation.DontChangeCharPointer   , 
value: 1 }` Users can make decision for their projects. For consistency with 
HN, the default will still be changed to `szBuffer` in your case.

If I add the option, does it make sense to you from your side?



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:309
+
+  if (PtrCount > 0) {
+for (size_t Idx = 0; Idx < PtrCount; Idx++) {

aaron.ballman wrote:
> No need for this `if` statement, the `for` loop won't run anyway if `PtrCount 
> == 0`.
OK! redundant code.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:319
+std::string
+IdentifierNamingCheck::getDeclTypeName(const clang::NamedDecl *Decl) const {
+  const ValueDecl *ValDecl = dyn_cast(Decl);

aaron.ballman wrote:
> `ND` instead of `Decl`.
> 
> The function name doesn't really help me to understand why you'd call this as 
> opposed to getting the type information as a string from the `NamedDecl` 
> itself. I'm a bit worried about maintaining this code as the language evolves 
> -- Clang will get new keywords, and someone will have to remember to come 
> update this code. Could you get away with using 
> `Decl->getType()->getAsString()` and working with that rather than going back 
> to the original source code and trying to parse manually?
OK, I should do it. (ND instead of Decl.)

Use `Decl->getType()->getAsString()` is a good way. But HN is a strongly-typed 
notation, if users haven't specific the include directories, the checking 
result may look messy (it will be changed to unexpected type). So I parse a 
string with `SourceLocation`, just for user-friendly.

I understand your consideration, maintaining-friendly is also important. I can 
implement it with `Decl->getType()->getAsString()`, if my explanation can't 
convince you still. It is also fine to me. Or you think it just need a better 
appropriate function name?



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:320
+IdentifierNamingCheck::getDeclTypeName(const clang::NamedDecl *Decl) const {
+  const ValueDecl *ValDecl = dyn_cast(Decl);
+  if (!ValDecl) {

aaron.ballman wrote:
> `const auto *` since the type is spelled out in the initialization.
OK.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:554
+  case IdentifierNamingCheck::CT_HungarianNotation: {
+const NamedDecl *ND = dyn_cast(InputDecl);
+const std::string TypePrefix =

aaron.ballman wrote:
> `const auto *` because the type is spelled out in the initialization.
OK.

[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.

In D86671#2259492 , @njames93 wrote:

> In D86671#2259443 , @dougpuob wrote:
>
>> In D86671#2259364 , @njames93 wrote:
>>
>>> Did you upload this incorrectly again, context is missing and seems to be a 
>>> relative diff from a previous version of this patch?
>>
>> Sorry for it, I think it's my bad. It is possible that I manually merged the 
>> last master(github) with changes then updated them both via web interface ...
>>
>> Can I fix it if switch back to the base (`14948a0`) then merge all my 
>> changes, then update the diff again via web interface? Or do you have any 
>> better suggestion?
>>
>> I am curious about how do you know this mistake? You got error messages with 
>> `arc patch D86671` ?
>
> The no context is easy to spot as phab says context not available. Its easy 
> to find knowing that there is no mention of hungarian notation in the trunk 
> version of IdentifierNamingCheck.cpp, yet there is mention of that in the 
> before diff.
>
> The way I do my patches is I create a branch from the current master. Then 
> all commits go into that branch. When its time to update the PR I can just do 
> a diff from  to .
> Though I do use arcanist for my patches
>
>   arc diff master
>
> arcanist will check to see if the current branch has tags for PR and 
> automatically update that PR. Otherwise it will create a new PR.
> If it goes to create a new PR instead of updating an existing one you can 
> pass update
>
>   arc diff master --update D86671

@njames93, thank you. I updated three updates with the way you told me, seems 
work fine.


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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 291371.
dougpuob added a comment.

Fixed crash on Windows when run regression test (llvm-lit for 
`readability-identifier-naming.cpp` file).

This is because over range parameter made ctor of std::string copying out of 
range memory from source. The over range parameter came from 
SourceManager::getCharacterData() function with the SourceLocation returning 
value of ValDecl->getLocation().

Find the terminated char insteads of calling `ValDecl->getLocation()` function.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,263 @@
+typedef signed char int8_t; // NOLINT
+typedef short   int16_t;// NOLINT
+typedef longint32_t;// NOLINT
+typedef long long   int64_t;// NOLINT
+typedef unsigned char   uint8_t;// NOLINT
+typedef unsigned short  uint16_t;   // NOLINT
+typedef unsigned long   uint32_t;   // NOLINT
+typedef unsigned long long  uint64_t;   // NOLINT
+#ifndef _WIN32
+typedef unsigned long long  size_t; // NOLINT
+#endif
+typedef longintptr_t;   // NOLINT
+typedef unsigned long   uintptr_t;  // NOLINT
+typedef long intptrdiff_t;  // NOLINT
+typedef unsigned char   BYTE;   // NOLINT
+typedef unsigned short  WORD;   // NOLINT
+typedef unsigned long   DWORD;  // NOLINT
+typedef int BOOL;   // NOLINT
+typedef BYTEBOOLEAN;// NOLINT
+#define NULL(0) // NOLINT
+
+// RUN: clang-tidy %s -checks=readability-identifier-naming \
+// RUN:   -config="{CheckOptions: [\
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.MemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.VariableCase   , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ParameterCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalPointerCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase , value: CamelCase } \
+// RUN:   ]}"
+
+class UnlistedClass { public: mutable int ValInt; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: invalid case style for member 'ValInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}class UnlistedClass { public: mutable int iValInt; };
+
+UnlistedClass cUnlisted2;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'cUnlisted2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass Unlisted2;
+
+UnlistedClass objUnlistedClass3;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'objUnlistedClass3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass UnlistedClass3;
+
+typedef int INDEX;
+INDEX iIndex = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global variable 'iIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}INDEX Index = 0;
+
+struct DataBuffer {
+mutable size_t Size;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: invalid case style for member 'Size' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}mutable size_t nSize;
+
+int  = iIndex;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global variable 'RefValueIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int  = Index;
+
+typedef void (*FUNC_PTR_HELLO)();
+FUNC_PTR_HELLO Hello = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'Hello' [readability-identifier-naming]
+// CHECK-FIXES: 

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 290726.
dougpuob added a comment.

- Fixed lint warnings and regression test failures on Windows.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,261 @@
+typedef signed char int8_t;  // NOLINT
+typedef short int16_t;   // NOLINT
+typedef long int32_t;// NOLINT
+typedef long long int64_t;   // NOLINT
+typedef unsigned char uint8_t;   // NOLINT
+typedef unsigned short uint16_t; // NOLINT
+typedef unsigned long uint32_t;  // NOLINT
+typedef unsigned long long uint64_t; // NOLINT
+typedef unsigned long long size_t;   // NOLINT
+typedef long intptr_t;   // NOLINT
+typedef unsigned long uintptr_t; // NOLINT
+typedef long int ptrdiff_t;  // NOLINT
+typedef unsigned char BYTE;  // NOLINT
+typedef unsigned short WORD; // NOLINT
+typedef unsigned long DWORD; // NOLINT
+typedef int BOOL;// NOLINT
+typedef BYTE BOOLEAN;// NOLINT
+#define NULL (0) // NOLINT
+
+// RUN: clang-tidy %s -checks=readability-identifier-naming \
+// RUN:   -config="{CheckOptions: [\
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.MemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.VariableCase   , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ParameterCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalPointerCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase , value: CamelCase } \
+// RUN:   ]}"
+
+class UnlistedClass { public: mutable int ValInt; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: invalid case style for member 'ValInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}class UnlistedClass { public: mutable int iValInt; };
+
+UnlistedClass cUnlisted2;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'cUnlisted2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass Unlisted2;
+
+UnlistedClass objUnlistedClass3;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'objUnlistedClass3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass UnlistedClass3;
+
+typedef int INDEX;
+INDEX iIndex = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global variable 'iIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}INDEX Index = 0;
+
+struct DataBuffer {
+mutable size_t Size;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: invalid case style for member 'Size' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}mutable size_t nSize;
+
+int  = iIndex;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global variable 'RefValueIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int  = Index;
+
+typedef void (*FUNC_PTR_HELLO)();
+FUNC_PTR_HELLO Hello = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'Hello' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}FUNC_PTR_HELLO fnHello = NULL;
+
+void *ValueVoidPtr = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global pointer 'ValueVoidPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pValueVoidPtr = NULL;
+
+ptrdiff_t PtrDiff = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global variable 'PtrDiff' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}ptrdiff_t pPtrDiff = NULL;
+
+const char 

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 290483.
dougpuob added a comment.

This is a test with `arc diff master --update D86671` command.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,266 @@
+typedef signed char int8_t; // NOLINT
+typedef short   int16_t;// NOLINT
+typedef longint32_t;// NOLINT
+typedef long long   int64_t;// NOLINT
+
+typedef unsigned char   uint8_t;// NOLINT
+typedef unsigned short  uint16_t;   // NOLINT
+typedef unsigned long   uint32_t;   // NOLINT
+typedef unsigned long long  uint64_t;   // NOLINT
+
+typedef unsigned intsize_t; // NOLINT
+typedef longintptr_t;   // NOLINT
+typedef unsigned long   uintptr_t;  // NOLINT
+typedef long intptrdiff_t;  // NOLINT
+
+typedef unsigned char   BYTE;   // NOLINT
+typedef unsigned short  WORD;   // NOLINT
+typedef unsigned long   DWORD;  // NOLINT
+
+typedef int BOOL;   // NOLINT
+typedef BYTEBOOLEAN;// NOLINT
+
+#define NULL(0) // NOLINT
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.MemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.VariableCase   , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ParameterCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalPointerCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase , value: CamelCase }, \
+// RUN:   ]}"
+
+class UnlistedClass { public: mutable int ValInt; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: invalid case style for member 'ValInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}class UnlistedClass { public: mutable int iValInt; };
+
+UnlistedClass cUnlisted2;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'cUnlisted2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass Unlisted2;
+
+UnlistedClass objUnlistedClass3;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'objUnlistedClass3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass UnlistedClass3;
+
+typedef int INDEX;
+INDEX iIndex = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global variable 'iIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}INDEX Index = 0;
+
+struct DataBuffer {
+mutable size_t Size;
+};
+// CHECK-MESSAGES: :[[@LINE-2]]:20: warning: invalid case style for member 'Size' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}mutable size_t nSize;
+
+int  = iIndex;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for global variable 'RefValueIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int  = Index;
+
+typedef void (*FUNC_PTR_HELLO)();
+FUNC_PTR_HELLO Hello = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for global pointer 'Hello' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}FUNC_PTR_HELLO fnHello = NULL;
+
+void *ValueVoidPtr = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for global pointer 'ValueVoidPtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pValueVoidPtr = NULL;
+
+ptrdiff_t PtrDiff = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:11: warning: invalid case style for global variable 'PtrDiff' 

[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.

In D86671#2259364 , @njames93 wrote:

> Did you upload this incorrectly again, context is missing and seems to be a 
> relative diff from a previous version of this patch?

Sorry for it, I think it's my bad. It is possible that I manually merged the 
last master(github) with changes then updated them both via web interface ...

Can I fix it if switch back to the base (`14948a0`) then merge all my changes, 
then update the diff again via web interface? Or do you have any better 
suggestion?

I am curious about how do you know this mistake? You got error messages with 
`arc patch D86671` ?


Repository:
  rCTE Clang Tools Extra

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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 290099.
dougpuob added a comment.

Improved suggestions of code review. (All suggestions from aaron.ballman)

1. Changed variables named with `Decl` to `InputDecl`.
2. Changed `TypeName.length()` --> `!TypeName.empty()`.
3. Added partial Microsft data types to the `HungarianNotationTable`.
4. Added `long long`, `long double`, `ptrdiff_t` to the 
`HungarianNotationTable`.
5. Added `char8_t`, `char16_t`, `char32_t` to the `HungarianNotationTable`. 
Variables name with `char8_t*` type start with `pc8`, Eg. `char8_t *pc8Value = 
0;`
6. Supported name of function pointer start with `fn`.
7. Supported to remove the `restrict` keyword in 
IdentifierNamingCheck::getDeclTypeName().
8. Removed `const_cast` from Keywords list.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -1,120 +1,266 @@
-#include 
-#include 
+typedef signed char int8_t; // NOLINT
+typedef short   int16_t;// NOLINT
+typedef longint32_t;// NOLINT
+typedef long long   int64_t;// NOLINT
+
+typedef unsigned char   uint8_t;// NOLINT
+typedef unsigned short  uint16_t;   // NOLINT
+typedef unsigned long   uint32_t;   // NOLINT
+typedef unsigned long long  uint64_t;   // NOLINT
+
+typedef unsigned intsize_t; // NOLINT
+typedef longintptr_t;   // NOLINT
+typedef unsigned long   uintptr_t;  // NOLINT
+typedef long intptrdiff_t;  // NOLINT
+
+typedef unsigned char   BYTE;   // NOLINT
+typedef unsigned short  WORD;   // NOLINT
+typedef unsigned long   DWORD;  // NOLINT
+
+typedef int BOOL;   // NOLINT
+typedef BYTEBOOLEAN;// NOLINT
+
+#define NULL(0) // NOLINT
 
 // RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
 // RUN:   -config="{CheckOptions: [\
-// RUN:   {key: readability-identifier-naming.VariableCase, value: szHungarianNotation}, \
+// RUN: {key: readability-identifier-naming.FunctionCase   , value: CamelCase },   \
+// RUN: {key: readability-identifier-naming.ClassCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.TypedefCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.MemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ClassMemberCase, value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ConstantMemberCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.VariableCase   , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.ParameterCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalPointerCase  , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalVariableCase , value: szHungarianNotation }, \
+// RUN: {key: readability-identifier-naming.GlobalFunctionCase , value: CamelCase }, \
 // RUN:   ]}"
 
-class UnlistedClass {};
-UnlistedClass Unlisted1;
-// CHECK-NOT: :[[@LINE-2]]
+class UnlistedClass { public: mutable int ValInt; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: invalid case style for member 'ValInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}class UnlistedClass { public: mutable int iValInt; };
 
 UnlistedClass cUnlisted2;
-// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for variable 'cUnlisted2' [readability-identifier-naming]
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'cUnlisted2' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}UnlistedClass Unlisted2;
 
 UnlistedClass objUnlistedClass3;
-// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for variable 'objUnlistedClass3' [readability-identifier-naming]
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for global variable 'objUnlistedClass3' [readability-identifier-naming]
 // CHECK-FIXES: {{^}}UnlistedClass UnlistedClass3;
 
 typedef int INDEX;
 INDEX iIndex = 0;
-// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'iIndex' [readability-identifier-naming]

[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.

Please no worry to give me your suggestions and feedback.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:185
+getHungarianNotationTypePrefix(const std::string ,
+   const NamedDecl *Decl) {
+  if (0 == TypeName.length()) {

aaron.ballman wrote:
> Please don't use `Decl` as the identifier since it mirrors the name of a type.
OK~ I will fix it. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:186
+   const NamedDecl *Decl) {
+  if (0 == TypeName.length()) {
+return TypeName;

aaron.ballman wrote:
> `if (TypeName.empty())`
OK~ I will fix it. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:208
+{"wchar_t", "wc"},
+{"short",   "s"},
+{"signed",  "i"},

aaron.ballman wrote:
> It's been a long while since I thought about Hungarian notation, but I 
> thought this was prefixed with `w` because it's word-sized (and `dw` for 
> double word size)?
> 
> FWIW: 
> https://docs.microsoft.com/en-us/windows/win32/stg/coding-style-conventions
It is a good question.

Microsoft redefines them, so I would like to add them as part of mapping table, 
`HungarianNotationTable`. That means the map include primitive types and 
partial windows data types. 

```
// Windows Data Type
{"BOOL","b"},   // typedef int BOOL;
{"BOOLEAN", "b"},   // typedef BYTE BOOLEAN;
{"BYTE","by"},  // typedef unsigned char BYTE;
{"WORD","w"},   // typedef unsigned short WORD;
{"DWORD",   "dw"},  // typedef unsigned long DWORD;
```

The result will like the following,
```
WORD wVid = 0x8086;
DWORD dwVidDid = 0x80861234;
```



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:211
+{"unsigned","u"},
+{"long","l"}};
+  // clang-format on

aaron.ballman wrote:
> How about: `long long`, `long double`, `ptrdiff_t`, and `void` (for `void *` 
> parameters)?
OK~ I will add them. Thank you. The `void*` and `ptrdiff_t` starts with `p`.







Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:222
+  // clang-format off
+  const static llvm::StringMap NullString = {
+{"char*", "sz"},

aaron.ballman wrote:
> Are there special prefixes for `char8_t *`, `char16_t *`, or `char32_t *`?
I will add them too. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:288
+  }
+
+  return PrefixStr;

aaron.ballman wrote:
> How about function pointers?
Nice consideration, thank you. I will add the feature. function pointers will 
start with `fn`.

I will add a test like this,
```
typedef void (*FUNC_PTR_HELLO)();
FUNC_PTR_HELLO Hello = NULL;
// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: invalid case style for variable 
'Hello' [readability-identifier-naming]
// CHECK-FIXES: {{^}}FUNC_PTR_HELLO fnHello = NULL;
```



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:310
+// Qualifier
+"const", "volatile",
+// Storage class specifiers

aaron.ballman wrote:
> `restrict`?
I will check it again. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:314
+// Constexpr specifiers
+"constexpr", "constinit", "const_cast", "consteval"};
+

aaron.ballman wrote:
> `const_cast` is not a constexpr specifier?
I will check it again. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:384
+static std::string fixupWithCase(const StringRef , const StringRef ,
+ const Decl *pDecl,
  IdentifierNamingCheck::CaseType Case) {

aaron.ballman wrote:
> `pDecl` doesn't match our usual conventions. ;-)
Oops! I will fix it later.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:478
+  case IdentifierNamingCheck::CT_HungarianNotation: {
+const NamedDecl *pNamedDecl = dyn_cast(pDecl);
+const std::string TypePrefix =

aaron.ballman wrote:
> Same here.
I will fix too.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:479
+const NamedDecl *pNamedDecl = dyn_cast(pDecl);
+const std::string TypePrefix =
+getHungarianNotationTypePrefix(Type.str(), pNamedDecl);

aaron.ballman wrote:
> `pNamedDecl` can be null, which could crash.
Make sense. If it was you, will you check it at the caller or in the callee? 
and could 

[PATCH] D86930: [clang-format] Handle typename macros inside cast expressions

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added a comment.

I am a beginner to compiler, interesting in how to write Unit Test case for 
change so I ran it, but found difference with my expection.

You mentioned 
Before: x = (STACK_OF(uint64_t)) & a;
After: x = (STACK_OF(uint64_t))

Your input test data is identical with the expected data. Does this exactly you 
need? Attach with a screenshot for your reference.
F12833965: 2020-09-02_081105.png 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86930

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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 288938.
dougpuob added a comment.

Improved suggestions of code review.

1. Moved release notes to right place. [Eugene.Zelenko]
2. Added new casting types to doc(readability-identifier-naming.rst) 
[Eugene.Zelenko]
3. Moved partial code to a new function, 
IdentifierNamingCheck::getDeclTypeName(). [njames93]


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,120 @@
+#include 
+#include 
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: readability-identifier-naming.VariableCase, value: szHungarianNotation}, \
+// RUN:   ]}"
+
+class UnlistedClass {};
+UnlistedClass Unlisted1;
+// CHECK-NOT: :[[@LINE-2]]
+
+UnlistedClass cUnlisted2;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for variable 'cUnlisted2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass Unlisted2;
+
+UnlistedClass objUnlistedClass3;
+// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: invalid case style for variable 'objUnlistedClass3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}UnlistedClass UnlistedClass3;
+
+typedef int INDEX;
+INDEX iIndex = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'iIndex' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}INDEX Index = 0;
+
+const char *NamePtr = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for variable 'NamePtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char *szNamePtr = "Name";
+
+const char NameArray[] = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for variable 'NameArray' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char szNameArray[] = "Name";
+
+void *BufferPtr1 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'BufferPtr1' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pBufferPtr1 = NULL;
+
+void **BufferPtr2 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'BufferPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr2 = NULL;
+
+void **pBufferPtr3 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'pBufferPtr3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr3 = NULL;
+
+int8_t ValueI8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueI8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int8_t i8ValueI8 = 0;
+
+int16_t ValueI16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int16_t i16ValueI16 = 0;
+
+int32_t ValueI32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int32_t i32ValueI32 = 0;
+
+int64_t ValueI64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int64_t i64ValueI64 = 0;
+
+uint8_t ValueU8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueU8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint8_t u8ValueU8 = 0;
+
+uint16_t ValueU16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint16_t u16ValueU16 = 0;
+
+uint32_t ValueU32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint32_t u32ValueU32 = 0;
+
+uint64_t ValueU64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint64_t u64ValueU64 = 0;
+
+float ValueFloat = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'ValueFloat' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}float fValueFloat = 0;
+
+double ValueDouble = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueDouble' [readability-identifier-naming]
+// 

[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.

In D86671#2246570 , @njames93 wrote:

> As Hungarian notation enforces prefixes as well as casing styles it would be 
> wise to warn on cases where the style is Hungarian but a prefix has also been 
> set.
>
> Another issue is this current set up will let you define any decls style as 
> hungarian, this doesn't make sense for most decl types. 
> Potentially some validation should happen for that too,

Hi @njames93:
If decl types were not in the `HungarianNotationTable` table, current 
implementation will treat it as `CamelCase`(UpperCamel), because the prefix is 
empty.




Comment at: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h:127
   virtual llvm::Optional
-  GetDeclFailureInfo(const NamedDecl *Decl, const SourceManager ) const = 0;
+  GetDeclFailureInfo(const StringRef , const NamedDecl *Decl,
+ const SourceManager ) const = 0;

njames93 wrote:
> I'm not a fan of changing the base class for this support. The Type paramater 
> can be inferred using the Decl. 
> You can `dyn_cast` the Decl to a `ValueDecl` and then get its type from that.
Agree with you. This change is possible to make it without changing the 
interface of function. Improve it in next patch. Thank you.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:73
 
+- Added: Support variables could be checked with Hungarian Notation which the
+  prefix encodes the actual data type of the variable.

Eugene.Zelenko wrote:
> dougpuob wrote:
> > Eugene.Zelenko wrote:
> > > See examples for entry format below.
> > Make it like the following ?
> > 
> > ```
> > Changes in existing checks
> > ^^
> > 
> > - Improved :doc:`readability-identifier-naming
> >   ` check.
> > 
> >   Added an option `GetConfigPerFile` to support including files which use
> >   different naming styles.
> > 
> > - Improved :doc:`readability-identifier-naming
> >   
> > ` 
> > check.  
> > 
> >   Support variables could be checked with Hungarian Notation which the 
> > prefix
> >   encodes the actual data type of the variable.
> > ```
> Yes, but link must be `clang-tidy/checks/readability-identifier-naming`, 
> because it refer to documentation file, not regression test.
 OK~ Fix it in the next patch. Thank you.


Repository:
  rCTE Clang Tools Extra

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

[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.

In D86671#2246480 , @Eugene.Zelenko 
wrote:

> By the word, you must mention new option in documentation too.

Hi @Eugene.Zelenko,
Is the `clang-tools-extra/docs/ReleaseNotes.rst` file that you mentioned?

If YES, seems pretty simple with one line at the end for the new casing type, 
like:

  Casing types include:
  
   - ``lower_case``,
   - ``UPPER_CASE``,
   - ``camelBack``,
   - ``CamelCase``,
   - ``camel_Snake_Back``,
   - ``Camel_Snake_Case``,
   - ``aNy_CasE``,
   - ``szHungarianNotation``. 

Is it right? and any more about document I have to do?


Repository:
  rCTE Clang Tools Extra

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

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

[Douglas Chen via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:73
 
+- Added: Support variables could be checked with Hungarian Notation which the
+  prefix encodes the actual data type of the variable.

Eugene.Zelenko wrote:
> See examples for entry format below.
Make it like the following ?

```
Changes in existing checks
^^

- Improved :doc:`readability-identifier-naming
  ` check.

  Added an option `GetConfigPerFile` to support including files which use
  different naming styles.

- Improved :doc:`readability-identifier-naming
  ` 
check.  

  Support variables could be checked with Hungarian Notation which the prefix
  encodes the actual data type of the variable.
```


Repository:
  rCTE Clang Tools Extra

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

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

[Douglas Chen via Phabricator via cfe-commits]

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

I fixed all review suggestions from @aaron.ballman, @Eugene.Zelenko, and 
@njames93, Thank you. But I still can't sure I did everything correct.

I encountered a problem when I use `arc diff` or `arc diff --update D86671`, 
Arcanist will create a new Review(D86838 ) to 
Phabricator. This makes me to update the diff on the web interface.

Another behavior I can't sure do I did it right. I will pull the latest master 
then create a new local branch(my-new-branch) then manually merge the diff from 
previous branch(my-prev-branch). After this I use `git diff HEAD~1 -U99 > 
mypatch.patch`, then update the diff, `mypatch.patch` file to Phabricator. Is 
it right? Or is there a smarter way to do it?


Repository:
  rCTE Clang Tools Extra

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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 288812.
dougpuob added a comment.
Herald added a subscriber: danielkiss.

Improved for suggestions of code review.

1. Moved release notes to `Changes in existing checks` section. Eugene.Zelenko]
2. Modified release notes can be describe user-facing. [Eugene.Zelenko]
3. Added newline at the end to 
`readability-identifier-naming-hungarain-notion.cpp` file. [Eugene.Zelenko]
4. Removed unrelated change. [Nathan James]
5. Renamed getDeclFailureInfo() --> GetDeclFailureInfo() for local consistency. 
[Aaron Ballman]


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,103 @@
+#include 
+#include 
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: readability-identifier-naming.VariableCase, value: szHungarianNotation}, \
+// RUN:   ]}"
+
+const char *NamePtr = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for variable 'NamePtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char *szNamePtr = "Name";
+
+const char NameArray[] = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for variable 'NameArray' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char szNameArray[] = "Name";
+
+void *BufferPtr1 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'BufferPtr1' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pBufferPtr1 = NULL;
+
+void **BufferPtr2 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'BufferPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr2 = NULL;
+
+void **pBufferPtr3 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'pBufferPtr3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr3 = NULL;
+
+int8_t ValueI8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueI8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int8_t i8ValueI8 = 0;
+
+int16_t ValueI16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int16_t i16ValueI16 = 0;
+
+int32_t ValueI32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int32_t i32ValueI32 = 0;
+
+int64_t ValueI64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int64_t i64ValueI64 = 0;
+
+uint8_t ValueU8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueU8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint8_t u8ValueU8 = 0;
+
+uint16_t ValueU16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint16_t u16ValueU16 = 0;
+
+uint32_t ValueU32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint32_t u32ValueU32 = 0;
+
+uint64_t ValueU64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint64_t u64ValueU64 = 0;
+
+float ValueFloat = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'ValueFloat' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}float fValueFloat = 0;
+
+double ValueDouble = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueDouble' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}double dValueDouble = 0;
+
+char ValueChar = 'c';
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueChar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}char cValueChar = 'c';
+
+bool ValueBool = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid 

[PATCH] D86838: Improved for suggestions of code review. 1. Moved release notes to `Changes in existing checks` section. [Eugene.Zelenko] 2. Modified release notes can be describe user-facing. [Eu

[Douglas Chen via Phabricator via cfe-commits]

dougpuob created this revision.
Herald added subscribers: cfe-commits, danielkiss.
Herald added a project: clang.
dougpuob requested review of this revision.

...-identifier-naming-hungarain-notion.cpp` file. [Eugene.Zelenko]   4. Removed 
unrelated change. [Nathan James]   5. Renamed getDeclFailureInfo() --> 
GetDeclFailureInfo() for local consistency. [Aaron Ballman]


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86838

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,103 @@
+#include 
+#include 
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: readability-identifier-naming.VariableCase, value: szHungarianNotation}, \
+// RUN:   ]}"
+
+const char *NamePtr = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for variable 'NamePtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char *szNamePtr = "Name";
+
+const char NameArray[] = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for variable 'NameArray' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char szNameArray[] = "Name";
+
+void *BufferPtr1 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'BufferPtr1' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pBufferPtr1 = NULL;
+
+void **BufferPtr2 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'BufferPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr2 = NULL;
+
+void **pBufferPtr3 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'pBufferPtr3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr3 = NULL;
+
+int8_t ValueI8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueI8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int8_t i8ValueI8 = 0;
+
+int16_t ValueI16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int16_t i16ValueI16 = 0;
+
+int32_t ValueI32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int32_t i32ValueI32 = 0;
+
+int64_t ValueI64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int64_t i64ValueI64 = 0;
+
+uint8_t ValueU8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueU8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint8_t u8ValueU8 = 0;
+
+uint16_t ValueU16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint16_t u16ValueU16 = 0;
+
+uint32_t ValueU32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint32_t u32ValueU32 = 0;
+
+uint64_t ValueU64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint64_t u64ValueU64 = 0;
+
+float ValueFloat = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'ValueFloat' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}float fValueFloat = 0;
+
+double ValueDouble = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueDouble' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}double dValueDouble = 0;
+
+char ValueChar = 'c';
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueChar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}char cValueChar = 'c';
+
+bool ValueBool = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueBool' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}bool bValueBool = true;
+
+int ValueInt = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for variable 'ValueInt' 

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:44
   llvm::Optional
-  GetDeclFailureInfo(const StringRef , const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef , const NamedDecl *Decl,
  const SourceManager ) const override;

dougpuob wrote:
> aaron.ballman wrote:
> > Given that the other functions in the class use the wrong style of casing, 
> > we should probably leave this declaration alone so it doesn't become 
> > locally inconsistent.
> Do you mean create another function with three parameters for it?
I think I got it. Keep consistent even it is wrong style of casting. 


Repository:
  rCTE Clang Tools Extra

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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob marked 2 inline comments as not done.
dougpuob added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h:44
   llvm::Optional
-  GetDeclFailureInfo(const StringRef , const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef , const NamedDecl *Decl,
  const SourceManager ) const override;

aaron.ballman wrote:
> Given that the other functions in the class use the wrong style of casing, we 
> should probably leave this declaration alone so it doesn't become locally 
> inconsistent.
Do you mean create another function with three parameters for it?



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:428
 
   case IdentifierNamingCheck::CT_HungarianNotation: {
 const NamedDecl *pNamedDecl = dyn_cast(pDecl);

njames93 wrote:
> aaron.ballman wrote:
> > I feel like I must be missing something because I don't have this enum in 
> > my copy of ToT despite pulling this morning. Is the patch missing some 
> > content?
> I feel like this is an incremental diff based on the first version of this 
> patch rather than a diff from trunk. 
@aaron.ballman and @njames93, the diff is based on first version not from 
master(trunk).

Is it the best way that I should merge with the latest master(trunk) every time 
before updating the diffs ?


Repository:
  rCTE Clang Tools Extra

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

[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.

In D86671#2243788 , @Eugene.Zelenko 
wrote:

> It'll be good idea to add test case.

Hi @Eugene.Zelenko,
I have created a `readability-identifier-naming-hungarain-notion.cpp`  file and 
several test cases for regression testing. Is it regression testing?

Find it here (https://reviews.llvm.org/differential/changeset/?ref=2137882)


Repository:
  rCTE Clang Tools Extra

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

[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.

In D86671#2243980 , @njames93 wrote:

> Can you make sure you upload diffs with full context (-U9). Or using 
> arcanist it will be done automatically.
>
> Make sure the diff is up to date with trunk
>
> Remove any changes that aren't related to this patch, they just make this 
> look noisy.

Hi @njames93,

1. It is my first time to use Phabricator and Arcanist, I will check how to 
make full context (-U9).
2. Do you mean that always sync with the latest master?
3. OK.




Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:317
+  if (Style.Case == IdentifierNamingCheck::CaseType::CT_HungarianNotation) {
+const auto TypePrefix = getHungarianNotationTypePrefix(Type.str(), Decl);
+if (TypePrefix.length() > 0) {

Eugene.Zelenko wrote:
> Please don't use auto when type is not specified explicitly in same statement 
> or iterator.
Got it, I fixed it in the next commit. Thank you.



Comment at: 
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp:430
+const NamedDecl *pNamedDecl = dyn_cast(pDecl);
+const auto TypePrefix =
+getHungarianNotationTypePrefix(Type.str(), pNamedDecl);

Eugene.Zelenko wrote:
> Please don't use auto when type is not specified explicitly in same statement 
> or iterator.
Got it, I fixed it in the next commit. Thank you.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:70
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting 
naming check with Hungarian notation.
+

Eugene.Zelenko wrote:
> Please move to `Changes in existing checks` section (in alphabetical order).
> 
> I think statement should describe user-facing changes.
OK~ Fix it in next commit.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:76
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   ` check.

Eugene.Zelenko wrote:
> Unrelated change.
OK~ Fix it on next commit.


Repository:
  rCTE Clang Tools Extra

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

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 288520.
dougpuob added a comment.
Herald added a subscriber: mgehre.

Improved suggestions from code review and clang-tidy.

1. Add keyword `const` to variables which checked via clang-tidy.
2. Add log to clang-tools-extra/docs/ReleaseNotes.rst. [Suggestion from 
Eugene.Zelenko]
3. Don't use `auto` with variables are not specified explicitly. [Suggestion 
from Eugene.Zelenko]


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,10 +67,13 @@
 Improvements to clang-tidy
 --
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting naming check with Hungarian notation.
+
+
 Changes in existing checks
 ^^
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   ` check.
 
   Added an option `GetConfigPerFile` to support including files which use
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -32,7 +32,7 @@
 
   /// Derived classes should not implement any matching logic themselves; this
   /// class will do the matching and call the derived class'
-  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// getDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
   /// given identifier passes or fails the check.
   void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
   void
@@ -124,7 +124,7 @@
   /// Overridden by derived classes, returns information about if and how a Decl
   /// failed the check. A 'None' result means the Decl did not fail the check.
   virtual llvm::Optional
-  GetDeclFailureInfo(const StringRef , const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef , const NamedDecl *Decl,
  const SourceManager ) const = 0;
 
   /// Overridden by derived classes, returns information about if and how a
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -400,11 +400,11 @@
 
   // Get type text of variable declarations.
   const auto  = Decl->getASTContext().getSourceManager();
-  const char *szBegin = SrcMgr.getCharacterData(Decl->getBeginLoc());
-  const char *szCurr = SrcMgr.getCharacterData(Decl->getLocation());
-  const intptr_t iPtrLen = szCurr - szBegin;
-  if (iPtrLen > 0) {
-std::string Type(szBegin, iPtrLen);
+  const char *Begin = SrcMgr.getCharacterData(Decl->getBeginLoc());
+  const char *Curr = SrcMgr.getCharacterData(Decl->getLocation());
+  const intptr_t StrLen = Curr - Begin;
+  if (StrLen > 0) {
+std::string Type(Begin, StrLen);
 
 const static std::list Keywords = {
 // Qualifier
@@ -415,23 +415,23 @@
 "constexpr", "constinit", "const_cast", "consteval"};
 
 // Remove keywords
-for (const auto  : Keywords) {
-  for (size_t pos = 0;
-   (pos = Type.find(kw, pos)) != std::string::npos;) {
-Type.replace(pos, kw.length(), "");
+for (const auto  : Keywords) {
+  for (size_t Pos = 0;
+   (Pos = Type.find(Kw, Pos)) != std::string::npos;) {
+Type.replace(Pos, Kw.length(), "");
   }
 }
 
 // Replace spaces with single space
-for (size_t pos = 0; (pos = Type.find("  ", pos)) != std::string::npos;
- pos += strlen(" ")) {
-  Type.replace(pos, strlen("  "), " ");
+for (size_t Pos = 0; (Pos = Type.find("  ", Pos)) != std::string::npos;
+ Pos += strlen(" ")) {
+  Type.replace(Pos, strlen("  "), " ");
 }
 
 // Replace " *" with "*"
-for (size_t pos = 0; (pos = Type.find(" *", pos)) != std::string::npos;
- pos += strlen("*")) {
-  Type.replace(pos, strlen(" *"), "*");
+for (size_t Pos = 0; (Pos = Type.find(" 

[PATCH] D86757: Improved suggestions from code review and clang-tidy. 1) Add keyword `const` to variables which checked via clang-tidy. 2) Add log to clang-tools-extra/docs/ReleaseNotes.rst. [Euge

[Douglas Chen via Phabricator via cfe-commits]

dougpuob created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
dougpuob requested review of this revision.

...ot. [Eugene.Zelenko]

...specified explicitly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86757

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst

Index: clang-tools-extra/docs/ReleaseNotes.rst
===
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -67,10 +67,13 @@
 Improvements to clang-tidy
 --
 
+* Added: Add IdentifierNamingCheck::CaseType, CT_HungarianNotation, supporting naming check with Hungarian notation.
+
+
 Changes in existing checks
 ^^
 
-- Improved :doc:`readability-identifier-naming
+- Improved: doc:`readability-identifier-naming
   ` check.
 
   Added an option `GetConfigPerFile` to support including files which use
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
@@ -32,7 +32,7 @@
 
   /// Derived classes should not implement any matching logic themselves; this
   /// class will do the matching and call the derived class'
-  /// GetDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
+  /// getDeclFailureInfo() and GetMacroFailureInfo() for determining whether a
   /// given identifier passes or fails the check.
   void registerMatchers(ast_matchers::MatchFinder *Finder) override final;
   void
@@ -124,7 +124,7 @@
   /// Overridden by derived classes, returns information about if and how a Decl
   /// failed the check. A 'None' result means the Decl did not fail the check.
   virtual llvm::Optional
-  GetDeclFailureInfo(const StringRef , const NamedDecl *Decl,
+  getDeclFailureInfo(const StringRef , const NamedDecl *Decl,
  const SourceManager ) const = 0;
 
   /// Overridden by derived classes, returns information about if and how a
Index: clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
===
--- clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
+++ clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
@@ -400,11 +400,11 @@
 
   // Get type text of variable declarations.
   const auto  = Decl->getASTContext().getSourceManager();
-  const char *szBegin = SrcMgr.getCharacterData(Decl->getBeginLoc());
-  const char *szCurr = SrcMgr.getCharacterData(Decl->getLocation());
-  const intptr_t iPtrLen = szCurr - szBegin;
-  if (iPtrLen > 0) {
-std::string Type(szBegin, iPtrLen);
+  const char *Begin = SrcMgr.getCharacterData(Decl->getBeginLoc());
+  const char *Curr = SrcMgr.getCharacterData(Decl->getLocation());
+  const intptr_t StrLen = Curr - Begin;
+  if (StrLen > 0) {
+std::string Type(Begin, StrLen);
 
 const static std::list Keywords = {
 // Qualifier
@@ -415,23 +415,23 @@
 "constexpr", "constinit", "const_cast", "consteval"};
 
 // Remove keywords
-for (const auto  : Keywords) {
-  for (size_t pos = 0;
-   (pos = Type.find(kw, pos)) != std::string::npos;) {
-Type.replace(pos, kw.length(), "");
+for (const auto  : Keywords) {
+  for (size_t Pos = 0;
+   (Pos = Type.find(Kw, Pos)) != std::string::npos;) {
+Type.replace(Pos, Kw.length(), "");
   }
 }
 
 // Replace spaces with single space
-for (size_t pos = 0; (pos = Type.find("  ", pos)) != std::string::npos;
- pos += strlen(" ")) {
-  Type.replace(pos, strlen("  "), " ");
+for (size_t Pos = 0; (Pos = Type.find("  ", Pos)) != std::string::npos;
+ Pos += strlen(" ")) {
+  Type.replace(Pos, strlen("  "), " ");
 }
 
 // Replace " *" with "*"
-for (size_t pos = 0; (pos = Type.find(" *", pos)) != std::string::npos;
- pos += strlen("*")) {
-  Type.replace(pos, strlen(" *"), "*");
+for (size_t Pos = 0; (Pos = Type.find(" *", Pos)) != std::string::npos;
+ Pos += strlen("*")) {
+  Type.replace(Pos, strlen(" *"), "*");
 }
 
 Type = Type.erase(Type.find_last_not_of(" ") + 1);
@@ -460,7 +460,7 @@
   return;
 
 Optional MaybeFailure =
-

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

[Douglas Chen via Phabricator via cfe-commits]

dougpuob updated this revision to Diff 288302.
dougpuob added a comment.

Fixed typos and add new Case Type, szHungarianNotation in doc.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  clang-tools-extra/docs/clang-tidy/checks/readability-identifier-naming.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarian-notation.cpp
@@ -0,0 +1,103 @@
+#include 
+#include 
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: readability-identifier-naming.VariableCase, value: szHungarianNotation}, \
+// RUN:   ]}"
+
+const char *NamePtr = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for variable 'NamePtr' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char *szNamePtr = "Name";
+
+const char NameArray[] = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for variable 'NameArray' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char szNameArray[] = "Name";
+
+void *BufferPtr1 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'BufferPtr1' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pBufferPtr1 = NULL;
+
+void **BufferPtr2 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'BufferPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr2 = NULL;
+
+void **pBufferPtr3 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'pBufferPtr3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr3 = NULL;
+
+int8_t ValueI8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueI8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int8_t i8ValueI8 = 0;
+
+int16_t ValueI16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int16_t i16ValueI16 = 0;
+
+int32_t ValueI32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int32_t i32ValueI32 = 0;
+
+int64_t ValueI64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int64_t i64ValueI64 = 0;
+
+uint8_t ValueU8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueU8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint8_t u8ValueU8 = 0;
+
+uint16_t ValueU16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint16_t u16ValueU16 = 0;
+
+uint32_t ValueU32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint32_t u32ValueU32 = 0;
+
+uint64_t ValueU64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint64_t u64ValueU64 = 0;
+
+float ValueFloat = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'ValueFloat' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}float fValueFloat = 0;
+
+double ValueDouble = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueDouble' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}double dValueDouble = 0;
+
+char ValueChar = 'c';
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueChar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}char cValueChar = 'c';
+
+bool ValueBool = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueBool' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}bool bValueBool = true;
+
+int ValueInt = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for variable 'ValueInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int iValueInt = 0;
+
+size_t ValueSize = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style 

[PATCH] D86671: Add new IdentifierNamingCheck::CaseType, CT_HungarianNotion, supporting naming check with Hungarian Notion.

[Douglas Chen via Phabricator via cfe-commits]

dougpuob created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.
dougpuob requested review of this revision.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D86671

Files:
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp
  clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
  
clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarain-notion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarain-notion.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-identifier-naming-hungarain-notion.cpp
@@ -0,0 +1,107 @@
+#include 
+#include 
+
+// RUN: %check_clang_tidy %s readability-identifier-naming %t -- \
+// RUN:   -config="{CheckOptions: [\
+// RUN:   {key: readability-identifier-naming.VariableCase, value: szHungarianNotion}, \
+// RUN:   ]}"
+
+const char* NamePtr1 = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for variable 'NamePtr1' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char* szNamePtr1 = "Name";
+
+const char *NamePtr2 = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: invalid case style for variable 'NamePtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char *szNamePtr2 = "Name";
+
+const char NameArray[] = "Name";
+// CHECK-MESSAGES: :[[@LINE-1]]:12: warning: invalid case style for variable 'NameArray' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}const char szNameArray[] = "Name";
+
+void *BufferPtr1 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'BufferPtr1' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void *pBufferPtr1 = NULL;
+
+void **BufferPtr2 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'BufferPtr2' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr2 = NULL;
+
+void **pBufferPtr3 = NULL;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'pBufferPtr3' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}void **ppBufferPtr3 = NULL;
+
+int8_t ValueI8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueI8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int8_t i8ValueI8 = 0;
+
+int16_t ValueI16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int16_t i16ValueI16 = 0;
+
+int32_t ValueI32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int32_t i32ValueI32 = 0;
+
+int64_t ValueI64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueI64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int64_t i64ValueI64 = 0;
+
+uint8_t ValueU8 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:9: warning: invalid case style for variable 'ValueU8' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint8_t u8ValueU8 = 0;
+
+uint16_t ValueU16 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU16' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint16_t u16ValueU16 = 0;
+
+uint32_t ValueU32 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU32' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint32_t u32ValueU32 = 0;
+
+uint64_t ValueU64 = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:10: warning: invalid case style for variable 'ValueU64' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}uint64_t u64ValueU64 = 0;
+
+float ValueFloat = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style for variable 'ValueFloat' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}float fValueFloat = 0;
+
+double ValueDouble = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:8: warning: invalid case style for variable 'ValueDouble' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}double dValueDouble = 0;
+
+char ValueChar = 'c';
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueChar' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}char cValueChar = 'c';
+
+bool ValueBool = true;
+// CHECK-MESSAGES: :[[@LINE-1]]:6: warning: invalid case style for variable 'ValueBool' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}bool bValueBool = true;
+
+int ValueInt = 0;
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: invalid case style for variable 'ValueInt' [readability-identifier-naming]
+// CHECK-FIXES: {{^}}int iValueInt = 0;
+
+size_t