PiotrZSL created this revision.
PiotrZSL added reviewers: njames93, carlosgalvezp.
Herald added a subscriber: xazax.hun.
Herald added a project: All.
PiotrZSL requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.
Detects cases where the result of a new expression is used as a bool.
Fixes: #64461
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D157188
Files:
clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.h
clang-tools-extra/docs/ReleaseNotes.rst
clang-tools-extra/docs/clang-tidy/checks/bugprone/new-bool-conversion.rst
clang-tools-extra/docs/clang-tidy/checks/list.rst
clang-tools-extra/test/clang-tidy/checkers/bugprone/new-bool-conversion.cpp
Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/new-bool-conversion.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/new-bool-conversion.cpp
@@ -0,0 +1,38 @@
+// RUN: %check_clang_tidy %s bugprone-new-bool-conversion %t
+
+void takeBool(bool);
+
+void testImplicit() {
+ takeBool(new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+ takeBool(new bool);
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+
+ bool value;
+
+ value = new int;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+ value = new bool;
+ // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+}
+
+void testExplicit() {
+ takeBool(static_cast<bool>(new int));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+ takeBool(static_cast<bool>(new bool));
+ // CHECK-MESSAGES: :[[@LINE-1]]:30: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+}
+
+void testNegation() {
+ takeBool(!new bool);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+ takeBool(!new int);
+ // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+
+ bool value;
+
+ value = !new int;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+ value = !new bool;
+ // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: result of the 'new' expression is being used as a boolean value, which may lead to unintended behavior or memory leaks [bugprone-new-bool-conversion]
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===================================================================
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -107,6 +107,7 @@
`bugprone-multi-level-implicit-pointer-conversion <bugprone/multi-level-implicit-pointer-conversion.html>`_,
`bugprone-multiple-new-in-one-expression <bugprone/multiple-new-in-one-expression.html>`_,
`bugprone-multiple-statement-macro <bugprone/multiple-statement-macro.html>`_,
+ `bugprone-new-bool-conversion <bugprone/new-bool-conversion.html>`_,
`bugprone-no-escape <bugprone/no-escape.html>`_,
`bugprone-non-zero-enum-to-bool-conversion <bugprone/non-zero-enum-to-bool-conversion.html>`_,
`bugprone-not-null-terminated-result <bugprone/not-null-terminated-result.html>`_, "Yes"
Index: clang-tools-extra/docs/clang-tidy/checks/bugprone/new-bool-conversion.rst
===================================================================
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/bugprone/new-bool-conversion.rst
@@ -0,0 +1,44 @@
+.. title:: clang-tidy - bugprone-new-bool-conversion
+
+bugprone-new-bool-conversion
+============================
+
+Detects cases where the result of a new expression is used as a ``bool``.
+
+In C++, the new expression dynamically allocates memory on the heap and returns
+a pointer to the newly created object. However, when developers inadvertently
+use this pointer directly as a boolean value, either through implicit or
+explicit conversion, a hidden problem emerges. The crux of the issue lies in
+the fact that any non-null pointer implicitly converts to ``true``, masking
+potential errors and making the code difficult to comprehend. Worse yet, it may
+trigger memory leaks, as dynamically allocated objects remain inaccessible,
+never to be released by the program.
+
+Example:
+
+.. code-block:: c++
+
+ #include <iostream>
+
+ bool processResource(bool resourceFlag) {
+ if (resourceFlag) {
+ std::cout << "Resource processing successful!" << std::endl;
+ return true;
+ } else {
+ std::cout << "Resource processing failed!" << std::endl;
+ return false;
+ }
+ }
+
+ int main() {
+ // Implicit conversion of int* to bool
+ processResource(new int());
+ return 0;
+ }
+
+In this example, we pass the result of the ``new int()`` expression directly to
+the ``processResource`` function as its argument. Since the pointer returned by
+``new`` is non-null, it is implicitly converted to ``true``, leading to
+incorrect behavior in the ``processResource`` function.
+
+Check comes with no configuration options and does not offer any auto-fixes.
Index: clang-tools-extra/docs/ReleaseNotes.rst
===================================================================
--- clang-tools-extra/docs/ReleaseNotes.rst
+++ clang-tools-extra/docs/ReleaseNotes.rst
@@ -129,6 +129,11 @@
Detects implicit conversions between pointers of different levels of
indirection.
+- New :doc:`bugprone-new-bool-conversion
+ <clang-tidy/checks/bugprone/new-bool-conversion>` check.
+
+ Detects cases where the result of a new expression is used as a ``bool``.
+
- New :doc:`bugprone-optional-value-conversion
<clang-tidy/checks/bugprone/optional-value-conversion>` check.
Index: clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.h
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.h
@@ -0,0 +1,33 @@
+//===--- NewBoolConversionCheck.h - clang-tidy ------------------*- C++ -*-===//
+//
+// 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_BUGPRONE_NEWBOOLCONVERSIONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NEWBOOLCONVERSIONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Detects cases where the result of a new expression is used as a bool.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/new-bool-conversion.html
+class NewBoolConversionCheck : public ClangTidyCheck {
+public:
+ NewBoolConversionCheck(StringRef Name, ClangTidyContext *Context)
+ : ClangTidyCheck(Name, 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;
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_NEWBOOLCONVERSIONCHECK_H
Index: clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.cpp
===================================================================
--- /dev/null
+++ clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.cpp
@@ -0,0 +1,33 @@
+//===--- NewBoolConversionCheck.cpp - clang-tidy --------------------------===//
+//
+// 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 "NewBoolConversionCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void NewBoolConversionCheck::registerMatchers(MatchFinder *Finder) {
+ Finder->addMatcher(
+ castExpr(hasCastKind(CK_PointerToBoolean),
+ hasSourceExpression(ignoringImplicit(cxxNewExpr())))
+ .bind("cast"),
+ this);
+}
+
+void NewBoolConversionCheck::check(const MatchFinder::MatchResult &Result) {
+ const auto *MatchedExpr = Result.Nodes.getNodeAs<CastExpr>("cast");
+
+ diag(MatchedExpr->getExprLoc(),
+ "result of the 'new' expression is being used as a boolean value, which "
+ "may lead to unintended behavior or memory leaks");
+}
+
+} // namespace clang::tidy::bugprone
Index: clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
+++ clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
@@ -22,6 +22,7 @@
ForwardingReferenceOverloadCheck.cpp
ImplicitWideningOfMultiplicationResultCheck.cpp
InaccurateEraseCheck.cpp
+ NewBoolConversionCheck.cpp
SwitchMissingDefaultCaseCheck.cpp
IncDecInConditionsCheck.cpp
IncorrectRoundingsCheck.cpp
Index: clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
===================================================================
--- clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
@@ -41,6 +41,7 @@
#include "MultiLevelImplicitPointerConversionCheck.h"
#include "MultipleNewInOneExpressionCheck.h"
#include "MultipleStatementMacroCheck.h"
+#include "NewBoolConversionCheck.h"
#include "NoEscapeCheck.h"
#include "NonZeroEnumToBoolConversionCheck.h"
#include "NotNullTerminatedResultCheck.h"
@@ -122,6 +123,8 @@
"bugprone-implicit-widening-of-multiplication-result");
CheckFactories.registerCheck<InaccurateEraseCheck>(
"bugprone-inaccurate-erase");
+ CheckFactories.registerCheck<NewBoolConversionCheck>(
+ "bugprone-new-bool-conversion");
CheckFactories.registerCheck<SwitchMissingDefaultCaseCheck>(
"bugprone-switch-missing-default-case");
CheckFactories.registerCheck<IncDecInConditionsCheck>(
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
