Skip to content

Conversation

@PiotrZSL
Copy link
Member

@PiotrZSL PiotrZSL commented Sep 19, 2023

Typedef rename were not properly handled when typedef were used behind pointer, or as a part of function type. Additionally because entire function were passed as an source-range, when function started with macro, such change were not marked for a fix.

Removed workaround and used proper TypedefTypeLoc instead.

Fixes #55156, #54699

Typedef rename were not properly handled when typedef were used behind pointer, or as a part of function type. Additionally because entire function were passed as an source-range, when function started with macro, such change were not marked for a fix. Removed workaround and used proper TypedefTypeLoc instead.
@llvmbot
Copy link
Member

llvmbot commented Sep 19, 2023

@llvm/pr-subscribers-clang-tidy

Changes

Typedef rename were not properly handled when typedef were used behind pointer, or as a part of function type. Additionally because entire function were passed as an source-range, when function started with macro, such change were not marked for a fix.

Removed workaround and used proper TypedefTypeLoc instead.

Fixes #55156


Full diff: https://github.com/llvm/llvm-project/pull/66835.diff

3 Files Affected:

  • (modified) clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp (+5-20)
  • (modified) clang-tools-extra/docs/ReleaseNotes.rst (+3-1)
  • (modified) clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp (+18)
diff --git a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp index d24b7a65b1c4334..da1433aa2d05d47 100644 --- a/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp +++ b/clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.cpp @@ -256,26 +256,6 @@ class RenamerClangTidyVisitor return true; } - // Fix type aliases in value declarations. - if (const auto *Value = dyn_cast<ValueDecl>(Decl)) { - if (const Type *TypePtr = Value->getType().getTypePtrOrNull()) { - if (const auto *Typedef = TypePtr->getAs<TypedefType>()) - Check->addUsage(Typedef->getDecl(), Value->getSourceRange(), SM); - } - } - - // Fix type aliases in function declarations. - if (const auto *Value = dyn_cast<FunctionDecl>(Decl)) { - if (const auto *Typedef = - Value->getReturnType().getTypePtr()->getAs<TypedefType>()) - Check->addUsage(Typedef->getDecl(), Value->getSourceRange(), SM); - for (const ParmVarDecl *Param : Value->parameters()) { - if (const TypedefType *Typedef = - Param->getType().getTypePtr()->getAs<TypedefType>()) - Check->addUsage(Typedef->getDecl(), Value->getSourceRange(), SM); - } - } - // Fix overridden methods if (const auto *Method = dyn_cast<CXXMethodDecl>(Decl)) { if (const CXXMethodDecl *Overridden = getOverrideMethod(Method)) { @@ -340,6 +320,11 @@ class RenamerClangTidyVisitor return true; } + bool VisitTypedefTypeLoc(const TypedefTypeLoc &Loc) { + Check->addUsage(Loc.getTypedefNameDecl(), Loc.getSourceRange(), SM); + return true; + } + bool VisitTagTypeLoc(const TagTypeLoc &Loc) { Check->addUsage(Loc.getDecl(), Loc.getSourceRange(), SM); return true; diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index 6d6f51998a01e57..b8977a39239ffce 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -291,7 +291,9 @@ Changes in existing checks warnings when a type's forward declaration precedes its definition. Additionally, it now provides appropriate warnings for ``struct`` and ``union`` in C, while also incorporating support for the - ``Leading_upper_snake_case`` naming convention. + ``Leading_upper_snake_case`` naming convention. The handling of ``typedef`` + has been enhanced, particularly within complex types like function pointers + and cases where style checks were omitted when functions started with macros. - Improved :doc:`readability-implicit-bool-conversion <clang-tidy/checks/readability/implicit-bool-conversion>` check to take diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp index 3445a21bfdbc4aa..0f5fc1145f8791e 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/readability/identifier-naming.cpp @@ -729,3 +729,21 @@ struct forward_declared_as_struct; class forward_declared_as_struct { }; +namespace pr55156 { + +typedef enum { + VALUE0, + VALUE1, +} ValueType; +// CHECK-MESSAGES: :[[@LINE-1]]:3: warning: invalid case style for typedef 'ValueType' [readability-identifier-naming] +// CHECK-FIXES: {{^}}} value_type_t; + +#define STATIC_MACRO static +STATIC_MACRO void someFunc(ValueType a_v1, const ValueType& a_v2) {} +// CHECK-FIXES: {{^}}STATIC_MACRO void someFunc(value_type_t a_v1, const value_type_t& a_v2) {} +STATIC_MACRO void someFunc(const ValueType** p_a_v1, ValueType (*p_a_v2)()) {} +// CHECK-FIXES: {{^}}STATIC_MACRO void someFunc(const value_type_t** p_a_v1, value_type_t (*p_a_v2)()) {} +STATIC_MACRO ValueType someFunc() {} +// CHECK-FIXES: {{^}}STATIC_MACRO value_type_t someFunc() {} +#undef STATIC_MACRO +} 
Copy link
Contributor

@HerrCai0907 HerrCai0907 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Contributor

@carlosgalvezp carlosgalvezp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks for the fix and the great cleanup!

@PiotrZSL PiotrZSL merged commit 5d95d27 into llvm:main Sep 20, 2023
@PiotrZSL PiotrZSL deleted the 55156-clang-tidy-fails-to-detect-readability-identifier-namingtypedefprefix-check-in-case-of-macro-usage branch September 20, 2023 19:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

4 participants