[clang] [clang][dataflow] Add matchers for smart pointer accessors to be cached (PR #120102)

[Jan Voung via cfe-commits]

================
@@ -0,0 +1,134 @@
+#include "clang/Analysis/FlowSensitive/SmartPointerAccessorCaching.h"
+
+#include "clang/AST/CanonicalType.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Basic/OperatorKinds.h"
+
+namespace clang::dataflow {
+
+namespace {
+
+using ast_matchers::callee;
+using ast_matchers::cxxMemberCallExpr;
+using ast_matchers::cxxMethodDecl;
+using ast_matchers::cxxOperatorCallExpr;
+using ast_matchers::hasName;
+using ast_matchers::hasOverloadedOperatorName;
+using ast_matchers::ofClass;
+using ast_matchers::parameterCountIs;
+using ast_matchers::pointerType;
+using ast_matchers::referenceType;
+using ast_matchers::returns;
+
+bool hasSmartPointerClassShape(const CXXRecordDecl &RD, bool &HasGet,
+                               bool &HasValue) {
+  // We may want to cache this search, but in current profiles it hasn't shown
+  // up as a hot spot (possibly because there aren't many hits, relatively).
+  bool HasArrow = false;
+  bool HasStar = false;
+  CanQualType StarReturnType, ArrowReturnType, GetReturnType, ValueReturnType;
+  for (const auto *MD : RD.methods()) {
+    // We only consider methods that are const and have zero parameters.
+    // It may be that there is a non-const overload for the method, but
+    // there should at least be a const overload as well.
+    if (!MD->isConst() || MD->getNumParams() != 0) {
+      continue;
+    }
+    if (MD->getOverloadedOperator() == OO_Star &&
+        MD->getReturnType()->isReferenceType()) {
+      HasStar = true;
+      StarReturnType = MD->getReturnType()
+                           .getNonReferenceType()
+                           ->getCanonicalTypeUnqualified();
+    } else if (MD->getOverloadedOperator() == OO_Arrow &&
+               MD->getReturnType()->isPointerType()) {
+      HasArrow = true;
+      ArrowReturnType =
+          MD->getReturnType()->getPointeeType()->getCanonicalTypeUnqualified();
+    } else {
+      IdentifierInfo *II = MD->getIdentifier();
+      if (II == nullptr)
+        continue;
+      if (II->isStr("get") && MD->getReturnType()->isPointerType()) {
+        HasGet = true;
+        GetReturnType = MD->getReturnType()
+                            ->getPointeeType()
+                            ->getCanonicalTypeUnqualified();
+      } else if (II->isStr("value") && MD->getReturnType()->isReferenceType()) {
+        HasValue = true;
+        ValueReturnType = MD->getReturnType()
+                              .getNonReferenceType()
+                              ->getCanonicalTypeUnqualified();
+      }
+    }
+  }
+
+  if (!HasStar || !HasArrow || StarReturnType != ArrowReturnType)
+    return false;
+  HasGet = HasGet && (GetReturnType == StarReturnType);
+  HasValue = HasValue && (ValueReturnType == StarReturnType);
+  return true;
+}
+
+} // namespace
+} // namespace clang::dataflow
+
+// AST_MATCHER macros create an "internal" namespace, so we put it in
+// its own anonymous namespace instead of in clang::dataflow.
+namespace {
+
+AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithGet) {
+  bool HasGet = false;
+  bool HasValue = false;
+  bool HasStarAndArrow =
+      clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
+  return HasStarAndArrow && HasGet;
+}
+
+AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithValue) {
+  bool HasGet = false;
+  bool HasValue = false;
+  bool HasStarAndArrow =
+      clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
+  return HasStarAndArrow && HasValue;
+}
+
+AST_MATCHER(clang::CXXRecordDecl, smartPointerClassWithGetOrValue) {
+  bool HasGet = false;
+  bool HasValue = false;
+  bool HasStarAndArrow =
+      clang::dataflow::hasSmartPointerClassShape(Node, HasGet, HasValue);
+  return HasStarAndArrow && (HasGet || HasValue);
+}
+
+} // namespace
+
+namespace clang::dataflow {
+
+ast_matchers::internal::Matcher<Stmt> isSmartPointerLikeOperatorStar() {
----------------
jvoung wrote:

Ah yes, it wasn't clear how these are used! I uploaded a draft of the user to https://github.com/llvm/llvm-project/pull/120249/files#diff-068c9724d855a9686b7ca6573bf3e966d856f2e4200d0eba45db0738e9c9e7afR1038 in case that helps (source examples in the test there).


I'm not too sure how to match the smart pointer class once and reuse (not as familiar with the matcher binding yet!) across different call ASTs, but I can imagine doing a pre-pass that computed smart pointer classes and then use that in the matcher.

So far, I hadn't seen the naive approach be slow in profiles, so I hadn't tried optimizing. If the matchers short circuit, then maybe it doesn't try to do the smart pointer class checks often? For example, if it has to match the 0 param check, and match the name first? With the names being disjoint, on an example call `ptr.get().x.value();` it would match the `hasName("get")` and then do the smart pointer class match on the type of `ptr` but it wouldn't have passed the `hasOverloadedOperatorName("->")` check and wouldn't redundantly try to check the class for that case (or * or value)?

https://github.com/llvm/llvm-project/pull/120102