merge main into amd-staging

Change-Id: I37f268af07ab9f60a3b47411f2ba10ae3074311c

Author: ronlieb

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

@@ -17,6 +17,7 @@
 #include "BoolPointerImplicitConversionCheck.h"
 #include "BranchCloneCheck.h"
 #include "CastingThroughVoidCheck.h"
+#include "ChainedComparisonCheck.h"
 #include "ComparePointerToMemberVirtualFunctionCheck.h"
 #include "CopyConstructorInitCheck.h"
 #include "DanglingHandleCheck.h"
@@ -108,6 +109,8 @@ class BugproneModule : public ClangTidyModule {
     CheckFactories.registerCheck<BranchCloneCheck>("bugprone-branch-clone");
     CheckFactories.registerCheck<CastingThroughVoidCheck>(
         "bugprone-casting-through-void");
+    CheckFactories.registerCheck<ChainedComparisonCheck>(
+        "bugprone-chained-comparison");
     CheckFactories.registerCheck<ComparePointerToMemberVirtualFunctionCheck>(
         "bugprone-compare-pointer-to-member-virtual-function");
     CheckFactories.registerCheck<CopyConstructorInitCheck>(

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

@@ -12,6 +12,7 @@ add_clang_library(clangTidyBugproneModule
   BranchCloneCheck.cpp
   BugproneTidyModule.cpp
   CastingThroughVoidCheck.cpp
+  ChainedComparisonCheck.cpp
   ComparePointerToMemberVirtualFunctionCheck.cpp
   CopyConstructorInitCheck.cpp
   DanglingHandleCheck.cpp

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

@@ -0,0 +1,150 @@
+//===--- ChainedComparisonCheck.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 "ChainedComparisonCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "llvm/ADT/SmallString.h"
+#include "llvm/ADT/SmallVector.h"
+#include <algorithm>
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+static bool isExprAComparisonOperator(const Expr *E) {
+  if (const auto *Op = dyn_cast_or_null<BinaryOperator>(E->IgnoreImplicit()))
+    return Op->isComparisonOp();
+  if (const auto *Op =
+          dyn_cast_or_null<CXXOperatorCallExpr>(E->IgnoreImplicit()))
+    return Op->isComparisonOp();
+  return false;
+}
+
+namespace {
+AST_MATCHER(BinaryOperator,
+            hasBinaryOperatorAChildComparisonOperatorWithoutParen) {
+  return isExprAComparisonOperator(Node.getLHS()) ||
+         isExprAComparisonOperator(Node.getRHS());
+}
+
+AST_MATCHER(CXXOperatorCallExpr,
+            hasCppOperatorAChildComparisonOperatorWithoutParen) {
+  return std::any_of(Node.arg_begin(), Node.arg_end(),
+                     isExprAComparisonOperator);
+}
+
+struct ChainedComparisonData {
+  llvm::SmallString<256U> Name;
+  llvm::SmallVector<const Expr *, 32U> Operands;
+
+  explicit ChainedComparisonData(const Expr *Op) { extract(Op); }
+
+private:
+  void add(const Expr *Operand);
+  void add(llvm::StringRef Opcode);
+  void extract(const Expr *Op);
+  void extract(const BinaryOperator *Op);
+  void extract(const CXXOperatorCallExpr *Op);
+};
+
+void ChainedComparisonData::add(const Expr *Operand) {
+  if (!Name.empty())
+    Name += ' ';
+  Name += 'v';
+  Name += std::to_string(Operands.size());
+  Operands.push_back(Operand);
+}
+
+void ChainedComparisonData::add(llvm::StringRef Opcode) {
+  Name += ' ';
+  Name += Opcode;
+}
+
+void ChainedComparisonData::extract(const BinaryOperator *Op) {
+  const Expr *LHS = Op->getLHS()->IgnoreImplicit();
+  if (isExprAComparisonOperator(LHS))
+    extract(LHS);
+  else
+    add(LHS);
+
+  add(Op->getOpcodeStr());
+
+  const Expr *RHS = Op->getRHS()->IgnoreImplicit();
+  if (isExprAComparisonOperator(RHS))
+    extract(RHS);
+  else
+    add(RHS);
+}
+
+void ChainedComparisonData::extract(const CXXOperatorCallExpr *Op) {
+  const Expr *FirstArg = Op->getArg(0U)->IgnoreImplicit();
+  if (isExprAComparisonOperator(FirstArg))
+    extract(FirstArg);
+  else
+    add(FirstArg);
+
+  add(getOperatorSpelling(Op->getOperator()));
+
+  const Expr *SecondArg = Op->getArg(1U)->IgnoreImplicit();
+  if (isExprAComparisonOperator(SecondArg))
+    extract(SecondArg);
+  else
+    add(SecondArg);
+}
+
+void ChainedComparisonData::extract(const Expr *Op) {
+  if (!Op)
+    return;
+
+  if (const auto *BinaryOp = dyn_cast<BinaryOperator>(Op)) {
+    extract(BinaryOp);
+    return;
+  }
+
+  if (const auto *OverloadedOp = dyn_cast<CXXOperatorCallExpr>(Op)) {
+    if (OverloadedOp->getNumArgs() == 2U)
+      extract(OverloadedOp);
+  }
+}
+
+} // namespace
+
+void ChainedComparisonCheck::registerMatchers(MatchFinder *Finder) {
+  const auto OperatorMatcher = expr(anyOf(
+      binaryOperator(isComparisonOperator(),
+                     hasBinaryOperatorAChildComparisonOperatorWithoutParen()),
+      cxxOperatorCallExpr(
+          isComparisonOperator(),
+          hasCppOperatorAChildComparisonOperatorWithoutParen())));
+
+  Finder->addMatcher(
+      expr(OperatorMatcher, unless(hasParent(OperatorMatcher))).bind("op"),
+      this);
+}
+
+void ChainedComparisonCheck::check(const MatchFinder::MatchResult &Result) {
+  const auto *MatchedOperator = Result.Nodes.getNodeAs<Expr>("op");
+
+  ChainedComparisonData Data(MatchedOperator);
+  if (Data.Operands.empty())
+    return;
+
+  diag(MatchedOperator->getBeginLoc(),
+       "chained comparison '%0' may generate unintended results, use "
+       "parentheses to specify order of evaluation or a logical operator to "
+       "separate comparison expressions")
+      << llvm::StringRef(Data.Name).trim() << MatchedOperator->getSourceRange();
+
+  for (std::size_t Index = 0U; Index < Data.Operands.size(); ++Index) {
+    diag(Data.Operands[Index]->getBeginLoc(), "operand 'v%0' is here",
+         DiagnosticIDs::Note)
+        << Index << Data.Operands[Index]->getSourceRange();
+  }
+}
+
+} // namespace clang::tidy::bugprone

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

@@ -0,0 +1,34 @@
+//===--- ChainedComparisonCheck.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_CHAINEDCOMPARISONCHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CHAINEDCOMPARISONCHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Check detects chained comparison operators that can lead to unintended
+/// behavior or logical errors.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/chained-comparison.html
+class ChainedComparisonCheck : public ClangTidyCheck {
+public:
+  ChainedComparisonCheck(StringRef Name, ClangTidyContext *Context)
+      : ClangTidyCheck(Name, Context) {}
+  void registerMatchers(ast_matchers::MatchFinder *Finder) override;
+  void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE_CHAINEDCOMPARISONCHECK_H

clang-tools-extra/clang-tidy/cppcoreguidelines/PreferMemberInitializerCheck.cpp

@@ -261,9 +261,9 @@ void PreferMemberInitializerCheck::check(
 
       SmallString<128> Insertion(
           {UseAssignment ? " = " : "{",
-           Lexer::getSourceText(
-               CharSourceRange(InitValue->getSourceRange(), true),
-               *Result.SourceManager, getLangOpts()),
+           Lexer::getSourceText(Result.SourceManager->getExpansionRange(
+                                    InitValue->getSourceRange()),
+                                *Result.SourceManager, getLangOpts()),
            UseAssignment ? "" : "}"});
 
       Diag << FixItHint::CreateInsertion(FieldEnd, Insertion)
@@ -346,7 +346,7 @@ void PreferMemberInitializerCheck::check(
       if (InvalidFix)
         continue;
       StringRef NewInit = Lexer::getSourceText(
-          CharSourceRange(InitValue->getSourceRange(), true),
+          Result.SourceManager->getExpansionRange(InitValue->getSourceRange()),
           *Result.SourceManager, getLangOpts());
       if (HasInitAlready) {
         if (InsertPos.isValid())

clang-tools-extra/docs/ReleaseNotes.rst

@@ -143,6 +143,12 @@ New checks
 
   Detects unsafe or redundant two-step casting operations involving ``void*``.
 
+- New :doc:`bugprone-chained-comparison
+  <clang-tidy/checks/bugprone/chained-comparison>` check.
+
+  Check detects chained comparison operators that can lead to unintended
+  behavior or logical errors.
+
 - New :doc:`bugprone-compare-pointer-to-member-virtual-function
   <clang-tidy/checks/bugprone/compare-pointer-to-member-virtual-function>` check.
 
@@ -337,7 +343,8 @@ Changes in existing checks
 - Improved :doc:`cppcoreguidelines-prefer-member-initializer
   <clang-tidy/checks/cppcoreguidelines/prefer-member-initializer>` check to
   ignore delegate constructors and ignore re-assignment for reference or when
-  initialization depend on field that is initialized before.
+  initialization depend on field that is initialized before. Additionally, it
+  now provides valid fixes for member variables initialized with macros.
 
 - Improved :doc:`cppcoreguidelines-pro-bounds-array-to-pointer-decay
   <clang-tidy/checks/cppcoreguidelines/pro-bounds-array-to-pointer-decay>` check

clang-tools-extra/docs/clang-tidy/checks/bugprone/chained-comparison.rst

@@ -0,0 +1,73 @@
+.. title:: clang-tidy - bugprone-chained-comparison
+
+bugprone-chained-comparison
+===========================
+
+Check detects chained comparison operators that can lead to unintended
+behavior or logical errors.
+
+Chained comparisons are expressions that use multiple comparison operators
+to compare three or more values. For example, the expression ``a < b < c``
+compares the values of ``a``, ``b``, and ``c``. However, this expression does
+not evaluate as ``(a < b) && (b < c)``, which is probably what the developer
+intended. Instead, it evaluates as ``(a < b) < c``, which may produce
+unintended results, especially when the types of ``a``, ``b``, and ``c`` are
+different.
+
+To avoid such errors, the check will issue a warning when a chained
+comparison operator is detected, suggesting to use parentheses to specify
+the order of evaluation or to use a logical operator to separate comparison
+expressions.
+
+Consider the following examples:
+
+.. code-block:: c++
+
+    int a = 2, b = 6, c = 4;
+    if (a < b < c) {
+        // This block will be executed
+    }
+
+
+In this example, the developer intended to check if ``a`` is less than ``b``
+and ``b`` is less than ``c``. However, the expression ``a < b < c`` is
+equivalent to ``(a < b) < c``. Since ``a < b`` is ``true``, the expression
+``(a < b) < c`` is evaluated as ``1 < c``, which is equivalent to ``true < c``
+and is invalid in this case as ``b < c`` is ``false``.
+
+Even that above issue could be detected as comparison of ``int`` to ``bool``,
+there is more dangerous example:
+
+.. code-block:: c++
+
+    bool a = false, b = false, c = true;
+    if (a == b == c) {
+        // This block will be executed
+    }
+
+In this example, the developer intended to check if ``a``, ``b``, and ``c`` are
+all equal. However, the expression ``a == b == c`` is evaluated as
+``(a == b) == c``. Since ``a == b`` is true, the expression ``(a == b) == c``
+is evaluated as ``true == c``, which is equivalent to ``true == true``.
+This comparison yields ``true``, even though ``a`` and ``b`` are ``false``, and
+are not equal to ``c``.
+
+To avoid this issue, the developer can use a logical operator to separate the
+comparison expressions, like this:
+
+.. code-block:: c++
+
+    if (a == b && b == c) {
+        // This block will not be executed
+    }
+
+
+Alternatively, use of parentheses in the comparison expressions can make the
+developer's intention more explicit and help avoid misunderstanding.
+
+.. code-block:: c++
+
+    if ((a == b) == c) {
+        // This block will be executed
+    }
+

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

@@ -83,6 +83,7 @@ Clang-Tidy Checks
    :doc:`bugprone-bool-pointer-implicit-conversion <bugprone/bool-pointer-implicit-conversion>`, "Yes"
    :doc:`bugprone-branch-clone <bugprone/branch-clone>`,
    :doc:`bugprone-casting-through-void <bugprone/casting-through-void>`,
+   :doc:`bugprone-chained-comparison <bugprone/chained-comparison>`,
    :doc:`bugprone-compare-pointer-to-member-virtual-function <bugprone/compare-pointer-to-member-virtual-function>`,
    :doc:`bugprone-copy-constructor-init <bugprone/copy-constructor-init>`, "Yes"
    :doc:`bugprone-dangling-handle <bugprone/dangling-handle>`,

clang-tools-extra/include-cleaner/test/tool.cpp

@@ -22,6 +22,10 @@ int x = foo();
 // IGNORE2-NOT: - "foobar.h"
 //     IGNORE2: + "foo.h"
 
+//        RUN: clang-include-cleaner -print=changes %s --only-headers="foo\.h" -- -I%S/Inputs/ | FileCheck --match-full-lines --allow-empty --check-prefix=ONLY %s
+//   ONLY-NOT: - "foobar.h"
+//       ONLY: + "foo.h"
+
 //        RUN: clang-include-cleaner -print %s -- -I%S/Inputs/ | FileCheck --match-full-lines --check-prefix=PRINT %s
 //      PRINT: #include "foo.h"
 //  PRINT-NOT: {{^}}#include "foobar.h"{{$}}