Merge branch 'main' into clang-tidy-infra

Author: zeyi2

.github/CODEOWNERS

@@ -93,9 +93,9 @@
 /mlir/lib/Dialect/Linalg/Transforms/DecomposeLinalgOps.cpp @MaheshRavishankar @nicolasvasilache
 /mlir/lib/Dialect/Linalg/Transforms/DropUnitDims.cpp @dcaballe @MaheshRavishankar @nicolasvasilache
 /mlir/lib/Dialect/Linalg/Transforms/ElementwiseOpFusion.cpp @MaheshRavishankar @nicolasvasilache
-/mlir/lib/Dialect/Linalg/Transforms/DataLayoutPropagation.cpp @hanhanW @nicolasvasilache
-/mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp @dcaballe @hanhanW @nicolasvasilache
-/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp @banach-space @dcaballe @hanhanW @nicolasvasilache @Groverkss
+/mlir/lib/Dialect/Linalg/Transforms/DataLayoutPropagation.cpp @nicolasvasilache
+/mlir/lib/Dialect/Linalg/Transforms/Transforms.cpp @dcaballe @nicolasvasilache
+/mlir/lib/Dialect/Linalg/Transforms/Vectorization.cpp @banach-space @dcaballe @nicolasvasilache @Groverkss
 
 # MemRef Dialect in MLIR.
 /mlir/lib/Dialect/MemRef/Transforms/EmulateNarrowType.cpp @MaheshRavishankar @nicolasvasilache
@@ -112,16 +112,16 @@
 /mlir/include/mlir/Dialect/Vector @banach-space @dcaballe @nicolasvasilache @Groverkss
 /mlir/include/mlir/Dialect/Vector/IR @kuhar
 /mlir/lib/Dialect/Vector @banach-space @dcaballe @nicolasvasilache @Groverkss
-/mlir/lib/Dialect/Vector/Transforms/* @banach-space @dcaballe @hanhanW @nicolasvasilache
+/mlir/lib/Dialect/Vector/Transforms/* @banach-space @dcaballe @nicolasvasilache
 /mlir/lib/Dialect/Vector/Transforms/VectorEmulateNarrowType.cpp @banach-space @dcaballe @MaheshRavishankar @nicolasvasilache
-/mlir/**/*EmulateNarrowType* @dcaballe @hanhanW
+/mlir/**/*EmulateNarrowType* @dcaballe
 
 # Presburger library in MLIR
 /mlir/**/*Presburger* @Groverkss @Superty
 
 # Tensor Dialect in MLIR.
-/mlir/lib/Dialect/Tensor/IR/TensorTilingInterfaceImpl.cpp @hanhanW @nicolasvasilache
-/mlir/lib/Dialect/Tensor/Transforms/* @hanhanW @nicolasvasilache
+/mlir/lib/Dialect/Tensor/IR/TensorTilingInterfaceImpl.cpp @nicolasvasilache
+/mlir/lib/Dialect/Tensor/Transforms/* @nicolasvasilache
 
 # Transform Dialect in MLIR.
 /mlir/include/mlir/Dialect/Transform/* @ftynse @nicolasvasilache @rolfmorel

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

@@ -19,6 +19,7 @@
 
 #include "../utils/ExprSequence.h"
 #include "../utils/Matchers.h"
+#include "../utils/OptionsUtils.h"
 #include <optional>
 
 using namespace clang::ast_matchers;
@@ -48,7 +49,8 @@ struct UseAfterMove {
 /// various internal helper functions).
 class UseAfterMoveFinder {
 public:
-  UseAfterMoveFinder(ASTContext *TheContext);
+  UseAfterMoveFinder(ASTContext *TheContext,
+                     llvm::ArrayRef<StringRef> InvalidationFunctions);
 
   // Within the given code block, finds the first use of 'MovedVariable' that
   // occurs after 'MovingCall' (the expression that performs the move). If a
@@ -71,13 +73,19 @@ class UseAfterMoveFinder {
                   llvm::SmallPtrSetImpl<const DeclRefExpr *> *DeclRefs);
 
   ASTContext *Context;
+  llvm::ArrayRef<StringRef> InvalidationFunctions;
   std::unique_ptr<ExprSequence> Sequence;
   std::unique_ptr<StmtToBlockMap> BlockMap;
   llvm::SmallPtrSet<const CFGBlock *, 8> Visited;
 };
 
 } // namespace
 
+static auto getNameMatcher(llvm::ArrayRef<StringRef> InvalidationFunctions) {
+  return anyOf(hasAnyName("::std::move", "::std::forward"),
+               matchers::matchesAnyListedName(InvalidationFunctions));
+}
+
 // Matches nodes that are
 // - Part of a decltype argument or class template argument (we check this by
 //   seeing if they are children of a TypeLoc), or
@@ -92,8 +100,9 @@ static StatementMatcher inDecltypeOrTemplateArg() {
                hasAncestor(expr(hasUnevaluatedContext())));
 }
 
-UseAfterMoveFinder::UseAfterMoveFinder(ASTContext *TheContext)
-    : Context(TheContext) {}
+UseAfterMoveFinder::UseAfterMoveFinder(
+    ASTContext *TheContext, llvm::ArrayRef<StringRef> InvalidationFunctions)
+    : Context(TheContext), InvalidationFunctions(InvalidationFunctions) {}
 
 std::optional<UseAfterMove>
 UseAfterMoveFinder::find(Stmt *CodeBlock, const Expr *MovingCall,
@@ -359,7 +368,7 @@ void UseAfterMoveFinder::getReinits(
                             unless(parmVarDecl(hasType(
                                 references(qualType(isConstQualified())))))),
                         unless(callee(functionDecl(
-                            hasAnyName("::std::move", "::std::forward")))))))
+                            getNameMatcher(InvalidationFunctions)))))))
           .bind("reinit");
 
   Stmts->clear();
@@ -388,18 +397,21 @@ void UseAfterMoveFinder::getReinits(
   }
 }
 
-enum class MoveType {
-  Move,    // std::move
-  Forward, // std::forward
+enum MoveType {
+  Forward = 0,      // std::forward
+  Move = 1,         // std::move
+  Invalidation = 2, // other
 };
 
 static MoveType determineMoveType(const FunctionDecl *FuncDecl) {
-  if (FuncDecl->getName() == "move")
-    return MoveType::Move;
-  if (FuncDecl->getName() == "forward")
-    return MoveType::Forward;
+  if (FuncDecl->isInStdNamespace()) {
+    if (FuncDecl->getName() == "move")
+      return MoveType::Move;
+    if (FuncDecl->getName() == "forward")
+      return MoveType::Forward;
+  }
 
-  llvm_unreachable("Invalid move type");
+  return MoveType::Invalidation;
 }
 
 static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg,
@@ -408,41 +420,53 @@ static void emitDiagnostic(const Expr *MovingCall, const DeclRefExpr *MoveArg,
   const SourceLocation UseLoc = Use.DeclRef->getExprLoc();
   const SourceLocation MoveLoc = MovingCall->getExprLoc();
 
-  const bool IsMove = (Type == MoveType::Move);
-
-  Check->diag(UseLoc, "'%0' used after it was %select{forwarded|moved}1")
-      << MoveArg->getDecl()->getName() << IsMove;
-  Check->diag(MoveLoc, "%select{forward|move}0 occurred here",
+  Check->diag(UseLoc,
+              "'%0' used after it was %select{forwarded|moved|invalidated}1")
+      << MoveArg->getDecl()->getName() << Type;
+  Check->diag(MoveLoc, "%select{forward|move|invalidation}0 occurred here",
               DiagnosticIDs::Note)
-      << IsMove;
+      << Type;
   if (Use.EvaluationOrderUndefined) {
     Check->diag(
         UseLoc,
-        "the use and %select{forward|move}0 are unsequenced, i.e. "
+        "the use and %select{forward|move|invalidation}0 are unsequenced, i.e. "
         "there is no guarantee about the order in which they are evaluated",
         DiagnosticIDs::Note)
-        << IsMove;
+        << Type;
   } else if (Use.UseHappensInLaterLoopIteration) {
     Check->diag(UseLoc,
                 "the use happens in a later loop iteration than the "
-                "%select{forward|move}0",
+                "%select{forward|move|invalidation}0",
                 DiagnosticIDs::Note)
-        << IsMove;
+        << Type;
   }
 }
 
+UseAfterMoveCheck::UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context)
+    : ClangTidyCheck(Name, Context),
+      InvalidationFunctions(utils::options::parseStringList(
+          Options.get("InvalidationFunctions", ""))) {}
+
+void UseAfterMoveCheck::storeOptions(ClangTidyOptions::OptionMap &Opts) {
+  Options.store(Opts, "InvalidationFunctions",
+                utils::options::serializeStringList(InvalidationFunctions));
+}
+
 void UseAfterMoveCheck::registerMatchers(MatchFinder *Finder) {
   // try_emplace is a common maybe-moving function that returns a
   // bool to tell callers whether it moved. Ignore std::move inside
   // try_emplace to avoid false positives as we don't track uses of
   // the bool.
   auto TryEmplaceMatcher =
       cxxMemberCallExpr(callee(cxxMethodDecl(hasName("try_emplace"))));
+  auto Arg = declRefExpr().bind("arg");
+  auto IsMemberCallee = callee(functionDecl(unless(isStaticStorageClass())));
   auto CallMoveMatcher =
-      callExpr(argumentCountIs(1),
-               callee(functionDecl(hasAnyName("::std::move", "::std::forward"))
+      callExpr(callee(functionDecl(getNameMatcher(InvalidationFunctions))
                           .bind("move-decl")),
-               hasArgument(0, declRefExpr().bind("arg")),
+               anyOf(cxxMemberCallExpr(IsMemberCallee, on(Arg)),
+                     callExpr(unless(cxxMemberCallExpr(IsMemberCallee)),
+                              hasArgument(0, Arg))),
                unless(inDecltypeOrTemplateArg()),
                unless(hasParent(TryEmplaceMatcher)), expr().bind("call-move"),
                anyOf(hasAncestor(compoundStmt(
@@ -521,7 +545,7 @@ void UseAfterMoveCheck::check(const MatchFinder::MatchResult &Result) {
   }
 
   for (Stmt *CodeBlock : CodeBlocks) {
-    UseAfterMoveFinder Finder(Result.Context);
+    UseAfterMoveFinder Finder(Result.Context, InvalidationFunctions);
     if (auto Use = Finder.find(CodeBlock, MovingCall, Arg))
       emitDiagnostic(MovingCall, Arg, *Use, this, Result.Context,
                      determineMoveType(MoveDecl));

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

@@ -20,13 +20,16 @@ namespace clang::tidy::bugprone {
 /// https://clang.llvm.org/extra/clang-tidy/checks/bugprone/use-after-move.html
 class UseAfterMoveCheck : public ClangTidyCheck {
 public:
-  UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context)
-      : ClangTidyCheck(Name, Context) {}
+  UseAfterMoveCheck(StringRef Name, ClangTidyContext *Context);
+  void storeOptions(ClangTidyOptions::OptionMap &Opts) override;
   bool isLanguageVersionSupported(const LangOptions &LangOpts) const override {
     return LangOpts.CPlusPlus11;
   }
   void registerMatchers(ast_matchers::MatchFinder *Finder) override;
   void check(const ast_matchers::MatchFinder::MatchResult &Result) override;
+
+private:
+  std::vector<StringRef> InvalidationFunctions;
 };
 
 } // namespace clang::tidy::bugprone

clang-tools-extra/clang-tidy/readability/UseStdMinMaxCheck.cpp

@@ -96,12 +96,11 @@ static QualType getReplacementCastType(const Expr *CondLhs, const Expr *CondRhs,
   return GlobalImplicitCastType;
 }
 
-static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs,
-                                     const Expr *AssignLhs,
-                                     const SourceManager &Source,
-                                     const LangOptions &LO,
-                                     StringRef FunctionName,
-                                     const BinaryOperator *BO) {
+static std::string
+createReplacement(const Expr *CondLhs, const Expr *CondRhs,
+                  const Expr *AssignLhs, const SourceManager &Source,
+                  const LangOptions &LO, StringRef FunctionName,
+                  const BinaryOperator *BO, StringRef Comment = "") {
   const llvm::StringRef CondLhsStr = Lexer::getSourceText(
       Source.getExpansionRange(CondLhs->getSourceRange()), Source, LO);
   const llvm::StringRef CondRhsStr = Lexer::getSourceText(
@@ -116,7 +115,8 @@ static std::string createReplacement(const Expr *CondLhs, const Expr *CondRhs,
           (!GlobalImplicitCastType.isNull()
                ? "<" + GlobalImplicitCastType.getAsString() + ">("
                : "(") +
-          CondLhsStr + ", " + CondRhsStr + ");")
+          CondLhsStr + ", " + CondRhsStr + ");" + (Comment.empty() ? "" : " ") +
+          Comment)
       .str();
 }
 
@@ -172,13 +172,65 @@ void UseStdMinMaxCheck::check(const MatchFinder::MatchResult &Result) {
 
   auto ReplaceAndDiagnose = [&](const llvm::StringRef FunctionName) {
     const SourceManager &Source = *Result.SourceManager;
+    llvm::SmallString<64> Comment;
+
+    const auto AppendNormalized = [&](llvm::StringRef Text) {
+      Text = Text.ltrim();
+      if (!Text.empty()) {
+        if (!Comment.empty())
+          Comment += " ";
+        Comment += Text;
+      }
+    };
+
+    const auto GetSourceText = [&](SourceLocation StartLoc,
+                                   SourceLocation EndLoc) {
+      return Lexer::getSourceText(
+          CharSourceRange::getCharRange(
+              Lexer::getLocForEndOfToken(StartLoc, 0, Source, LO), EndLoc),
+          Source, LO);
+    };
+
+    // Captures:
+    // if (cond) // Comment A
+    // if (cond) /* Comment A */ { ... }
+    // if (cond) /* Comment A */ x = y;
+    AppendNormalized(
+        GetSourceText(If->getRParenLoc(), If->getThen()->getBeginLoc()));
+
+    if (const auto *CS = dyn_cast<CompoundStmt>(If->getThen())) {
+      const Stmt *Inner = CS->body_front();
+
+      // Captures:
+      // if (cond) { // Comment B
+      // ...
+      // }
+      // if (cond) { /* Comment B */ x = y; }
+      AppendNormalized(GetSourceText(CS->getBeginLoc(), Inner->getBeginLoc()));
+
+      // Captures:
+      // if (cond) { x = y; // Comment C }
+      // if (cond) { x = y; /* Comment C */ }
+      llvm::StringRef PostInner =
+          GetSourceText(Inner->getEndLoc(), CS->getEndLoc());
+
+      // Strip the trailing semicolon to avoid fixes like:
+      // x = std::min(x, y);; // comment
+      const size_t Semi = PostInner.find(';');
+      if (Semi != llvm::StringRef::npos &&
+          PostInner.take_front(Semi).trim().empty()) {
+        PostInner = PostInner.drop_front(Semi + 1);
+      }
+      AppendNormalized(PostInner);
+    }
+
     diag(IfLocation, "use `%0` instead of `%1`")
         << FunctionName << BinaryOp->getOpcodeStr()
         << FixItHint::CreateReplacement(
                SourceRange(IfLocation, Lexer::getLocForEndOfToken(
                                            ThenLocation, 0, Source, LO)),
                createReplacement(CondLhs, CondRhs, AssignLhs, Source, LO,
-                                 FunctionName, BinaryOp))
+                                 FunctionName, BinaryOp, Comment))
         << IncludeInserter.createIncludeInsertion(
                Source.getFileID(If->getBeginLoc()), AlgorithmHeader);
   };

clang-tools-extra/docs/ReleaseNotes.rst

@@ -408,6 +408,10 @@ Changes in existing checks
   suffix when the reason starts with the character `>` in the `CustomFunctions`
   option.
 
+- Improved :doc:`bugprone-use-after-move
+  <clang-tidy/checks/bugprone/use-after-move>` check by adding
+  `InvalidationFunctions` option to support custom invalidation functions.
+
 - Improved :doc:`cppcoreguidelines-avoid-non-const-global-variables
   <clang-tidy/checks/cppcoreguidelines/avoid-non-const-global-variables>` check
   by adding a new option `AllowThreadLocal` that suppresses warnings on
@@ -579,6 +583,11 @@ Changes in existing checks
   <clang-tidy/checks/readability/use-concise-preprocessor-directives>` check to
   generate correct fix-its for forms without a space after the directive.
 
+- Improved :doc:`readability-use-std-min-max
+  <clang-tidy/checks/readability/use-std-min-max>` check by ensuring that
+  comments between the ``if`` condition and the ``then`` block are preserved
+  when applying the fix.
+
 Removed checks
 ^^^^^^^^^^^^^^
 

clang-tools-extra/docs/clang-tidy/checks/bugprone/use-after-move.rst

@@ -253,3 +253,13 @@ For example, if an additional member variable is added to ``S``, it is easy to
 forget to add the reinitialization for this additional member. Instead, it is
 safer to assign to the entire struct in one go, and this will also avoid the
 use-after-move warning.
+
+Options
+-------
+
+.. option:: InvalidationFunctions
+
+  A semicolon-separated list of names of functions that cause their first
+  arguments to be invalidated (e.g., closing a handle).
+  For member functions, the first argument is considered to be the implicit
+  object argument (``this``). Default value is an empty string.