NoQ added inline comments.
================ Comment at: lib/StaticAnalyzer/Checkers/IteratorChecker.cpp:478-479 + auto &SymMgr = C.getSymbolManager(); + EndSym = SymMgr.conjureSymbol(CE, C.getLocationContext(), + C.getASTContext().LongTy, C.blockCount()); + State = createContainerEnd(State, ContReg, EndSym); ---------------- baloghadamsoftware wrote: > NoQ wrote: > > I see what you did here! And i suggest considering `SymbolMetadata` here > > instead of `SymbolConjured`, because it was designed for this purpose: the > > checker for some reason knows there's a special property of an object he > > wants to track, the checker doesn't have a ready-made symbolic value to > > represent this property (because the engine didn't know this property even > > exists, so it didn't denote its value), so the checker comes up with its > > own notation. `SymbolMetadata` is used, for example, in `CStringChecker` to > > denote an unknown string length for a given null-terminated string region. > > > > This symbol carries the connection to the region (you may use it to > > reverse-lookup the region if necessary), and in particular it is considered > > to be live as long as the base region is live (so you don't have to deal > > with liveness manually; see also comments around > > `SymbolReaper::MarkInUse`). It's also easier to debug, because we can track > > the symbol back to the checker. Generally, symbol class hierarchy is cool > > because we know a lot about the symbol by just looking at it, and > > `SymbolConjured` is a sad fallback we use when we didn't care or manage to > > come up with a better symbol. > > > > I'm not sure if `LongTy` is superior to `IntTy` here, since we don't know > > what to expect from the container anyway. > > > > Also, please de-duplicate the symbol creation code. Birth of a symbol is > > something so rare it deserves a drum roll :) > > > > I'd take a pause to figure out if the same logic should be applied to the > > map from containers to end-iterators. > SymbolMetaData is bound to a MemRegion. Iterators are sometimes symbols and > sometimes memory regions, this was one of the first lessons I learned from my > first iterator checker. Oh. Hmm. Ok. Right. To be sure: in what cases do you need to create a new symbol when the iterator is already a symbol? How broken do we become if we try to say that the symbolic iterator and its own offset are the same thing? https://reviews.llvm.org/D32592 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
