[clang-tidy] Add bugprone-allocation-bool-conversion

[PiotrZSL]

Summary

Detects cases where the result of a resource allocation is used as a bool.

Fixes: #64461

[Eugene.Zelenko]

Shouldn't C-style allocation be checked too? Same for custom allocators via configuration option.

[Eugene.Zelenko]

Same for file, threads and other resources allocation.

[PiotrZSL]

In D157188#4563105, @Eugene.Zelenko wrote:

Shouldn't C-style allocation be checked too? Same for custom allocators via configuration option.

In theory it could, but in such case name of check would need to change...

[Eugene.Zelenko]

In D157188#4563139, @PiotrZSL wrote:

In D157188#4563105, @Eugene.Zelenko wrote:

Shouldn't C-style allocation be checked too? Same for custom allocators via configuration option.

In theory it could, but in such case name of check would need to change...

Something like bugprone-resource-bool-conversion should be generic enough.

[chandlerc]

In D157188#4563143, @Eugene.Zelenko wrote:

In D157188#4563139, @PiotrZSL wrote:

In D157188#4563105, @Eugene.Zelenko wrote:

Shouldn't C-style allocation be checked too? Same for custom allocators via configuration option.

In theory it could, but in such case name of check would need to change...

Something like bugprone-resource-bool-conversion should be generic enough.

Maybe bugprone-allocation-bool-conversion?

[PiotrZSL]

Change check anme, add configuration option.

[Eugene.Zelenko]

clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.cpp

25 ↗

(On Diff #547523)

POSIX open, openat, creat, pthread_create should be added too.

60 ↗

(On Diff #547523)

Should be merged together.

[PiotrZSL]

Formating + fix tests

clang-tools-extra/clang-tidy/bugprone/AllocationBoolConversionCheck.cpp

25 ↗

(On Diff #547523)

Those return integers, not pointers. Check is for pointers only, but probably could be extended to integers if we go into this direction.

[PiotrZSL]

TODO: Fix CI, Add support for allocators that return 'integer', hardcode most of functions, change config into AdditionalAllocationFunctions

[PiotrZSL]

Rebase, add support for integer returnign functions, fixes in tests.

clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp

 #include "MacroRepeatedSideEffectsCheck.h"
 #include "MisplacedOperatorInStrlenInAllocCheck.h"
 #include "MisplacedPointerArithmeticInAllocCheck.h"
 #include "MisplacedWideningCastCheck.h"
 #include "MoveForwardingReferenceCheck.h"
 #include "MultiLevelImplicitPointerConversionCheck.h"
 #include "MultipleNewInOneExpressionCheck.h"
 #include "MultipleStatementMacroCheck.h"
+#include "NewBoolConversionCheck.h"
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
 #include "NotNullTerminatedResultCheck.h"
 #include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
 #include "PosixReturnCheck.h"
 #include "RedundantBranchConditionCheck.h"
 #include "ReservedIdentifierCheck.h"
void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override {
     CheckFactories.registerCheck<ForwardDeclarationNamespaceCheck>(
         "bugprone-forward-declaration-namespace");
     CheckFactories.registerCheck<ForwardingReferenceOverloadCheck>(
         "bugprone-forwarding-reference-overload");
     CheckFactories.registerCheck<ImplicitWideningOfMultiplicationResultCheck>(
         "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>(
         "bugprone-inc-dec-in-conditions");
     CheckFactories.registerCheck<IncorrectRoundingsCheck>(
         "bugprone-incorrect-roundings");
     CheckFactories.registerCheck<InfiniteLoopCheck>("bugprone-infinite-loop");
     CheckFactories.registerCheck<IntegerDivisionCheck>(

clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt

add_clang_library(clangTidyBugproneModule
   EasilySwappableParametersCheck.cpp
   EmptyCatchCheck.cpp
   ExceptionEscapeCheck.cpp
   FoldInitTypeCheck.cpp
   ForwardDeclarationNamespaceCheck.cpp
   ForwardingReferenceOverloadCheck.cpp
   ImplicitWideningOfMultiplicationResultCheck.cpp
   InaccurateEraseCheck.cpp
+  NewBoolConversionCheck.cpp
   SwitchMissingDefaultCaseCheck.cpp
   IncDecInConditionsCheck.cpp
   IncorrectRoundingsCheck.cpp
   InfiniteLoopCheck.cpp
   IntegerDivisionCheck.cpp
   LambdaFunctionNameCheck.cpp
   MacroParenthesesCheck.cpp
   MacroRepeatedSideEffectsCheck.cpp

clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.h

+//===--- 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

clang-tools-extra/clang-tidy/bugprone/NewBoolConversionCheck.cpp

+//===--- 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

clang-tools-extra/docs/ReleaseNotes.rst

- New :doc:`bugprone-inc-dec-in-conditions
   the variable's value.
 
 - New :doc:`bugprone-multi-level-implicit-pointer-conversion
   <clang-tidy/checks/bugprone/multi-level-implicit-pointer-conversion>` check.
 
   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.
 
   Detects potentially unintentional and redundant conversions where a value is
   extracted from an optional-like type and then used to create a new instance
   of the same optional-like type.
 
 - New :doc:`modernize-use-constraints

clang-tools-extra/docs/clang-tidy/checks/bugprone/new-bool-conversion.rst

+.. 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.

clang-tools-extra/docs/clang-tidy/checks/list.rst

.. csv-table::
    `bugprone-macro-repeated-side-effects <bugprone/macro-repeated-side-effects.html>`_,
    `bugprone-misplaced-operator-in-strlen-in-alloc <bugprone/misplaced-operator-in-strlen-in-alloc.html>`_, "Yes"
    `bugprone-misplaced-pointer-arithmetic-in-alloc <bugprone/misplaced-pointer-arithmetic-in-alloc.html>`_, "Yes"
    `bugprone-misplaced-widening-cast <bugprone/misplaced-widening-cast.html>`_,
    `bugprone-move-forwarding-reference <bugprone/move-forwarding-reference.html>`_, "Yes"
    `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"
    `bugprone-optional-value-conversion <bugprone/optional-value-conversion.html>`_, "Yes"
    `bugprone-parent-virtual-call <bugprone/parent-virtual-call.html>`_, "Yes"
    `bugprone-posix-return <bugprone/posix-return.html>`_, "Yes"
    `bugprone-redundant-branch-condition <bugprone/redundant-branch-condition.html>`_, "Yes"
    `bugprone-reserved-identifier <bugprone/reserved-identifier.html>`_, "Yes"

clang-tools-extra/test/clang-tidy/checkers/bugprone/new-bool-conversion.cpp

+// 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]
+}