merge main into amd-staging

Jenkins · Jenkins · commit 47b6a30e42df · 2024-01-14T21:35:37.000Z
Change-Id: I2cd6079ebcf03d73978a89cbb86e45f76567baea
diff --git a/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp b/clang-tools-extra/clang-tidy/readability/ContainerSizeEmptyCheck.cpp
@@ -291,10 +291,14 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
                        OpCode == BinaryOperatorKind::BO_NE))
       return;
 
-    // Always true, no warnings for that.
-    if ((OpCode == BinaryOperatorKind::BO_GE && Value == 0 && ContainerIsLHS) ||
-        (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && !ContainerIsLHS))
-      return;
+    // Always true/false, no warnings for that.
+    if (Value == 0) {
+      if ((OpCode == BinaryOperatorKind::BO_GT && !ContainerIsLHS) ||
+          (OpCode == BinaryOperatorKind::BO_LT && ContainerIsLHS) ||
+          (OpCode == BinaryOperatorKind::BO_LE && !ContainerIsLHS) ||
+          (OpCode == BinaryOperatorKind::BO_GE && ContainerIsLHS))
+        return;
+    }
 
     // Do not warn for size > 1, 1 < size, size <= 1, 1 >= size.
     if (Value == 1) {
@@ -306,12 +310,32 @@ void ContainerSizeEmptyCheck::check(const MatchFinder::MatchResult &Result) {
         return;
     }
 
+    // Do not warn for size < 1, 1 > size, size <= 0, 0 >= size for non signed
+    // types
+    if ((OpCode == BinaryOperatorKind::BO_GT && Value == 1 &&
+         !ContainerIsLHS) ||
+        (OpCode == BinaryOperatorKind::BO_LT && Value == 1 && ContainerIsLHS) ||
+        (OpCode == BinaryOperatorKind::BO_GE && Value == 0 &&
+         !ContainerIsLHS) ||
+        (OpCode == BinaryOperatorKind::BO_LE && Value == 0 && ContainerIsLHS)) {
+      const Expr *Container = ContainerIsLHS
+                                  ? BinaryOp->getLHS()->IgnoreImpCasts()
+                                  : BinaryOp->getRHS()->IgnoreImpCasts();
+      if (Container->getType()
+              .getCanonicalType()
+              .getNonReferenceType()
+              ->isSignedIntegerType())
+        return;
+    }
+
     if (OpCode == BinaryOperatorKind::BO_NE && Value == 0)
       Negation = true;
+
     if ((OpCode == BinaryOperatorKind::BO_GT ||
          OpCode == BinaryOperatorKind::BO_GE) &&
         ContainerIsLHS)
       Negation = true;
+
     if ((OpCode == BinaryOperatorKind::BO_LT ||
          OpCode == BinaryOperatorKind::BO_LE) &&
         !ContainerIsLHS)
diff --git a/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp b/clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.cpp
@@ -7,6 +7,7 @@
 //===----------------------------------------------------------------------===//
 
 #include "RedundantMemberInitCheck.h"
+#include "../utils/LexerUtils.h"
 #include "../utils/Matchers.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
@@ -18,52 +19,80 @@ using namespace clang::tidy::matchers;
 
 namespace clang::tidy::readability {
 
+static SourceRange
+getFullInitRangeInclWhitespaces(SourceRange Range, const SourceManager &SM,
+                                const LangOptions &LangOpts) {
+  const Token PrevToken =
+      utils::lexer::getPreviousToken(Range.getBegin(), SM, LangOpts, false);
+  if (PrevToken.is(tok::unknown))
+    return Range;
+
+  if (PrevToken.isNot(tok::equal))
+    return {PrevToken.getEndLoc(), Range.getEnd()};
+
+  return getFullInitRangeInclWhitespaces(
+      {PrevToken.getLocation(), Range.getEnd()}, SM, LangOpts);
+}
+
 void RedundantMemberInitCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
   Options.store(Opts, "IgnoreBaseInCopyConstructors",
                 IgnoreBaseInCopyConstructors);
 }
 
 void RedundantMemberInitCheck::registerMatchers(MatchFinder *Finder) {
+  auto ConstructorMatcher =
+      cxxConstructExpr(argumentCountIs(0),
+                       hasDeclaration(cxxConstructorDecl(ofClass(cxxRecordDecl(
+                           unless(isTriviallyDefaultConstructible()))))))
+          .bind("construct");
+
   Finder->addMatcher(
       cxxConstructorDecl(
           unless(isDelegatingConstructor()), ofClass(unless(isUnion())),
           forEachConstructorInitializer(
-              cxxCtorInitializer(
-                  withInitializer(
-                      cxxConstructExpr(
-                          hasDeclaration(
-                              cxxConstructorDecl(ofClass(cxxRecordDecl(
-                                  unless(isTriviallyDefaultConstructible()))))))
-                          .bind("construct")),
-                  unless(forField(hasType(isConstQualified()))),
-                  unless(forField(hasParent(recordDecl(isUnion())))))
+              cxxCtorInitializer(withInitializer(ConstructorMatcher),
+                                 unless(forField(fieldDecl(
+                                     anyOf(hasType(isConstQualified()),
+                                           hasParent(recordDecl(isUnion())))))))
                   .bind("init")))
           .bind("constructor"),
       this);
+
+  Finder->addMatcher(fieldDecl(hasInClassInitializer(ConstructorMatcher),
+                               unless(hasParent(recordDecl(isUnion()))))
+                         .bind("field"),
+                     this);
 }
 
 void RedundantMemberInitCheck::check(const MatchFinder::MatchResult &Result) {
-  const auto *Init = Result.Nodes.getNodeAs<CXXCtorInitializer>("init");
   const auto *Construct = Result.Nodes.getNodeAs<CXXConstructExpr>("construct");
+
+  if (const auto *Field = Result.Nodes.getNodeAs<FieldDecl>("field")) {
+    const Expr *Init = Field->getInClassInitializer();
+    diag(Construct->getExprLoc(), "initializer for member %0 is redundant")
+        << Field
+        << FixItHint::CreateRemoval(getFullInitRangeInclWhitespaces(
+               Init->getSourceRange(), *Result.SourceManager, getLangOpts()));
+    return;
+  }
+
+  const auto *Init = Result.Nodes.getNodeAs<CXXCtorInitializer>("init");
   const auto *ConstructorDecl =
       Result.Nodes.getNodeAs<CXXConstructorDecl>("constructor");
 
   if (IgnoreBaseInCopyConstructors && ConstructorDecl->isCopyConstructor() &&
       Init->isBaseInitializer())
     return;
 
-  if (Construct->getNumArgs() == 0 ||
-      Construct->getArg(0)->isDefaultArgument()) {
-    if (Init->isAnyMemberInitializer()) {
-      diag(Init->getSourceLocation(), "initializer for member %0 is redundant")
-          << Init->getAnyMember()
-          << FixItHint::CreateRemoval(Init->getSourceRange());
-    } else {
-      diag(Init->getSourceLocation(),
-           "initializer for base class %0 is redundant")
-          << Construct->getType()
-          << FixItHint::CreateRemoval(Init->getSourceRange());
-    }
+  if (Init->isAnyMemberInitializer()) {
+    diag(Init->getSourceLocation(), "initializer for member %0 is redundant")
+        << Init->getAnyMember()
+        << FixItHint::CreateRemoval(Init->getSourceRange());
+  } else {
+    diag(Init->getSourceLocation(),
+         "initializer for base class %0 is redundant")
+        << Construct->getType()
+        << FixItHint::CreateRemoval(Init->getSourceRange());
   }
 }
 
diff --git a/clang-tools-extra/docs/ReleaseNotes.rst b/clang-tools-extra/docs/ReleaseNotes.rst
@@ -467,7 +467,9 @@ Changes in existing checks
 - Improved :doc:`readability-container-size-empty
   <clang-tidy/checks/readability/container-size-empty>` check to
   detect comparison between string and empty string literals and support
-  ``length()`` method as an alternative to ``size()``.
+  ``length()`` method as an alternative to ``size()``. Resolved false positives
+  tied to negative values from size-like methods, and one triggered by size
+  checks below zero.
 
 - Improved :doc:`readability-function-size
   <clang-tidy/checks/readability/function-size>` check configuration to use
@@ -501,6 +503,10 @@ Changes in existing checks
   <clang-tidy/checks/readability/non-const-parameter>` check to ignore
   false-positives in initializer list of record.
 
+- Improved :doc:`readability-redundant-member-init
+  <clang-tidy/checks/readability/redundant-member-init>` check to now also
+  detect redundant in-class initializers.
+
 - Improved :doc:`readability-simplify-boolean-expr
   <clang-tidy/checks/readability/simplify-boolean-expr>` check by adding the
   new option `IgnoreMacros` that allows to ignore boolean expressions originating
diff --git a/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst b/clang-tools-extra/docs/clang-tidy/checks/readability/redundant-member-init.rst
@@ -11,13 +11,14 @@ Example
 
 .. code-block:: c++
 
-  // Explicitly initializing the member s is unnecessary.
+  // Explicitly initializing the member s and v is unnecessary.
   class Foo {
   public:
     Foo() : s() {}
 
   private:
     std::string s;
+    std::vector<int> v {};
   };
 
 Options
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/container-size-empty.cpp
@@ -33,7 +33,7 @@ class TemplatedContainer {
 public:
   bool operator==(const TemplatedContainer<T>& other) const;
   bool operator!=(const TemplatedContainer<T>& other) const;
-  int size() const;
+  unsigned long size() const;
   bool empty() const;
 };
 
@@ -42,7 +42,7 @@ class PrivateEmpty {
 public:
   bool operator==(const PrivateEmpty<T>& other) const;
   bool operator!=(const PrivateEmpty<T>& other) const;
-  int size() const;
+  unsigned long size() const;
 private:
   bool empty() const;
 };
@@ -61,7 +61,7 @@ struct EnumSize {
 class Container {
 public:
   bool operator==(const Container& other) const;
-  int size() const;
+  unsigned long size() const;
   bool empty() const;
 };
 
@@ -70,13 +70,13 @@ class Derived : public Container {
 
 class Container2 {
 public:
-  int size() const;
+  unsigned long size() const;
   bool empty() const { return size() == 0; }
 };
 
 class Container3 {
 public:
-  int size() const;
+  unsigned long size() const;
   bool empty() const;
 };
 
@@ -85,7 +85,7 @@ bool Container3::empty() const { return this->size() == 0; }
 class Container4 {
 public:
   bool operator==(const Container4& rhs) const;
-  int size() const;
+  unsigned long size() const;
   bool empty() const { return *this == Container4(); }
 };
 
@@ -815,3 +815,49 @@ bool testNotEmptyStringLiterals(const std::string& s)
   using namespace std::string_literals;
   return s == "foo"s;
 }
+
+namespace PR72619 {
+  struct SS {
+    bool empty() const;
+    int size() const;
+  };
+
+  struct SU {
+    bool empty() const;
+    unsigned size() const;
+  };
+
+  void f(const SU& s) {
+    if (s.size() < 0) {}
+    if (0 > s.size()) {}
+    if (s.size() >= 0) {}
+    if (0 <= s.size()) {}
+    if (s.size() < 1)
+      ;
+    // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+    // CHECK-FIXES: {{^    }}if (s.empty()){{$}}
+    if (1 > s.size())
+      ;
+    // CHECK-MESSAGES: :[[@LINE-2]]:13: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+    // CHECK-FIXES: {{^    }}if (s.empty()){{$}}
+    if (s.size() <= 0)
+      ;
+    // CHECK-MESSAGES: :[[@LINE-2]]:9: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+    // CHECK-FIXES: {{^    }}if (s.empty()){{$}}
+    if (0 >= s.size())
+      ;
+    // CHECK-MESSAGES: :[[@LINE-2]]:14: warning: the 'empty' method should be used to check for emptiness instead of 'size' [readability-container-size-empty]
+    // CHECK-FIXES: {{^    }}if (s.empty()){{$}}
+  }
+
+  void f(const SS& s) {
+    if (s.size() < 0) {}
+    if (0 > s.size()) {}
+    if (s.size() >= 0) {}
+    if (0 <= s.size()) {}
+    if (s.size() < 1) {}
+    if (1 > s.size()) {}
+    if (s.size() <= 0) {}
+    if (0 >= s.size()) {}
+  }
+}
diff --git a/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp b/clang-tools-extra/test/clang-tidy/checkers/readability/redundant-member-init.cpp
@@ -250,3 +250,55 @@ struct NF15 {
     S s2;
   };
 };
+
+// Direct in-class initialization with default constructor
+struct D1 {
+  S f1 {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: initializer for member 'f1' is redundant
+  // CHECK-FIXES: S f1;
+};
+
+// Direct in-class initialization with constructor with default argument
+struct D2 {
+  T f2  {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: initializer for member 'f2' is redundant
+  // CHECK-FIXES: T f2;
+};
+
+// Direct in-class initialization with default constructor (assign)
+struct D3 {
+  S f3 = {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: initializer for member 'f3' is redundant
+  // CHECK-FIXES: S f3;
+};
+
+// Direct in-class initialization with constructor with default argument (assign)
+struct D4 {
+  T f4 = {};
+  // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: initializer for member 'f4' is redundant
+  // CHECK-FIXES: T f4;
+};
+
+// Templated class independent type
+template <class V>
+struct D5 {
+  S f5 /*comment*/ = S();
+  // CHECK-MESSAGES: :[[@LINE-1]]:22: warning: initializer for member 'f5' is redundant
+  // CHECK-FIXES: S f5 /*comment*/;
+};
+D5<int> d5i;
+D5<S> d5s;
+
+struct D6 {
+  UsesCleanup uc2{};
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: initializer for member 'uc2' is redundant
+  // CHECK-FIXES: UsesCleanup uc2;
+};
+
+template<typename V>
+struct D7 {
+  V f7;
+};
+
+D7<int> d7i;
+D7<S> d7s;
diff --git a/llvm/include/llvm/Analysis/DOTGraphTraitsPass.h b/llvm/include/llvm/Analysis/DOTGraphTraitsPass.h
