vsavchenko added a comment. In D104135#2814081 <https://reviews.llvm.org/D104135#2814081>, @NoQ wrote:
> P.S. I like this design! > > I'm trying to remember why we needed a factory in the first place. Who's gonna carry your callbacks for ya, huh? > I think a lot of tags work just fine as static variables. Sure, but if you have data, let's say it's something that doesn't need to be stored somewhere to prolong its lifetime, you still need to have static variable for each possible value of that piece of data. Not really feasible in the vast majority of cases. Because you might need to refer to the same tag with different possible values. > In case of D104136 <https://reviews.llvm.org/D104136> IIUC `IdentityCall` > could always be discovered as the current program point... That's true, I put this one as "just in case". > ...and its `Source` is its only argument... Not really, there is also case when the call is `IdentityThis`, so its `Source` is not its only argument. > ...so all we need is a marker that says "we did indeed model this code as > identity" so it could potentially be allocated statically. But when the tag > really needs to carry more data then there's probably no way around the > factory. ================ Comment at: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h:777 static bool classof(const ProgramPointTag *T) { return T->getTagKind() == &Kind; } ---------------- NoQ wrote: > NoQ wrote: > > It sounds like `NoteTag` `isn't<DataTag>` despite inheriting from it. > Wait nvm, `DataTag` doesn't provide a `classof` at all. This means we can't > write `isa<DataTag>` at all, right? Yes, it's purely abstract. `ProgramPointTag` also doesn't have `classof` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D104135/new/ https://reviews.llvm.org/D104135 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
