[webkit.UncountedLambdaCapturesChecker] Treat arguments of std::ranges::all_of as [[clang::noescape]]

[rniwa]

The checker already had std::ranges hard-coded to treat its arguments as [[clang::oescape]] but the fact std::ranges::all_of is implemented as a struct instead of a function confused the checker and resuled in a superflous warning being emitted for std::ranges::all_of.

This PR adds the support for recognizing DeclRefExpr which appears as a callee in VisitCallExpr and generalizes the check in shouldTreatAllArgAsNoEscape to walk up the decl contexts to find the target namespaces such as std::ranges:: or a namespace and a function like WTF::switchOn.

[llvmbot]

@llvm/pr-subscribers-clang

@llvm/pr-subscribers-clang-static-analyzer-1

Author: Ryosuke Niwa (rniwa)

Changes

The checker already had std::ranges hard-coded to treat its arguments as [[clang::oescape]] but the fact std::ranges::all_of is implemented as a struct instead of a function confused the checker and resuled in a superflous warning being emitted for std::ranges::all_of.

This PR adds the support for recognizing DeclRefExpr which appears as a callee in VisitCallExpr and generalizes the check in shouldTreatAllArgAsNoEscape to walk up the decl contexts to find the target namespaces such as std::ranges:: or a namespace and a function like WTF::switchOn.

---

Full diff: https://github.com/llvm/llvm-project/pull/158419.diff

2 Files Affected:

• (modified) clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp (+36-27)
• (modified) clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp (+32-3)

diff --git a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
index 03eeb9999c4dd..c4f42b8ac3407 100644
--- a/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
+++ b/clang/lib/StaticAnalyzer/Checkers/WebKit/RawPtrRefLambdaCapturesChecker.cpp
@@ -126,20 +126,20 @@ class RawPtrRefLambdaCapturesChecker
         return true;
       }
 
-      bool shouldTreatAllArgAsNoEscape(FunctionDecl *Decl) {
-        auto *NsDecl = Decl->getParent();
-        if (!NsDecl || !isa<NamespaceDecl>(NsDecl))
-          return false;
-        // WTF::switchOn(T, F... f) is a variadic template function and couldn't
-        // be annotated with NOESCAPE. We hard code it here to workaround that.
-        if (safeGetName(NsDecl) == "WTF" && safeGetName(Decl) == "switchOn")
-          return true;
-        // Treat every argument of functions in std::ranges as noescape.
-        if (safeGetName(NsDecl) == "ranges") {
-          if (auto *OuterDecl = NsDecl->getParent();
-              OuterDecl && isa<NamespaceDecl>(OuterDecl) &&
-              safeGetName(OuterDecl) == "std")
+      bool shouldTreatAllArgAsNoEscape(FunctionDecl *FDecl) {
+        std::string PreviousName = safeGetName(FDecl);
+        for (auto *Decl = FDecl->getParent(); Decl; Decl = Decl->getParent()) {
+          if (!isa<NamespaceDecl>(Decl) && !isa<CXXRecordDecl>(Decl))
+            return false;
+          auto Name = safeGetName(Decl);
+          // WTF::switchOn(T, F... f) is a variadic template function and couldn't
+          // be annotated with NOESCAPE. We hard code it here to workaround that.
+          if (Name == "WTF" && PreviousName == "switchOn")
             return true;
+          // Treat every argument of functions in std::ranges as noescape.
+          if (Name == "std" && PreviousName == "ranges")
+            return true;
+          PreviousName = Name;
         }
         return false;
       }
@@ -167,25 +167,34 @@ class RawPtrRefLambdaCapturesChecker
 
       bool VisitCallExpr(CallExpr *CE) override {
         checkCalleeLambda(CE);
-        if (auto *Callee = CE->getDirectCallee()) {
-          unsigned ArgIndex = isa<CXXOperatorCallExpr>(CE);
-          bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
-          for (auto *Param : Callee->parameters()) {
-            if (ArgIndex >= CE->getNumArgs())
-              return true;
-            auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
-            if (auto *L = findLambdaInArg(Arg)) {
-              LambdasToIgnore.insert(L);
-              if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape)
-                Checker->visitLambdaExpr(
-                    L, shouldCheckThis() && !hasProtectedThis(L), ClsType);
-            }
-            ++ArgIndex;
+        if (auto *Callee = CE->getDirectCallee())
+          checkParameters(CE, Callee);
+        else if (auto *CalleeE = CE->getCallee()) {
+          if (auto *DRE = dyn_cast<DeclRefExpr>(CalleeE->IgnoreParenCasts())) {
+            if (auto *Callee = dyn_cast_or_null<FunctionDecl>(DRE->getDecl()))
+              checkParameters(CE, Callee);
           }
         }
         return true;
       }
 
+      void checkParameters(CallExpr* CE, FunctionDecl *Callee) {
+        unsigned ArgIndex = isa<CXXOperatorCallExpr>(CE);
+        bool TreatAllArgsAsNoEscape = shouldTreatAllArgAsNoEscape(Callee);
+        for (auto *Param : Callee->parameters()) {
+          if (ArgIndex >= CE->getNumArgs())
+            return;
+          auto *Arg = CE->getArg(ArgIndex)->IgnoreParenCasts();
+          if (auto *L = findLambdaInArg(Arg)) {
+            LambdasToIgnore.insert(L);
+            if (!Param->hasAttr<NoEscapeAttr>() && !TreatAllArgsAsNoEscape)
+              Checker->visitLambdaExpr(
+                  L, shouldCheckThis() && !hasProtectedThis(L), ClsType);
+          }
+          ++ArgIndex;
+        }
+      }
+
       LambdaExpr *findLambdaInArg(Expr *E) {
         if (auto *Lambda = dyn_cast_or_null<LambdaExpr>(E))
           return Lambda;
diff --git a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
index 0b8af0d1e8dc1..a4ad741182f56 100644
--- a/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
+++ b/clang/test/Analysis/Checkers/WebKit/uncounted-lambda-captures.cpp
@@ -17,6 +17,18 @@ void for_each(IteratorType first, IteratorType last, CallbackType callback) {
     callback(*it);
 }
 
+struct all_of_impl {
+  template <typename Collection, typename Predicate>
+  constexpr bool operator()(const Collection& collection, Predicate predicate) const {
+    for (auto it = collection.begin(); it != collection.end(); ++it) {
+      if (!predicate(*it))
+        return false;
+    }
+    return true;
+  }
+};
+inline constexpr auto all_of = all_of_impl {};
+
 }
 
 }
@@ -435,7 +447,7 @@ class Iterator {
   bool operator==(const Iterator&);
 
   Iterator& operator++();
-  void* operator*();
+  int& operator*();
 
 private:
   void* current { nullptr };
@@ -444,22 +456,39 @@ class Iterator {
 
 void ranges_for_each(RefCountable* obj) {
   int array[] = { 1, 2, 3, 4, 5 };
-  std::ranges::for_each(Iterator(array, sizeof(*array), 0), Iterator(array, sizeof(*array), 5), [&](void* item) {
+  std::ranges::for_each(Iterator(array, sizeof(*array), 0), Iterator(array, sizeof(*array), 5), [&](int& item) {
     obj->method();
-    ++(*static_cast<unsigned*>(item));
+    ++item;
   });
 }
 
+class IntCollection {
+public:
+  int* begin();
+  int* end();
+  const int* begin() const;
+  const int* end() const;
+};
+
 class RefCountedObj {
 public:
   void ref();
   void deref();
 
+  bool allOf(const IntCollection&);
+  bool isMatch(int);
+
   void call() const;
   void callLambda([[clang::noescape]] const WTF::Function<void ()>& callback) const;
   void doSomeWork() const;
 };
 
+bool RefCountedObj::allOf(const IntCollection& collection) {
+  return std::ranges::all_of(collection, [&](auto& number) {
+    return isMatch(number);
+  });
+}
+
 void RefCountedObj::callLambda([[clang::noescape]] const WTF::Function<void ()>& callback) const
 {
     callback();

[github-actions]

✅ With the latest revision this PR passed the C/C++ code formatter.

[t-rasmud]

LGTM!

[rniwa]

Thanks for the review!