martong marked 3 inline comments as done. martong added a comment. Herald added a project: All.
Thanks for the review Denys, and sorry for the long delay with the update. I hope that this patch is going to complement nicely the rest of the cast patches. ================ Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1088 } + +SVal clang::ento::SValBuilder::simplifySymbolCast(nonloc::SymbolVal V, ---------------- ASDenysPetrov wrote: > ASDenysPetrov wrote: > > Notify the user about the contract. > Mistake. `CastTy`, not `SafeTy` Okay, I've added an assertion for this check. ================ Comment at: clang/lib/StaticAnalyzer/Core/SValBuilder.cpp:1095-1097 + auto SV = makeSymbolVal(SE); + assert(SV == V); + return SV; ---------------- ASDenysPetrov wrote: > Explain please, why do you need to create a new `SymbolVal` here, but not > just returning `V` as long as you know that the type is the same? > And `assert` also seems weird for me. If there's smth obscure that you're > trying to handle, please add some explaination comment. You're right, we can just simply return `V`, I've updated like so. (I just wanted to make sure that the `makeSymbolVal` is consistent, but that has nothing to do with this patch.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D117229/new/ https://reviews.llvm.org/D117229 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
