[PATCH] D106296: [analyzer] Fix for faulty namespace test in SmartPtrModelling

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun requested changes to this revision.
xazax.hun added a comment.
This revision now requires changes to proceed.

Commented some nits, but overall looks good to me.

However, could you include some tests? We usually do not commit any changes without tests unless it is really hard to create one. But I suspect that this is not the case here.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:275
+    return false;
+  const auto *Decl = Call.getDecl();
+  if (!Decl)
----------------
Can we model a function call without a declaration? I wonder if we should make this check more eagerly in `evalCall`. 


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:291
   // we can try this function
-  if (Call.getNumArgs() == 2 &&
-      Call.getDecl()->getDeclContext()->isStdNamespace())
-    if (smartptr::isStdSmartPtr(Call.getArgExpr(0)) ||
-        smartptr::isStdSmartPtr(Call.getArgExpr(1)))
-      if (handleComparisionOp(Call, C))
-        return true;
-
-  if (isStdOstreamOperatorCall(Call))
+  if (ModelSmartPtrDereference && isPotentiallyComparisionOpCall(Call))
+    if (handleComparisionOp(Call, C))
----------------
I'd prefer not repeating the `ModelSmartPtrDereference` check.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106296/new/

https://reviews.llvm.org/D106296