[clang] [nfc][analyzer] Add MemSpace trait to program state (PR #123003)
DonĂ¡t Nagy via cfe-commits
cfe-commits at lists.llvm.org
Tue Jan 28 07:19:44 PST 2025
================
@@ -119,7 +120,26 @@ class MemRegion : public llvm::FoldingSetNode {
virtual MemRegionManager &getMemRegionManager() const = 0;
- LLVM_ATTRIBUTE_RETURNS_NONNULL const MemSpaceRegion *getMemorySpace() const;
+ /// Deprecated. Gets the 'raw' memory space of a memory region's base region.
+ /// Deprecated in favor of the memory space trait which maps memory regions to
+ /// memory spaces, allowing dynamic updating of a memory region's memory
+ /// space.
+ /// @deprecated Use getMemorySpace(ProgramStateRef) instead.
----------------
NagyDonat wrote:
> In your place I'd comment out (aka. Remove the overload not taking a State).
Then try to fix all the build errors.
Once all the ones are fixed that deserves to be fixed, reenable the overload to make the build errors go away.
As the state-less getter function was renamed (from `getMemorySpace` to `getRawMemorySpace`) it's already easy to see al the locations where it's used (if you scroll down within the review of this PR). I marked a few applications of `getRawMemorySpace` either as "this should continue to use the raw variant" or as "this should be replaced by `getMemorySpace(PorgramStateRef)`", but it seems that the difference is inconsequential in many situations.
> I definitely think you shouldnt add deprecation attribute to the api that shouls be avoided. A comment or a slightly more mouthful spelling of the api shouls be enough to deliver the intent.
I agree with @steakhal's suggestion that you shouldn't use the deprecation attribute. In fact, I'm not entirely sure that speaking about deprecation is justified, as there are a few tricky examples (the ArrayBoundV2 lower bound and the one that I marked in `RegionStoreManager::getBinding`, there might be one or two others) where it's important to use the `Raw` variant and replacing it with `getMemorySpace(ProgramStateRef)`" would cause incorrect behavior in a few corner cases.
Perhaps it'd be better to just say something like "Usually it's better to use `getMemorySpace(ProgramStateRef)`"...
https://github.com/llvm/llvm-project/pull/123003
More information about the cfe-commits
mailing list