https://github.com/steakhal commented:
I had a quick look at the PR, and over all it looks as expected. Good job! Have you considered changing `MemRegion::getMemorySpace()` into `MemRegion::getMemorySpace(ProgramStateRef)`? This should lessen the confusion of which APIs should a dev use to get the memspace of a region (aka. the getter). Speaking of the setter counter part, I think that's is special-purpose enough that most people shouldn't know it exist, so I don't think that would bring much confusion. So the question is, would changing it into `MemRegion::getMemorySpace(ProgramStateRef)` be too intrusive to consider? (Maybe in some contexts where we currently call `getMemorySpace` we just don't have a State, and can't easily get one - and that would kill this approach.) Alternatively, we could say, just introduce this as an overload, and highlight that people should really really use this one. That would bring consistency while being pragmatic and accepting past design choices. 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
