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

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:87
+static bool hasStdClassWithName(const CXXRecordDecl *RD,
+                                const ArrayRef<StringRef> &Names) {
+  if (!RD || !RD->getDeclContext()->isStdNamespace())
----------------
`ArrayRef` is already a ref (and it says it in its name), so you should remove it there.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:103
+static bool isStdSmartPtr(const CXXRecordDecl *RD) {
+  return hasStdClassWithName(RD, {STD_PTR_NAMES, 3});
+}
----------------
`ArrayRef` has an implicit constructor from C-arrays, putting size here is unnecessary.
And the reason why you were not able to do this is **type mismatch**.  `STD_PTR_NAMES` is array of `llvm::StringLiteral`, but you chose `ArrayRef<StringRef>` as the type of the argument.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:195
 
+const SmallVector<StringRef, 1> BasicOstreamName = {"basic_ostream"};
+
----------------
RedDocMD wrote:
> vsavchenko wrote:
> > Same here + don't call it "Name" (singular).  It is a) an array and b) in the future, we might add more things to it, so we shouldn't need to rename it everywhere.
> Well, we won't have to rename but we will still have to change the array length everywhere if more names are added.
No, we don't have to do that.  See the reason in the other comment.
Whenever you need to explicitly hardcode size for a stack-allocated array, you are doing something wrong.  Stack-allocated everything (VLAs aside) is required to have a fixed size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105421