steakhal accepted this revision. steakhal added a comment. This revision is now accepted and ready to land.
Feel free to commit your change given that you fix my final nits. Yeah, I'm soo bad in review. I should have proposed these previously. Sorry. ================ Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1641 +/// +/// \param CAT [in] The given type of ConstantArrayType. +/// ---------------- You don't need to document this. Aside from that consider marking these //implementation detail// functions `static` if they are not yet in an anonymous namespace - I haven't checked. ================ Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1646 + assert(CAT && "ConstantArrayType should not be null"); + // Get a canonical type of the array. + CAT = cast<ConstantArrayType>(CAT->getCanonicalTypeInternal()); ---------------- I think we will get it :D ================ Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1751 + // Treat an n-dimensional array. + const std::pair<SmallVector<SVal, 2>, const MemRegion *> SValOffsetsAndBase = + getElementRegionOffsetsWithBase(R); ---------------- The `std::tie()` pattern is probably more readable. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111654/new/ https://reviews.llvm.org/D111654 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
