[PATCH] D81315: [analyzer] Warning for default constructed unique pointer dereferences

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

xazax.hun added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:139
+
+  const auto *MethodDecl = dyn_cast_or_null<CXXMethodDecl>(OC->getDecl());
+
----------------
You should never get null here due to `isStdSmartPointerClass` guarding above. I think the null check could be transformed into an assert.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:141
+
+  if (!MethodDecl || !MethodDecl->isOverloadedOperator())
+    return;
----------------
You have a `CXXMemberOperatorCall` which already represents an overloaded operator. This check is either redundant or could be an assert instead.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:144
+
+  OverloadedOperatorKind OOK = MethodDecl->getOverloadedOperator();
+  if (OOK == OO_Star || OOK == OO_Arrow) {
----------------
In case `CXXMemberOperatorCall ` class does not have a way to query the overloaded operator kind maybe it would be worth to add a helper method to that class. So you can do most of the checking using one interface only.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:198
+                            *InnerPointVal);
+    C.addTransition(State);
+  }
----------------
It is possible that both `updateTrackedRegion` and this adds a transition. Both transition will have the same predecessor, i.e. you introduce a split in the exploded graph. Is this intended?


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

https://reviews.llvm.org/D81315