[PATCH] D97183: [analyzer] Add NoteTag for smart-ptr get()

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD added inline comments.


================
Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:258
+  const Decl *D = DS->getSingleDecl();
+  assert(D && "DeclStmt should have at least one Decl");
+  const auto *VD = llvm::dyn_cast<VarDecl>(D);
----------------
NoQ wrote:
> That's not what the assert is saying; the assert is saying that the `DeclStmt` has //exactly// one `Decl`. It basically forbids code like
> ```
> int x = 1, y = 2;
> ```
> . You may wonder why don't you crash all over the place. That's because Clang CFG [[ https://github.com/llvm/llvm-project/blob/llvmorg-12.0.0/clang/lib/Analysis/CFG.cpp#L2826 | creates its own ]] `DeclStmt`s that aren't normally present in the AST, that always have exactly one declaration. This is necessary because there may be non-trivial control flow between these declarations (due to, say, presence of operator `?:` in the initializer) so they have to be represented as different elements (possibly even in different blocks) in the CFG.
So I guess the tests at lines `317` and `378` of `smart-ptr-text-output.cpp` work because of the CFG messing with the AST? So should I remove the assert?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D97183