[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
cfe-commits at lists.llvm.org
Sun Apr 9 02:20:33 PDT 2023
dougpuob marked 2 inline comments as done.
dougpuob added a comment.
In D147779#4251081 <https://reviews.llvm.org/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<ValueDecl>(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 &TypeName) 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<unsigned char**>, will this function also be executed ?
Yes, this function is also executed in this case, but the `std::optional<unsigned char**>` 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<unsigned char**>` --> `std::optional<unsigned char` (weird result)
- NEW: `std::optional<unsigned char**>` --> `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
More information about the cfe-commits
mailing list