[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Good job, great to see how you are going through the whole list of methods!

As for the patch, some tests and COMMENTS will be nice.  Also, I think that for a checker that models quite a lot of functions and methods, we need to follow the pattern:

  if (isMethodA(...)) {
    return handleMethodA(...);
  }
  if (isMethodB(...)) {
    return handleMethodB(...);
  }
  ...

however small implementations for those methods are.  It will give fast insight into which methods are actually supported.



================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:86
+bool hasStdClassWithName(const CXXRecordDecl *RD,
+                         const SmallVectorImpl<StringRef> &Names) {
+  if (!RD || !RD->getDeclContext()->isStdNamespace())
----------------
When you change those vectors of names to global arrays, we can change it to `ArrayRef` to be more idiomatic LLVM code.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:111
+  return hasStdClassWithName(
+      RD, SmallVector<StringRef, 3>{"shared_ptr", "unique_ptr", "weak_ptr"});
+}
----------------
This is a compile-time constant, I don't think we should construct it every time in runtime.  Global array of three names is totally fine.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:194
+  const auto *RD = E->getType()->getAsCXXRecordDecl();
+  return hasStdClassWithName(RD, SmallVector<StringRef, 1>{"basic_ostream"});
+}
----------------
Same goes here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105421