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

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:275
+    return false;
+  const auto *Decl = Call.getDecl();
+  if (!Decl)
----------------
xazax.hun wrote:
> Can we model a function call without a declaration? I wonder if we should make this check more eagerly in `evalCall`. 
I think it is not that there is no Decl, but it is more likely the Decl is not available at that time.
```
void foo(void (*bar)(bool, bool)) {
    bar();
}
```
If just foo is analyzed, then there is no Decl.
If a call to foo is analyzed, then there is a Decl.
That said, I think the check can be done in `evalCall` itself. I don't think it will cause a problem.


================
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))
----------------
xazax.hun wrote:
> I'd prefer not repeating the `ModelSmartPtrDereference` check.
I don't think we can do that, since the `isBoolConversion()` check has a branch which is executed when `ModelSmartPtrDereference` is false.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D106296