[PATCH] D26837: [analyzer] Litter the SVal/SymExpr/MemRegion class hierarchy with asserts.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.


================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:319
 public:
-  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) {}
+  SymbolVal() = delete;
+  SymbolVal(SymbolRef sym) : NonLoc(SymbolValKind, sym) { assert(sym); }
----------------
a.sidorin wrote:
> I cannot completely agree with this change. For example, some STL container methods require their type to be default constructible. Yes, it is a very limited usage case but  I don't see strong reason to do it because there also may be some another usage scenarios.
Hmm. I'd definitely suggest a container of `SymbolRef`s or `SVal`s for this use case.

Compare to memory region classes, which aren't constructible at all.


================
Comment at: include/clang/StaticAnalyzer/Core/PathSensitive/SVals.h:478
 public:
-  explicit GotoLabel(LabelDecl *Label) : Loc(GotoLabelKind, Label) {}
+  explicit GotoLabel(LabelDecl *Label) : Loc(GotoLabelKind, Label) {
+    assert(Label);
----------------
a.sidorin wrote:
> By the way, why does this ctor accept a non-constant pointer?
Ouch. I think it's because `AddrLabelExpr::getLabel()` returns a non-constant pointer. Worth fixing, i guess.


https://reviews.llvm.org/D26837