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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
