steakhal added a comment. In D96090#2572069 <https://reviews.llvm.org/D96090#2572069>, @ASDenysPetrov wrote:
> In D96090#2568431 <https://reviews.llvm.org/D96090#2568431>, @steakhal wrote: > >> Could you please rebase this? > > I'll update and rebase this patch soon. > >> If it depends on any parent revisions please document them as well. > > It does. You can see this in the stack. Do I need to mention this somewhere > else? No, I probably missed that. My bad. ================ Comment at: clang/include/clang/StaticAnalyzer/Core/PathSensitive/SValBuilder.h:101-103 // FIXME: Make these protected again once RegionStoreManager correctly // handles loads from different bound value types. virtual SVal dispatchCast(SVal val, QualType castTy) = 0; ---------------- ASDenysPetrov wrote: > steakhal wrote: > > I'm not sure if this FIXME is still applicable. > > > > I'm also confused about having two functions doing effectively the same > > thing. > > `SimpleSValBuilder::dispatchCast` is a virtual function, which just invokes > > a non-virtual function `SValBuilder::evalCast`. > > > > Why should it be virtual in the first place? > I want to make the patch smaller avoiding things that could be changed > individually. We can remove `dispatchCast` in the next post-cleanup patch. Seems reasonable to me. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96090/new/ https://reviews.llvm.org/D96090 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
