Author: Piotr Zegar Date: 2023-10-26T07:11:01+02:00 New Revision: af07d7ba883b6e4921820d88b6679f294a0b9fa5
URL: https://github.com/llvm/llvm-project/commit/af07d7ba883b6e4921820d88b6679f294a0b9fa5 DIFF: https://github.com/llvm/llvm-project/commit/af07d7ba883b6e4921820d88b6679f294a0b9fa5.diff LOG: [clang-tidy] Improved cppcoreguidelines-pro-type-const-cast (#69501) Improved cppcoreguidelines-pro-type-const-cast check to ignore casts to const type (controlled by option) and casts in implicitly invoked code. Fixes #69319 Added: Modified: clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h clang-tools-extra/docs/ReleaseNotes.rst clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test Removed: ################################################################################ diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp index ef803ab85fa0841..8c44c1bfb62b6c7 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.cpp @@ -14,13 +14,60 @@ using namespace clang::ast_matchers; namespace clang::tidy::cppcoreguidelines { +static bool hasConstQualifier(QualType Type) { + const QualType PtrType = Type->getPointeeType(); + if (!PtrType.isNull()) + return hasConstQualifier(PtrType); + + return Type.isConstQualified(); +} + +static bool hasVolatileQualifier(QualType Type) { + const QualType PtrType = Type->getPointeeType(); + if (!PtrType.isNull()) + return hasVolatileQualifier(PtrType); + return Type.isVolatileQualified(); +} + +ProTypeConstCastCheck::ProTypeConstCastCheck(StringRef Name, + ClangTidyContext *Context) + : ClangTidyCheck(Name, Context), + StrictMode(Options.getLocalOrGlobal("StrictMode", false)) {} + +void ProTypeConstCastCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) { + Options.store(Opts, "StrictMode", StrictMode); +} + void ProTypeConstCastCheck::registerMatchers(MatchFinder *Finder) { Finder->addMatcher(cxxConstCastExpr().bind("cast"), this); } void ProTypeConstCastCheck::check(const MatchFinder::MatchResult &Result) { const auto *MatchedCast = Result.Nodes.getNodeAs<CXXConstCastExpr>("cast"); - diag(MatchedCast->getOperatorLoc(), "do not use const_cast"); + if (StrictMode) { + diag(MatchedCast->getOperatorLoc(), "do not use const_cast"); + return; + } + + const QualType TargetType = MatchedCast->getType().getCanonicalType(); + const QualType SourceType = + MatchedCast->getSubExpr()->getType().getCanonicalType(); + + const bool RemovingConst = + hasConstQualifier(SourceType) && !hasConstQualifier(TargetType); + const bool RemovingVolatile = + hasVolatileQualifier(SourceType) && !hasVolatileQualifier(TargetType); + + if (!RemovingConst && !RemovingVolatile) { + // Cast is doing nothing. + return; + } + + diag(MatchedCast->getOperatorLoc(), + "do not use const_cast to remove%select{| const}0%select{| " + "and}2%select{| volatile}1 qualifier") + << RemovingConst << RemovingVolatile + << (RemovingConst && RemovingVolatile); } } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h index f7ae9bbb60dcda3..8d93633a321b53f 100644 --- a/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h +++ b/clang-tools-extra/clang-tidy/cppcoreguidelines/ProTypeConstCastCheck.h @@ -13,19 +13,25 @@ namespace clang::tidy::cppcoreguidelines { -/// This check flags all instances of const_cast +/// Imposes limitations on the use of const_cast within C++ code. /// /// For the user-facing documentation see: /// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.html class ProTypeConstCastCheck : public ClangTidyCheck { public: - ProTypeConstCastCheck(StringRef Name, ClangTidyContext *Context) - : ClangTidyCheck(Name, Context) {} + ProTypeConstCastCheck(StringRef Name, ClangTidyContext *Context); bool isLanguageVersionSupported(const LangOptions &LangOpts) const override { return LangOpts.CPlusPlus; } void registerMatchers(ast_matchers::MatchFinder *Finder) override; void check(const ast_matchers::MatchFinder::MatchResult &Result) override; + void storeOptions(ClangTidyOptions::OptionMap &Opts) override; + std::optional<TraversalKind> getCheckTraversalKind() const override { + return TK_IgnoreUnlessSpelledInSource; + } + +private: + const bool StrictMode; }; } // namespace clang::tidy::cppcoreguidelines diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst index ac95afd782e1dbe..13003a118c36a51 100644 --- a/clang-tools-extra/docs/ReleaseNotes.rst +++ b/clang-tools-extra/docs/ReleaseNotes.rst @@ -255,6 +255,11 @@ Changes in existing checks <clang-tidy/checks/cppcoreguidelines/pro-bounds-constant-array-index>` check to perform checks on derived classes of ``std::array``. +- Improved :doc:`cppcoreguidelines-pro-type-const-cast + <clang-tidy/checks/cppcoreguidelines/pro-type-const-cast>` check to ignore + casts to ``const`` or ``volatile`` type (controlled by `StrictMode` option) + and casts in implicitly invoked code. + - Improved :doc:`cppcoreguidelines-pro-type-member-init <clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` check to ignore dependent delegate constructors. diff --git a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst index eb572e625f12977..961a591cb81f868 100644 --- a/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst +++ b/clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/pro-type-const-cast.rst @@ -3,11 +3,35 @@ cppcoreguidelines-pro-type-const-cast ===================================== -This check flags all uses of ``const_cast`` in C++ code. +Imposes limitations on the use of ``const_cast`` within C++ code. It depends on +the :option:`StrictMode` option setting to determine whether it should flag all +instances of ``const_cast`` or only those that remove either ``const`` or +``volatile`` qualifier. -Modifying a variable that was declared const is undefined behavior, even with -``const_cast``. +Modifying a variable that has been declared as ``const`` in C++ is generally +considered undefined behavior, and this remains true even when using +``const_cast``. In C++, the ``const`` qualifier indicates that a variable is +intended to be read-only, and the compiler enforces this by disallowing any +attempts to change the value of that variable. + +Removing the ``volatile`` qualifier in C++ can have serious consequences. This +qualifier indicates that a variable's value can change unpredictably, and +removing it may lead to undefined behavior, optimization problems, and debugging +challenges. It's essential to retain the ``volatile`` qualifier in situations +where the variable's volatility is a crucial aspect of program correctness and +reliability. This rule is part of the `Type safety (Type 3) <https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-type-constcast>`_ -profile from the C++ Core Guidelines. +profile and `ES.50: Don’t cast away const +<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#es50-dont-cast-away-const>`_ +rule from the C++ Core Guidelines. + +Options +------- + +.. option:: StrictMode + + When this setting is set to `true`, it means that any usage of ``const_cast`` + is not allowed. On the other hand, when it's set to `false`, it permits + casting to ``const`` or ``volatile`` types. Default value is `false`. diff --git a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp index 2d32e13723abf83..be70e3ba356991a 100644 --- a/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp +++ b/clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/pro-type-const-cast.cpp @@ -1,6 +1,86 @@ -// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-const-cast %t +// RUN: %check_clang_tidy -check-suffix=STRICT %s cppcoreguidelines-pro-type-const-cast %t -- -config="{CheckOptions: {StrictMode: true}}" +// RUN: %check_clang_tidy -check-suffix=NSTRICT %s cppcoreguidelines-pro-type-const-cast %t +namespace Const { const int *i; int *j; -void f() { j = const_cast<int *>(i); } -// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + +void f() { + j = const_cast<int *>(i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:7: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = const_cast<const int*>(j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + j = *const_cast<int **>(&i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = *const_cast<const int**>(&j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + j = &const_cast<int&>(*i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = &const_cast<const int&>(*j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] +} +} + +namespace Volatile { +volatile int *i; +int *j; + +void f() { + j = const_cast<int *>(i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:7: warning: do not use const_cast to remove volatile qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = const_cast<volatile int*>(j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + j = *const_cast<int **>(&i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove volatile qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = *const_cast<volatile int**>(&j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + j = &const_cast<int&>(*i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove volatile qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = &const_cast<volatile int&>(*j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] +} +} + +namespace ConstAndVolatile { +const volatile int *i; +int *j; + +void f() { + j = const_cast<int *>(i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:7: warning: do not use const_cast to remove const and volatile qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = const_cast<const volatile int*>(j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:7: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + j = *const_cast<int **>(&i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const and volatile qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = *const_cast<const volatile int**>(&j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + j = &const_cast<int&>(*i); + // CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:8: warning: do not use const_cast to remove const and volatile qualifier [cppcoreguidelines-pro-type-const-cast] + // CHECK-MESSAGES-STRICT: :[[@LINE-2]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] + + i = &const_cast<const volatile int&>(*j); + // CHECK-MESSAGES-STRICT: :[[@LINE-1]]:8: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] +} +} diff --git a/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test b/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test index 4cb4d1171195a01..9f98d86d0bcc0f1 100644 --- a/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test +++ b/clang-tools-extra/test/clang-tidy/infrastructure/nonstandard-file-extension.test @@ -3,4 +3,4 @@ const int *i; int *j; void f() { j = const_cast<int *>(i); } -// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use const_cast [cppcoreguidelines-pro-type-const-cast] +// CHECK-MESSAGES: :[[@LINE-1]]:16: warning: do not use const_cast to remove const qualifier [cppcoreguidelines-pro-type-const-cast] _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
