[PATCH] D86027: [analyzer] Add bool operator modeling for unque_ptr

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:130
+// Returns empty type if not found valid inner pointer type.
+static QualType getInnerPointerType(const CallEvent &Call, CheckerContext &C) {
+  QualType InnerType{};
----------------
vrnithinkumar wrote:
> It seems like a long shot to me.
> I am not sure is there any direct or easy way to get inner pointer type from a smart pointer  
That's about right. You're doing exactly what you're asked: grab the template parameter of the class. The problem is indeed that complicated!


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:145
+        TemplateArgs.size() > 0 &&
+        "Smart pointer should have specialized with atleast one template type");
+    auto InnerValueType = TemplateArgs[0].getAsType();
----------------
That's pretty fundamental, right? If it's a specialization, it must have something specialized. It isn't specific to unique pointers, right?

Because unique pointers aren't special; technically anybody can define an arbitrary class with name `std::unique_ptr` and any properties they'd like. It's going to be undefined behavior according to the standard (because namespace `std` is explicitly reserved for the standard library) but if the compiler *crashes* it'll still be our fault.




================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:448
+
+    if (NullState) {
+      auto NullVal = C.getSValBuilder().makeNull();
----------------
There's no need to check. You just conjured a brand new symbol out of thin air; you can be sure that it's completely unconstrained and both states are feasible. You can instead `assert()` that they're both feasible.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:458-460
+                        OS << "Assuming smart pointer ";
+                        ThisRegion->printPretty(OS);
+                        OS << " is null";
----------------
Wait, what happens when the region can't be pretty-printed? Does it print two spaces between "pointer" and "is"?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86027