[clang-tidy] [analyzer] Nondeterministic pointer usage improvements

RFC: This PR is not ready to merge yet, preliminary comments are welcome.
     Here are some questions to consider...

Q) Is the name and file location ok in the directory structure?
   I initially chose bugprone and NondeterministicPointerUsage as a
   starting point for review.
Q) I wrote an explanation for the check based on internal discussion
   since I felt like this was missing. Please check and comment.
Q) There are more ways to iterate over an unordered container
   than captured here. Do those need to be detected as well?
Q) I squashed PointerIteration and PointerSorting. I think that works
   in this case, but interested to hear your comments about that.
Q) I ended up expanding upon the C++ simulation header, but had thoughts
   about breaking that up into multiple smaller files. Maybe there's an
   opportunity to refactor some C++ simulation files across multiple
   checkers as a seperate PR first, or maybe even as part of this one?

This change moves the alpha.nondeterministic.PointerSorting and
alpha.nondeterministic.PointerIteration static analyzer checkers to
a single clang-tidy check. Those checkers were implemented as clang-tidy
checks wrapped in the static analyzer framework. The documentation was
updated to describe what the checks can and cannot do, and testing
was completed on a broad set of open source projects.

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

@@ -48,6 +48,7 @@
 #include "MultipleStatementMacroCheck.h"
 #include "NoEscapeCheck.h"
 #include "NonZeroEnumToBoolConversionCheck.h"
+#include "NondeterministicPointerUsageCheck.h"
 #include "NotNullTerminatedResultCheck.h"
 #include "OptionalValueConversionCheck.h"
 #include "ParentVirtualCallCheck.h"
@@ -179,6 +180,8 @@ class BugproneModule : public ClangTidyModule {
     CheckFactories.registerCheck<cppcoreguidelines::NarrowingConversionsCheck>(
         "bugprone-narrowing-conversions");
     CheckFactories.registerCheck<NoEscapeCheck>("bugprone-no-escape");
+    CheckFactories.registerCheck<NondeterministicPointerUsageCheck>(
+        "bugprone-nondeterministic-pointer-usage");
     CheckFactories.registerCheck<NonZeroEnumToBoolConversionCheck>(
         "bugprone-non-zero-enum-to-bool-conversion");
     CheckFactories.registerCheck<NotNullTerminatedResultCheck>(

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

@@ -44,6 +44,7 @@ add_clang_library(clangTidyBugproneModule
   MultipleNewInOneExpressionCheck.cpp
   MultipleStatementMacroCheck.cpp
   NoEscapeCheck.cpp
+  NondeterministicPointerUsageCheck.cpp
   NonZeroEnumToBoolConversionCheck.cpp
   NotNullTerminatedResultCheck.cpp
   OptionalValueConversionCheck.cpp

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

@@ -0,0 +1,67 @@
+//===--- NondetermnisticPointerUsageCheck.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 "NondeterministicPointerUsageCheck.h"
+#include "clang/AST/ASTContext.h"
+#include "clang/Lex/Lexer.h"
+
+using namespace clang::ast_matchers;
+
+namespace clang::tidy::bugprone {
+
+void NondeterministicPointerUsageCheck::registerMatchers(MatchFinder *Finder) {
+
+  auto LoopVariable = varDecl(hasType(hasCanonicalType(pointerType())));
+
+  auto RangeInit = declRefExpr(to(varDecl(hasType(recordDecl(
+      anyOf(hasName("std::unordered_set"), hasName("std::unordered_map"),
+            hasName("std::unordered_multiset"),
+            hasName("std::unordered_multimap")))))));
+
+  Finder->addMatcher(
+      stmt(cxxForRangeStmt(hasRangeInit(RangeInit.bind("rangeinit")),
+                           hasLoopVariable(LoopVariable.bind("loopVar"))))
+          .bind("cxxForRangeStmt"),
+      this);
+
+  auto SortFuncM = anyOf(callee(functionDecl(hasName("std::is_sorted"))),
+                         callee(functionDecl(hasName("std::nth_element"))),
+                         callee(functionDecl(hasName("std::sort"))),
+                         callee(functionDecl(hasName("std::partial_sort"))),
+                         callee(functionDecl(hasName("std::partition"))),
+                         callee(functionDecl(hasName("std::stable_partition"))),
+                         callee(functionDecl(hasName("std::stable_sort"))));
+
+  auto IteratesPointerEltsM = hasArgument(
+      0,
+      cxxMemberCallExpr(on(hasType(cxxRecordDecl(has(fieldDecl(hasType(
+          hasCanonicalType(pointsTo(hasCanonicalType(pointerType())))))))))));
+
+  Finder->addMatcher(stmt(callExpr(allOf(SortFuncM, IteratesPointerEltsM)))
+                         .bind("sortsemantic"),
+                     this);
+}
+
+void NondeterministicPointerUsageCheck::check(
+    const MatchFinder::MatchResult &Result) {
+  const auto *ForRangePointers =
+      Result.Nodes.getNodeAs<Stmt>("cxxForRangeStmt");
+  const auto *SortPointers = Result.Nodes.getNodeAs<Stmt>("sortsemantic");
+
+  if ((ForRangePointers) && !(ForRangePointers->getBeginLoc().isMacroID())) {
+    const auto *Node = dyn_cast<CXXForRangeStmt>(ForRangePointers);
+    diag(Node->getRParenLoc(), "Iteration of pointers is nondeterministic");
+  }
+
+  if ((SortPointers) && !(SortPointers->getBeginLoc().isMacroID())) {
+    const auto *Node = dyn_cast<Stmt>(SortPointers);
+    diag(Node->getBeginLoc(), "Sorting pointers is nondeterministic");
+  }
+}
+
+} // namespace clang::tidy::bugprone

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

@@ -0,0 +1,36 @@
+//===--- NondeterministicPointerUsageCheck.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__NONDETERMINISTICPOINTERUSAGECHECK_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE__NONDETERMINISTICPOINTERUSAGECHECK_H
+
+#include "../ClangTidyCheck.h"
+
+namespace clang::tidy::bugprone {
+
+/// Finds temporaries that look like RAII objects.
+///
+/// For the user-facing documentation see:
+/// http://clang.llvm.org/extra/clang-tidy/checks/bugprone/nondeterministic-pointer-usage.html
+class NondeterministicPointerUsageCheck : public ClangTidyCheck {
+public:
+  NondeterministicPointerUsageCheck(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;
+  std::optional<TraversalKind> getCheckTraversalKind() const override {
+    return TK_IgnoreUnlessSpelledInSource;
+  }
+};
+
+} // namespace clang::tidy::bugprone
+
+#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_BUGPRONE__NONDETERMINISTICPOINTERUSAGECHECK_H

clang-tools-extra/docs/clang-tidy/checks/bugprone/nondeterministic-pointer-usage.rst

@@ -0,0 +1,31 @@
+.. title:: clang-tidy - bugprone-nondeterministic-pointer-usage
+
+nondeterministic-pointer-usage
+==============================
+
+Finds nondeterministic usages of pointers in unordered containers.
+
+One canonical example is iteration across a container of pointers.
+
+.. code-block:: c++
+
+  {
+    for (auto i : UnorderedPtrSet)
+      f(i);
+  }
+
+Another such example is sorting a container of pointers.
+
+.. code-block:: c++
+
+  {
+    std::sort(VectorOfPtr.begin(), VectorOfPtr.end());
+  }
+
+Iteration of a containers of pointers may present the order of different
+pointers differently across different runs of a program. In some cases this
+may be acceptable behavior, in others this may be unexpected behavior. This
+check is advisory for this reason.
+
+This check only detects range-based for loops over unordered sets. Other
+similar usages will not be found and are false negatives.