NoQ accepted this revision. NoQ added a comment. This revision is now accepted and ready to land.
Mm, ok, i admit that i don't know what specific de-duplication do we want to have and what are the usability implications of it. If we want de-duplication for the same region reported in different spots of the same path, we need a state trait. If we want de-duplication for the same region globally across the whole analysis, we need a checker-wide set of regions, like what you're doing. If we want de-duplication for the same region globally across the whole translation unit, we need a checker-wide set of AST nodes (eg., field chains) that describe regions in terms that outlive `ExprEngine`. If we want de-duplication for different regions within the same constructor call site, we need a uniqueing location at the call site, like what you're doing. If we want de-duplication for different regions within the same constructor itself, we don't need a uniqueing location. I guess you should just do whatever you want here? In https://reviews.llvm.org/D51531#1286110, @Szelethus wrote: > Oh, and the reason why I think it would add a lot of complication: since only > `ExprEngine` is avaible in the `checkEndAnalysis` callback, which, from what > I can see, doesn't have a handly `isDead` method, so I'm not even sure how > I'd implement it. Mm, what do you need `isDead` for? Everything is dead at the end of the analysis, you need to clean up everything, otherwise you get use-after-free(?) ================ Comment at: lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp:203 const Stmt *CallSite = Context.getStackFrame()->getCallSite(); if (CallSite) LocUsedForUniqueing = PathDiagnosticLocation::createBegin( ---------------- Hmm, does `LocUsedForUniqueing` remain uninitialized sometimes? I guess it's on the top frame. What does it do in this case? Why do we need a uniqueing location in other cases? I guess it only *increases* the amount of reports, right? I.e., constructors called from different call sites produce different reports(?) https://reviews.llvm.org/D51531 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
