Skip to content

[clang-tidy] Add StrictMode to cppcoreguidelines-pro-type-static-cast-downcast #69529

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

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,24 @@ using namespace clang::ast_matchers;

namespace clang::tidy::cppcoreguidelines {

ProTypeStaticCastDowncastCheck::ProTypeStaticCastDowncastCheck(
StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context),
StrictMode(Options.getLocalOrGlobal("StrictMode", true)) {}

void ProTypeStaticCastDowncastCheck::storeOptions(
ClangTidyOptions::OptionMap &Opts) {
Options.store(Opts, "StrictMode", StrictMode);
}

void ProTypeStaticCastDowncastCheck::registerMatchers(MatchFinder *Finder) {
Finder->addMatcher(
cxxStaticCastExpr(unless(isInTemplateInstantiation())).bind("cast"),
this);
cxxStaticCastExpr(hasCastKind(CK_BaseToDerived)).bind("cast"), this);
}

void ProTypeStaticCastDowncastCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *MatchedCast = Result.Nodes.getNodeAs<CXXStaticCastExpr>("cast");
if (MatchedCast->getCastKind() != CK_BaseToDerived)
return;

QualType SourceType = MatchedCast->getSubExpr()->getType();
const auto *SourceDecl = SourceType->getPointeeCXXRecordDecl();
Expand All @@ -33,15 +40,20 @@ void ProTypeStaticCastDowncastCheck::check(
if (!SourceDecl)
return;

if (SourceDecl->isPolymorphic())
if (SourceDecl->isPolymorphic()) {
diag(MatchedCast->getOperatorLoc(),
"do not use static_cast to downcast from a base to a derived class; "
"use dynamic_cast instead")
<< FixItHint::CreateReplacement(MatchedCast->getOperatorLoc(),
"dynamic_cast");
else
diag(MatchedCast->getOperatorLoc(),
"do not use static_cast to downcast from a base to a derived class");
return;
}

if (!StrictMode)
return;

diag(MatchedCast->getOperatorLoc(),
"do not use static_cast to downcast from a base to a derived class");
}

} // namespace clang::tidy::cppcoreguidelines
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,19 @@ namespace clang::tidy::cppcoreguidelines {
/// http://clang.llvm.org/extra/clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast.html
class ProTypeStaticCastDowncastCheck : public ClangTidyCheck {
public:
ProTypeStaticCastDowncastCheck(StringRef Name, ClangTidyContext *Context)
: ClangTidyCheck(Name, Context) {}
ProTypeStaticCastDowncastCheck(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
Expand Down
5 changes: 5 additions & 0 deletions clang-tools-extra/docs/ReleaseNotes.rst
Original file line number Diff line number Diff line change
Expand Up @@ -240,6 +240,11 @@ Changes in existing checks
<clang-tidy/checks/cppcoreguidelines/pro-type-member-init>` check to ignore
dependent delegate constructors.

- Improved :doc:`cppcoreguidelines-pro-type-static-cast-downcast
<clang-tidy/checks/cppcoreguidelines/pro-type-static-cast-downcast>` check to
disregard casts on non-polymorphic types when the `StrictMode` option is set
to `false`.

- Improved :doc:`cppcoreguidelines-pro-type-vararg
<clang-tidy/checks/cppcoreguidelines/pro-type-vararg>` check to ignore
false-positives in unevaluated context (e.g., ``decltype``, ``sizeof``, ...).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,3 +14,11 @@ unrelated type ``Z``.
This rule is part of the `Type safety (Type.2)
<https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines#Pro-type-downcast>`_
profile from the C++ Core Guidelines.

Options
-------

.. option:: StrictMode

When set to `false`, no warnings are emitted for casts on non-polymorphic
types. Default is `true`.
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// RUN: %check_clang_tidy %s cppcoreguidelines-pro-type-static-cast-downcast %t
// RUN: %check_clang_tidy -check-suffixes=NSTRICT,STRICT %s cppcoreguidelines-pro-type-static-cast-downcast %t
// RUN: %check_clang_tidy -check-suffix=NSTRICT %s cppcoreguidelines-pro-type-static-cast-downcast %t -- -config="{CheckOptions: {StrictMode: false}}"

class Base {
};
Expand Down Expand Up @@ -26,11 +27,11 @@ class PolymorphicMultiDerived : public Base, public PolymorphicBase {
void pointers() {

auto P0 = static_cast<Derived*>(new Base());
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]

const Base* B0;
auto PC0 = static_cast<const Derived*>(B0);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]

auto P1 = static_cast<Base*>(new Derived()); // OK, upcast to a public base
auto P2 = static_cast<Base*>(new MultiDerived()); // OK, upcast to a public base
Expand All @@ -40,13 +41,13 @@ void pointers() {
void pointers_polymorphic() {

auto PP0 = static_cast<PolymorphicDerived*>(new PolymorphicBase());
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES: auto PP0 = dynamic_cast<PolymorphicDerived*>(new PolymorphicBase());
// CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES-NSTRICT: auto PP0 = dynamic_cast<PolymorphicDerived*>(new PolymorphicBase());

const PolymorphicBase* B0;
auto PPC0 = static_cast<const PolymorphicDerived*>(B0);
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES: auto PPC0 = dynamic_cast<const PolymorphicDerived*>(B0);
// CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES-NSTRICT: auto PPC0 = dynamic_cast<const PolymorphicDerived*>(B0);


auto B1 = static_cast<PolymorphicBase*>(new PolymorphicDerived()); // OK, upcast to a public base
Expand All @@ -57,27 +58,27 @@ void pointers_polymorphic() {
void arrays() {
Base ArrayOfBase[10];
auto A0 = static_cast<Derived*>(ArrayOfBase);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
}

void arrays_polymorphic() {
PolymorphicBase ArrayOfPolymorphicBase[10];
auto AP0 = static_cast<PolymorphicDerived*>(ArrayOfPolymorphicBase);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
// CHECK-FIXES: auto AP0 = dynamic_cast<PolymorphicDerived*>(ArrayOfPolymorphicBase);
// CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
// CHECK-FIXES-NSTRICT: auto AP0 = dynamic_cast<PolymorphicDerived*>(ArrayOfPolymorphicBase);
}

void references() {
Base B0;
auto R0 = static_cast<Derived&>(B0);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
Base& RefToBase = B0;
auto R1 = static_cast<Derived&>(RefToBase);
// CHECK-MESSAGES: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:13: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]

const Base& ConstRefToBase = B0;
auto RC1 = static_cast<const Derived&>(ConstRefToBase);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-MESSAGES-STRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class [cppcoreguidelines-pro-type-static-cast-downcast]


Derived RD1;
Expand All @@ -87,18 +88,18 @@ void references() {
void references_polymorphic() {
PolymorphicBase B0;
auto RP0 = static_cast<PolymorphicDerived&>(B0);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
// CHECK-FIXES: auto RP0 = dynamic_cast<PolymorphicDerived&>(B0);
// CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead
// CHECK-FIXES-NSTRICT: auto RP0 = dynamic_cast<PolymorphicDerived&>(B0);

PolymorphicBase& RefToPolymorphicBase = B0;
auto RP1 = static_cast<PolymorphicDerived&>(RefToPolymorphicBase);
// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES: auto RP1 = dynamic_cast<PolymorphicDerived&>(RefToPolymorphicBase);
// CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:14: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES-NSTRICT: auto RP1 = dynamic_cast<PolymorphicDerived&>(RefToPolymorphicBase);

const PolymorphicBase& ConstRefToPolymorphicBase = B0;
auto RPC2 = static_cast<const PolymorphicDerived&>(ConstRefToPolymorphicBase);
// CHECK-MESSAGES: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES: auto RPC2 = dynamic_cast<const PolymorphicDerived&>(ConstRefToPolymorphicBase);
// CHECK-MESSAGES-NSTRICT: :[[@LINE-1]]:15: warning: do not use static_cast to downcast from a base to a derived class; use dynamic_cast instead [cppcoreguidelines-pro-type-static-cast-downcast]
// CHECK-FIXES-NSTRICT: auto RPC2 = dynamic_cast<const PolymorphicDerived&>(ConstRefToPolymorphicBase);

PolymorphicDerived d1;
auto RP2 = static_cast<PolymorphicBase&>(d1); // OK, upcast to a public base
Expand Down