-
Notifications
You must be signed in to change notification settings - Fork 15.1k
New check for static_cast of an enum class type #167212
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be notified. If you wish to, you can add reviewers by using the "Reviewers" section on this page. If this is not working for you, it is probably because you do not have write permissions for the repository. In which case you can instead tag reviewers by name in a comment by using If you have received no comments on your PR for a week, you can request a review by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate is once a week. Please remember that you are asking for valuable time from other developers. If you have further questions, they may be answered by the LLVM GitHub User Guide. You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums. |
|
@llvm/pr-subscribers-clang-tidy @llvm/pr-subscribers-clang-tools-extra Author: None (arjunkonidala) ChangesIt warns the user to use to_underlying function in the utility header (which is introduced in c++23 )instead of static_cast with a hard coded tyep to prevent truncation errors. Full diff: https://github.com/llvm/llvm-project/pull/167212.diff 3 Files Affected:
diff --git a/clang-tools-extra/clang-tidy/modernize/UseToUnderlyingCheck.cpp b/clang-tools-extra/clang-tidy/modernize/UseToUnderlyingCheck.cpp
new file mode 100644
index 0000000000000..4ef2305d2d210
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseToUnderlyingCheck.cpp
@@ -0,0 +1,77 @@
+//===----------------------------------------------------------------------===//
+//
+// 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 "UseToUnderlyingCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::modernize {
+
+UseToUnderlyingCheck::UseToUnderlyingCheck(StringRef Name,
+ ClangTidyContext *Context)
+ : ClangTidyCheck(Name, Context),
+ Inserter(Options.getLocalOrGlobal("IncludeStyle",
+ utils::IncludeSorter::IS_LLVM),
+ areDiagsSelfContained()) {}
+//
+void UseToUnderlyingCheck::registerPPCallbacks(const SourceManager &SM,
+ Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) {
+ Inserter.registerPreprocessor(PP);
+}
+
+void UseToUnderlyingCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+ Options.store(Opts, "IncludeStyle", Inserter.getStyle());
+}
+
+bool UseToUnderlyingCheck::isLanguageVersionSupported(
+ const LangOptions &LangOpts) const {
+ return LangOpts.CPlusPlus23;
+}
+
+void UseToUnderlyingCheck::registerMatchers(MatchFinder *Finder) {
+ // FIXME: Add matchers.
+ Finder->addMatcher(
+ cxxStaticCastExpr( // C++ cast
+ hasDestinationType(
+ isInteger()), // casting to any type of integer (int,long,etc)
+ hasSourceExpression( // is an enum class
+ expr(hasType(enumType(hasDeclaration(enumDecl(isScoped())))))
+ .bind("enumExpr"))) // giving the name enumExpr
+ .bind("castExpr"), // giving the name castExpr
+ this);
+}
+
+void UseToUnderlyingCheck::check(const MatchFinder::MatchResult &Result) {
+ // Acquiring the enumExpr and castExpr using getNodeAS
+ const auto *Enum = Result.Nodes.getNodeAs<Expr>("enumExpr");
+ const auto *Cast = Result.Nodes.getNodeAs<CXXStaticCastExpr>("castExpr");
+
+ // getting contents of that node using getsourcetext
+ StringRef EnumExprText = Lexer::getSourceText(
+ CharSourceRange::getTokenRange(Enum->getSourceRange()),
+ *Result.SourceManager, getLangOpts());
+ // Suggestion to the user regarding the cast expr
+ std::string Replacement = ("std::to_underlying(" + EnumExprText + ")").str();
+ // gives and warning message if static cast is used instead to_underlying
+ auto Diag = diag(
+ Cast->getBeginLoc(),
+ "use 'std::to_underlying' instead of 'static_cast' for 'enum class'");
+ // suggest and hint for fixing it.
+ Diag << FixItHint::CreateReplacement(Cast->getSourceRange(), Replacement);
+
+ Diag << Inserter.createIncludeInsertion(
+ Result.Context->getSourceManager().getFileID(Cast->getBeginLoc()),
+ "<utility>");
+}
+
+} // namespace clang::tidy::modernize
diff --git a/clang-tools-extra/clang-tidy/modernize/UseToUnderlyingCheck.h b/clang-tools-extra/clang-tidy/modernize/UseToUnderlyingCheck.h
new file mode 100644
index 0000000000000..823bae6115cc7
--- /dev/null
+++ b/clang-tools-extra/clang-tidy/modernize/UseToUnderlyingCheck.h
@@ -0,0 +1,37 @@
+//===----------------------------------------------------------------------===//
+//
+// 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_MODERNIZE_USETOUNDERLYINGCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USETOUNDERLYINGCHECK_H
+
+#include "../ClangTidyCheck.h"
+#include "../utils/IncludeInserter.h"
+
+namespace clang::tidy::modernize {
+
+/// Warns user to use to_underlying function from utility header instead of static_cast<T> for a enum class type
+///
+/// For the user-facing documentation see:
+/// https://clang.llvm.org/extra/clang-tidy/checks/modernize/use-to-underlying.html
+class UseToUnderlyingCheck : public ClangTidyCheck {
+public:
+ UseToUnderlyingCheck(StringRef Name, ClangTidyContext *Context);
+ void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+ void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+ bool isLanguageVersionSupported(const LangOptions &LangOpts) const override;
+ void registerPPCallbacks(const SourceManager &SM, Preprocessor *PP,
+ Preprocessor *ModuleExpanderPP) override;
+ void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
+
+private:
+ utils::IncludeInserter Inserter;
+};
+
+} // namespace clang::tidy::modernize
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_MODERNIZE_USETOUNDERLYINGCHECK_H
diff --git a/clang-tools-extra/test/clang-tidy/checkers/modernize/use-to-underlying.cpp b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-to-underlying.cpp
new file mode 100644
index 0000000000000..8eb4fa0c67921
--- /dev/null
+++ b/clang-tools-extra/test/clang-tidy/checkers/modernize/use-to-underlying.cpp
@@ -0,0 +1,42 @@
+// RUN: %check_clang_tidy -std=c++23 %s modernize-use-to-underlying %t
+
+namespace std {
+template<typename T>
+constexpr auto to_underlying(T value) noexcept {
+ return static_cast<__underlying_type(T)>(value);
+}
+}
+
+
+enum class MyEnum { A = 1, B = 2 };
+
+void test_basic_cast() {
+ int value = static_cast<int>(MyEnum::A);
+ // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: use 'std::to_underlying' instead of 'static_cast' for 'enum class' [modernize-use-to-underlying]
+ // CHECK-FIXES: int value = std::to_underlying(MyEnum::A);
+}
+
+
+void test_long_cast() {
+ long value = static_cast<long>(MyEnum::B);
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use 'std::to_underlying' instead of 'static_cast' for 'enum class' [modernize-use-to-underlying]
+ // CHECK-FIXES: long value = std::to_underlying(MyEnum::B);
+}
+
+
+void test_expression() {
+ int result = static_cast<int>(MyEnum::A) + 10;
+ // CHECK-MESSAGES: :[[@LINE-1]]:16: warning: use 'std::to_underlying' instead of 'static_cast' for 'enum class' [modernize-use-to-underlying]
+ // CHECK-FIXES: int result = std::to_underlying(MyEnum::A) + 10;
+}
+
+void test_already_correct() {
+ int value = std::to_underlying(MyEnum::B);
+ // No warning expected
+}
+
+void test_float_cast() {
+ float y = 8.34;
+ int z = static_cast<int>(y);
+ // No warning expected
+}
|
EugeneZelenko
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add documentation and Release Notes entry.
| Inserter(Options.getLocalOrGlobal("IncludeStyle", | ||
| utils::IncludeSorter::IS_LLVM), | ||
| areDiagsSelfContained()) {} | ||
| // |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // |
| } | ||
|
|
||
| void UseToUnderlyingCheck::registerMatchers(MatchFinder *Finder) { | ||
| // FIXME: Add matchers. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| // FIXME: Add matchers. |
| const auto *Cast = Result.Nodes.getNodeAs<CXXStaticCastExpr>("castExpr"); | ||
|
|
||
| // getting contents of that node using getsourcetext | ||
| StringRef EnumExprText = Lexer::getSourceText( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| StringRef EnumExprText = Lexer::getSourceText( | |
| const StringRef EnumExprText = Lexer::getSourceText( |
| CharSourceRange::getTokenRange(Enum->getSourceRange()), | ||
| *Result.SourceManager, getLangOpts()); | ||
| // Suggestion to the user regarding the cast expr | ||
| std::string Replacement = ("std::to_underlying(" + EnumExprText + ")").str(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| std::string Replacement = ("std::to_underlying(" + EnumExprText + ")").str(); | |
| const std::string Replacement = ("std::to_underlying(" + EnumExprText + ")").str(); |
zwuis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please finish #71543 (comment).
HerrCai0907
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please use clang-tools-extra/clang-tidy/add_new_check.py to create new check
It warns the user to use to_underlying function in the utility header (which is introduced in c++23 )instead of static_cast with a hard coded tyep to prevent truncation errors.