ASDenysPetrov added inline comments.
================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1643-1654
+ const VarRegion *VR = dyn_cast<VarRegion>(R->getSuperRegion());
+ SmallVector<SVal, 2> SValOffsets;
+ SValOffsets.push_back(R->getIndex());
+ if (const ElementRegion *ER = dyn_cast<ElementRegion>(R->getSuperRegion())) {
+ const ElementRegion *LastER = nullptr;
+ do {
+ SValOffsets.push_back(ER->getIndex());
----------------
steakhal wrote:
> I dislike this part of code. It has side effects all over the place.
> Please use immediately called lambdas to describe your intentions in a more
> //pure// manner.
>
> Besides that, when I see `ElementRegions`, I'm always scared. They sometimes
> represent reinterpret casts.
> Since you have dealt with so many casts and region store stuff in the past,
> I'm assuming you have thought about this here.
> Although, I'm still scared slightly, and I'd like to have evidence of this
> thought process. Likely some assertions or at least some comments? Could you
> @ASDenysPetrov elaborate on this?
> Please use immediately called lambdas.
I'll do.
> They sometimes represent reinterpret casts. ... I'm assuming you have
> thought about this here.
Yes. This is my another revision in the stack. You are welcome: D110927
================
Comment at: clang/lib/StaticAnalyzer/Core/RegionStore.cpp:1656
+
+ // TODO: Add a test case for this check.
+ if (!VR)
----------------
martong wrote:
> Please address this TODO.
I found the tests:
- Analysis/MemRegion.cpp
- Analysis/ctor.mm
- Analysis/mpichecker.cpp
- Analysis/string.c
They cover this check.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111654/new/
https://reviews.llvm.org/D111654
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
