https://github.com/NagyDonat approved this pull request.

I added two minor remarks about comments that were a bit unclear for me; the 
commit LGTM if you handle those. I like the detailed tests and I feel that they 
convincingly demonstrate that this revert is necessary, but I admit that I 
don't have a deep understanding of the situation.

My vague feeling is that the now-reverted commits were reasonable and it's 
surprising and unfortunate that they caused problems.

In general I think that these aspects of our memory model are burdened by lots 
of technical debt (presumably because they were developed through gradual 
evolution). I would say that in an ideal world the behavior of the analyzer 
should only depend on the actual data (what it knows about the memory contents) 
and not our way of representing it (directly copied fields vs LazyCompoundVal).

I hope that eventually I will be able to spend some time to get a deep 
understanding of our memory model and symbol handling (including the 
[RegionStore++](https://discourse.llvm.org/t/rfc-regionstore/70954/24) 
prototype) and perhaps then I'll be able to suggest or develop some 
improvements in this area.

Until then, this revert is the correct move.

https://github.com/llvm/llvm-project/pull/163461
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to