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

2023-04-14 Thread 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/

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

2023-04-14 Thread 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

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

2023-04-14 Thread 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:

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

2023-04-14 Thread 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

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

2023-04-14 Thread 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:

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

2023-04-14 Thread 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] D147779: [clang-tidy] Fix hungarian notation failed to indicate the number of asterisks in check-clang-extra-clang-tidy-checkers-readability

2023-04-11 Thread 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

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

2023-04-10 Thread 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`?

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

2023-04-10 Thread 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 >>

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

2023-04-10 Thread 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

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

2023-04-09 Thread 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:

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

2023-04-09 Thread 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

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

2023-04-09 Thread 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:

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

2023-04-07 Thread 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

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

2021-08-03 Thread 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/

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

2021-08-03 Thread 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 ___

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

2021-08-03 Thread 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

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

2021-08-03 Thread 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:

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

2021-08-03 Thread 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 >

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

2021-08-03 Thread 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

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

2021-08-03 Thread 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?

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

2021-08-03 Thread 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

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

2021-08-03 Thread 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

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

2021-08-02 Thread 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

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

2021-06-25 Thread 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

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

2021-06-10 Thread 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

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

2021-05-13 Thread 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

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

2021-05-11 Thread 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

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

2021-05-08 Thread 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 {

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

2021-05-02 Thread 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

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

2021-05-02 Thread 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()) +

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

2021-01-13 Thread 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

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

2020-12-30 Thread 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:

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

2020-12-22 Thread 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

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

2020-12-11 Thread 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

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

2020-12-06 Thread 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().

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

2020-11-24 Thread 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/

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

2020-11-22 Thread 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

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

2020-11-18 Thread 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

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

2020-11-04 Thread 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

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

2020-11-01 Thread 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

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

2020-10-30 Thread 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),

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

2020-10-23 Thread 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

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

2020-10-20 Thread 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

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

2020-10-20 Thread 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

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

2020-10-13 Thread 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

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

2020-10-11 Thread 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

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

2020-10-10 Thread 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:

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

2020-09-30 Thread 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

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

2020-09-30 Thread 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 >>

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

2020-09-20 Thread 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

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

2020-09-19 Thread 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"},

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

2020-09-13 Thread 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

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

2020-09-11 Thread 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

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

2020-09-09 Thread 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:

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

2020-09-08 Thread 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:

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

2020-09-07 Thread 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

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

2020-09-05 Thread 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

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

2020-09-02 Thread 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) {

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

2020-09-01 Thread 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

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

2020-08-31 Thread 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,

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

2020-08-30 Thread 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

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

2020-08-29 Thread 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

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

2020-08-29 Thread 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.

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

2020-08-29 Thread 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`,

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

2020-08-29 Thread 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.

[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

2020-08-29 Thread 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

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

2020-08-28 Thread 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,

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

2020-08-28 Thread 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 ,

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

2020-08-28 Thread 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.

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

2020-08-28 Thread 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

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

2020-08-27 Thread 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

[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

2020-08-27 Thread 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:

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

2020-08-27 Thread 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:

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

2020-08-26 Thread 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