llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT-->
@llvm/pr-subscribers-clang-tools-extra
Author: Jakub Kuderski (kuhar)
<details>
<summary>Changes</summary>
Adds a new check for `llvm::TypeSwitch`:
* `.Case<T>([](T *X) { ... })` --> `.Case([](T *X) { ... } )`
* `.Case<T>([](auto X) { ... })` --> warning only
I ran this on mlir to make sure the warning make sense and that all fixits
compile: https://gist.github.com/kuhar/a2276a2364325521ea6f39d2098497b9.
---
Full diff: https://github.com/llvm/llvm-project/pull/177892.diff
8 Files Affected:
- (modified) clang-tools-extra/clang-tidy/llvm/CMakeLists.txt (+1)
- (modified) clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp (+3)
- (added) clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.cpp (+104)
- (added) clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.h (+36)
- (modified) clang-tools-extra/docs/ReleaseNotes.rst (+6)
- (modified) clang-tools-extra/docs/clang-tidy/checks/list.rst (+1)
- (added)
clang-tools-extra/docs/clang-tidy/checks/llvm/type-switch-case-types.rst (+37)
- (added)
clang-tools-extra/test/clang-tidy/checkers/llvm/type-switch-case-types.cpp
(+175)
``````````diff
diff --git a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
index 56bf4f31bd0e8..a807f0ab65f87 100644
--- a/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
+++ b/clang-tools-extra/clang-tidy/llvm/CMakeLists.txt
@@ -11,6 +11,7 @@ add_clang_library(clangTidyLLVMModule STATIC
PreferRegisterOverUnsignedCheck.cpp
PreferStaticOverAnonymousNamespaceCheck.cpp
TwineLocalCheck.cpp
+ TypeSwitchCaseTypesCheck.cpp
UseNewMLIROpBuilderCheck.cpp
UseRangesCheck.cpp
UseVectorUtilsCheck.cpp
diff --git a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
index eb1ae820dabf8..c180574bdeed6 100644
--- a/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
+++ b/clang-tools-extra/clang-tidy/llvm/LLVMTidyModule.cpp
@@ -17,6 +17,7 @@
#include "PreferRegisterOverUnsignedCheck.h"
#include "PreferStaticOverAnonymousNamespaceCheck.h"
#include "TwineLocalCheck.h"
+#include "TypeSwitchCaseTypesCheck.h"
#include "UseNewMLIROpBuilderCheck.h"
#include "UseRangesCheck.h"
#include "UseVectorUtilsCheck.h"
@@ -43,6 +44,8 @@ class LLVMModule : public ClangTidyModule {
CheckFactories.registerCheck<readability::QualifiedAutoCheck>(
"llvm-qualified-auto");
CheckFactories.registerCheck<TwineLocalCheck>("llvm-twine-local");
+ CheckFactories.registerCheck<TypeSwitchCaseTypesCheck>(
+ "llvm-type-switch-case-types");
CheckFactories.registerCheck<UseNewMlirOpBuilderCheck>(
"llvm-use-new-mlir-op-builder");
CheckFactories.registerCheck<UseRangesCheck>("llvm-use-ranges");
diff --git a/clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.cpp
b/clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.cpp
new file mode 100644
index 0000000000000..e5baf164218e0
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.cpp
@@ -0,0 +1,104 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TypeSwitchCaseTypesCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::llvm_check {
+
+void TypeSwitchCaseTypesCheck::registerMatchers(MatchFinder *Finder) {
+ // Match calls to `llvm::TypeSwitch::Case` with a lambda expression.
+ // Explicit template arguments and their count are checked in `check()`.
+ Finder->addMatcher(
+ cxxMemberCallExpr(
+ argumentCountIs(1),
+ callee(memberExpr(member(cxxMethodDecl(hasName("Case"),
+ ofClass(cxxRecordDecl(hasName(
+ "::llvm::TypeSwitch"))))))
+ .bind("member")),
+ hasArgument(0, lambdaExpr().bind("lambda")))
+ .bind("call"),
+ this);
+}
+
+void TypeSwitchCaseTypesCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *Call = Result.Nodes.getNodeAs<CXXMemberCallExpr>("call");
+ assert(Call);
+ const auto *Lambda = Result.Nodes.getNodeAs<LambdaExpr>("lambda");
+ assert(Lambda);
+ const auto *MemExpr = Result.Nodes.getNodeAs<MemberExpr>("member");
+ assert(MemExpr);
+
+ // Only handle `Case<T>` with exactly one explicit template argument.
+ if (!MemExpr->hasExplicitTemplateArgs() || MemExpr->getNumTemplateArgs() !=
1)
+ return;
+
+ const TemplateArgumentLoc &TemplateArg = MemExpr->getTemplateArgs()[0];
+ if (TemplateArg.getArgument().getKind() != TemplateArgument::Type)
+ return;
+
+ QualType CaseType = TemplateArg.getArgument().getAsType();
+
+ // Get the lambda's call operator to examine its parameter.
+ const CXXMethodDecl *CallOp = Lambda->getCallOperator();
+ if (!CallOp || CallOp->getNumParams() != 1)
+ return;
+
+ const ParmVarDecl *LambdaParam = CallOp->getParamDecl(0);
+ QualType ParamType = LambdaParam->getType();
+
+ // Check if the parameter uses `auto`.
+ QualType ParamBaseType = ParamType.getNonReferenceType();
+ while (ParamBaseType->isPointerType())
+ ParamBaseType = ParamBaseType->getPointeeType();
+ const bool ParamIsAuto = ParamBaseType->getUnqualifiedDesugaredType()
+ ->getAs<TemplateTypeParmType>() != nullptr;
+
+ if (ParamIsAuto) {
+ // Warn about `.Case<T>([](auto x) {...})` -- prefer explicit lambda
+ // parameter type. We only emit a warning without a fixit because we cannot
+ // reliably determine the deduced type of `auto`. The actual type depends
on
+ // how `dyn_cast<CaseT>` behaves for the `TypeSwitch` value type, which
+ // varies (e.g., pointer types return pointers, but MLIR handle types may
+ // return by value).
+ diag(Call->getExprLoc(),
+ "lambda parameter needlessly uses 'auto', use explicit type instead");
+ diag(LambdaParam->getTypeSourceInfo()->getTypeLoc().getBeginLoc(),
+ "replace 'auto' with explicit type", DiagnosticIDs::Note);
+ diag(TemplateArg.getLocation(),
+ "type from template argument can be inferred and removed",
+ DiagnosticIDs::Note);
+ return;
+ }
+
+ // Handle `.Case<T>([](T x) {...})` -> `.Case([](T x) {...})`.
+ // Only warn if the types match (otherwise it might be intentional or a bug).
+ if (CaseType->getCanonicalTypeUnqualified() !=
+ ParamBaseType->getCanonicalTypeUnqualified())
+ return;
+
+ auto Diag = diag(Call->getExprLoc(), "redundant explicit template argument");
+
+ // Skip fixit if template argument involves macros.
+ const SourceLocation LAngleLoc = MemExpr->getLAngleLoc();
+ const SourceLocation RAngleLoc = MemExpr->getRAngleLoc();
+ if (LAngleLoc.isInvalid() || RAngleLoc.isInvalid() || LAngleLoc.isMacroID()
||
+ RAngleLoc.isMacroID())
+ return;
+
+ Diag << FixItHint::CreateRemoval(SourceRange(LAngleLoc, RAngleLoc));
+
+ // Also remove `template` keyword, if present.
+ if (MemExpr->hasTemplateKeyword())
+ Diag << FixItHint::CreateRemoval(MemExpr->getTemplateKeywordLoc());
+}
+
+} // namespace clang::tidy::llvm_check
diff --git a/clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.h
b/clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.h
new file mode 100644
index 0000000000000..7f1b857557e37
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/llvm/TypeSwitchCaseTypesCheck.h
@@ -0,0 +1,36 @@
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_TYPESWITCHCASETYPESCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_TYPESWITCHCASETYPESCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::llvm_check {
+
+/// Simplifies llvm::TypeSwitch Case calls by removing redundant explicit
+/// template arguments or replacing 'auto' lambda parameters with explicit
+/// types.
+///
+/// For the user-facing documentation see:
+///
https://clang.llvm.org/extra/clang-tidy/checks/llvm/type-switch-case-types.html
+class TypeSwitchCaseTypesCheck : public ClangTidyCheck {
+public:
+ TypeSwitchCaseTypesCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context) {}
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
+ return LangOpts.CPlusPlus;
+ }
+};
+
+} // namespace clang::tidy::llvm_check
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_LLVM_TYPESWITCHCASETYPESCHECK_H
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst
b/clang-tools-extra/docs/ReleaseNotes.rst
index 7f55341e2995b..c368d792dfcd6 100644
--- a/clang-tools-extra/docs/ReleaseNotes.rst
+++ b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -97,6 +97,12 @@ Improvements to clang-tidy
New checks
^^^^^^^^^^
+- New :doc:`llvm-type-switch-case-types
+ <clang-tidy/checks/llvm/type-switch-case-types>` check.
+
+ Finds ``llvm::TypeSwitch::Case`` calls with redundant explicit template
+ arguments that can be inferred from the lambda parameter type.
+
- New :doc:`llvm-use-vector-utils
<clang-tidy/checks/llvm/use-vector-utils>` check.
diff --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 25d1354fc4c20..d9a6e4fd6593c 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -256,6 +256,7 @@ Clang-Tidy Checks
:doc:`llvm-prefer-register-over-unsigned
<llvm/prefer-register-over-unsigned>`, "Yes"
:doc:`llvm-prefer-static-over-anonymous-namespace
<llvm/prefer-static-over-anonymous-namespace>`,
:doc:`llvm-twine-local <llvm/twine-local>`, "Yes"
+ :doc:`llvm-type-switch-case-types <llvm/type-switch-case-types>`, "Yes"
:doc:`llvm-use-new-mlir-op-builder <llvm/use-new-mlir-op-builder>`, "Yes"
:doc:`llvm-use-ranges <llvm/use-ranges>`, "Yes"
:doc:`llvm-use-vector-utils <llvm/use-vector-utils>`, "Yes"
diff --git
a/clang-tools-extra/docs/clang-tidy/checks/llvm/type-switch-case-types.rst
b/clang-tools-extra/docs/clang-tidy/checks/llvm/type-switch-case-types.rst
new file mode 100644
index 0000000000000..90a37e27b18ea
--- /dev/null
+++ b/clang-tools-extra/docs/clang-tidy/checks/llvm/type-switch-case-types.rst
@@ -0,0 +1,37 @@
+.. title:: clang-tidy - llvm-type-switch-case-types
+
+llvm-type-switch-case-types
+===========================
+
+Finds ``llvm::TypeSwitch::Case`` calls with redundant explicit template
+arguments that can be inferred from the lambda parameter type.
+
+This check identifies two patterns:
+
+1. **Redundant explicit type**: When the lambda parameter type matches the
+ ``Case`` template argument, the explicit type can be removed.
+
+2. **Auto parameter with explicit type**: When a lambda uses ``auto`` but
+ ``Case`` has an explicit template argument, suggests using an explicit
+ type in the lambda instead.
+
+Example
+-------
+
+.. code-block:: c++
+
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>([](DerivedA *a) { return 1; }) // Redundant.
+ .Case<DerivedB>([](auto b) { return 2; }); // `auto` with explicit
type.
+
+Transforms to:
+
+.. code-block:: c++
+
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case([](DerivedA *a) { return 1; }) // Type inferred from lambda.
+ .Case<DerivedB>([](auto b) { return 2; }); // Warning only.
+
+Note: The second case (``auto`` parameter) only emits a warning without a
+fix-it, because the deduced type of ``auto`` depends on ``dyn_cast`` behavior
+which varies between pointer types and MLIR handle types.
diff --git
a/clang-tools-extra/test/clang-tidy/checkers/llvm/type-switch-case-types.cpp
b/clang-tools-extra/test/clang-tidy/checkers/llvm/type-switch-case-types.cpp
new file mode 100644
index 0000000000000..87a8200800a18
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/llvm/type-switch-case-types.cpp
@@ -0,0 +1,175 @@
+// RUN: %check_clang_tidy -std=c++17-or-later %s llvm-type-switch-case-types %t
+
+namespace llvm {
+
+template <typename T, typename ResultT = int>
+class TypeSwitch {
+public:
+ TypeSwitch(T) {}
+
+ // Single-type Case with explicit template argument.
+ template <typename CaseT, typename CallableT>
+ TypeSwitch &Case(CallableT &&) { return *this; }
+
+ // Inferred Case: callable's first argument determines the type.
+ template <typename CallableT>
+ TypeSwitch &Case(CallableT &&) { return *this; }
+
+ // Variadic Case: multiple types with single callable.
+ template <typename CaseT, typename CaseT2, typename... CaseTs,
+ typename CallableT>
+ TypeSwitch &Case(CallableT &&) { return *this; }
+};
+
+} // namespace llvm
+
+// Test types for the switch cases.
+struct Base {};
+struct DerivedA : Base {};
+struct DerivedB : Base {};
+
+void test_explicit_type_matches_lambda_param(Base *base) {
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>([](DerivedA *a) { return 10; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](DerivedA *a) { return 10; });
+
+ llvm::TypeSwitch<Base *>(base)
+ .Case<DerivedA>([](DerivedA *) {});
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](DerivedA *) {});
+}
+
+void test_value_type_switch() {
+ // TypeSwitch on value types (not pointers) -- common in MLIR.
+ struct Type {};
+ struct Float16Type : Type {};
+
+ llvm::TypeSwitch<Type, int>(Type())
+ .Case<Float16Type>([](Float16Type) { return 1; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](Float16Type) { return 1; });
+}
+
+void test_auto_param_with_explicit_type(Base *base) {
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>([](auto a) { return 20; })
+ .Case<DerivedB>([](auto *b) { return 80; });
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: lambda parameter needlessly uses
'auto', use explicit type instead
+ // CHECK-MESSAGES: :[[@LINE-2]]:8: warning: lambda parameter needlessly uses
'auto', use explicit type instead
+}
+
+void test_already_inferred_case(Base *base) {
+ // Already using type-inferred Case - no warning expected.
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case([](DerivedA *a) { return 1; })
+ .Case([](DerivedB *b) { return 2; });
+}
+
+void test_const_param(const Base *base) {
+ llvm::TypeSwitch<const Base *, int>(base)
+ .Case<DerivedA>([](const DerivedA *a) { return 30; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](const DerivedA *a) { return 30; });
+}
+
+void test_comments_preserved(Base *base) {
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>(/*comment*/ [](DerivedA *a) { return 40; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case(/*comment*/ [](DerivedA *a) { return 40; });
+}
+
+void test_whitespace(Base *base) {
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case< DerivedA >([](DerivedA *a) { return 50; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](DerivedA *a) { return 50; });
+}
+
+template <typename T>
+void test_template_keyword(T *base) {
+ llvm::TypeSwitch<T *, int>(base)
+ .template Case<DerivedA>([](DerivedA *a) { return 90; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:17: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](DerivedA *a) { return 90; });
+}
+// Explicit instantiation to trigger the check.
+template void test_template_keyword<Base>(Base *);
+
+void test_fully_qualified(Base *base) {
+ ::llvm::TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>([](DerivedA *a) { return 60; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](DerivedA *a) { return 60; });
+}
+
+namespace llvm {
+void test_inside_llvm_namespace(Base *base) {
+ TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>([](DerivedA *a) { return 70; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: redundant explicit template
argument
+ // CHECK-FIXES: .Case([](DerivedA *a) { return 70; });
+
+ TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>([](auto a) { return 71; });
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: lambda parameter needlessly uses
'auto', use explicit type instead
+}
+} // namespace llvm
+
+void test_macro_in_type(Base *base) {
+#define CASE_TYPE DerivedA
+ // Warning + fix-it: angle brackets are real tokens, macro content is
deleted.
+ llvm::TypeSwitch<Base *, int>(base)
+ // CHECK-MESSAGES: :[[@LINE+1]]:8: warning: redundant explicit template
argument
+ .Case<CASE_TYPE>([](DerivedA *a) { return 1; });
+ // CHECK-FIXES: .Case([](DerivedA *a) { return 1; });
+#undef CASE_TYPE
+}
+
+void test_macro_entire_case(Base *base) {
+#define MAKE_CASE(Type) .Case<Type>([](Type *x) { return 1; })
+ // Warning emitted but no fix-it when angle brackets are in a macro.
+ llvm::TypeSwitch<Base *, int>(base)
+ MAKE_CASE(DerivedA);
+ // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: redundant explicit template
argument
+#undef MAKE_CASE
+}
+
+//===----------------------------------------------------------------------===//
+// Negative test cases - should NOT trigger any warnings.
+//===----------------------------------------------------------------------===//
+
+// Non-TypeSwitch Case method -- should not be modified.
+struct OtherClass {
+ template <typename T, typename F>
+ OtherClass &Case(F &&f) { return *this; }
+};
+
+void test_negative_non_type_switch() {
+ OtherClass()
+ .Case<DerivedA>([](DerivedA *a) {});
+ // No warning expected -- this is not `llvm::TypeSwitch`.
+}
+
+// TypeSwitch in non-llvm namespace.
+namespace other {
+template <typename T, typename R = int>
+struct TypeSwitch {
+ TypeSwitch(T) {}
+ template <typename CaseT, typename F>
+ TypeSwitch &Case(F &&) { return *this; }
+};
+} // namespace other
+
+void test_negative_non_llvm_namespace(Base *base) {
+ // No warning expected -- this is not `llvm::TypeSwitch`.
+ other::TypeSwitch<Base *, int>(base)
+ .Case<DerivedA>([](DerivedA *a) { return 1; });
+}
+
+void test_variadic_case_no_change(Base *base) {
+ // Variadic Case with multiple types - do not modify.
+ llvm::TypeSwitch<Base *, int>(base)
+ .Case<DerivedA, DerivedB>([](auto x) { return 1; });
+}
``````````
</details>
https://github.com/llvm/llvm-project/pull/177892
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
