NagyDonat wrote: > > > Have you considered changing `MemRegion::getMemorySpace()` into > > > `MemRegion::getMemorySpace(ProgramStateRef)`? > > > > > > I thought about this, but I decided against it since I am thinking that > > MemSpaces will eventually be their own separate thing, not part of the > > MemRegion hierarchy, i.e., MemSpaces will eventually just be a trait thing, > > maybe with its own checker depending on how much functionality can be added > > with this different implementation? > > All this being said, I am happy to change the API of how we get memory > > spaces, I am not tied to the current approach/API, just that we can move > > towards better results for #115410. > > Let express that I don't think we will ever drop memspaces from memregions, > even if we strive for. The return of investment is not going to be there once > we cover the unknownspace case - which you plan to fix the issue you referred > to. So I think we should aim for a consistent and better landscape to achieve > that minimal goal. > > I think having a single API (or overload set) is important for this. This is > why I'd push for patching the `getMemorySpace()` overloads.
I would strongly support changing `MemRegion::getMemorySpace()` into `MemRegion::getMemorySpace(ProgramStateRef)` even within this commit if it's feasible (and perhaps leave behind a `MemRegion::getRawMemorySpace()` if needed e.g. for the ArrayBoundCheckerV2 lower bound check) -- simply because there is no reason to leave behind the `ProgramStateRef`-less version, even temporarily. However, I would strongly support continuing this refactoring effort by dropping the memspaces from the memregions in follow-up commits -- I don't think that it would be terribly difficult and it would make it easier to reason about memregions. (If nobody else does it, I will eventually do it when I get fed up on some confusing `MemRegion`-related code.) https://github.com/llvm/llvm-project/pull/123003 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
