[cfe-commits] r110020 - in /cfe/trunk: include/clang/Checker/PathSensitive/MemRegion.h lib/Checker/FlatStore.cpp lib/Checker/MemRegion.cpp lib/Checker/RegionStore.cpp lib/Checker/SimpleSValuator.cpp lib/Checker/Store.cpp test/Analysis/flat-store.

Zhongxing Xu xuzhongxing at gmail.com
Mon Aug 2 18:27:46 PDT 2010

On Tue, Aug 3, 2010 at 12:21 AM, Ted Kremenek <kremenek at apple.com> wrote:
> On Aug 1, 2010, at 9:56 PM, Zhongxing Xu <xuzhongxing at gmail.com> wrote:
>> +  /// Compute the offset within the top level memory object.
>> +  virtual RegionOffset getAsOffset() const {
>> +    assert(0 && "unimplemented");
>> +  }
> Instead of embedding the algorithm for computing RegionOffsets inside methods of SubRegion and it's derived classes, perhaps we should separate them into their own functions (or a factory class).  After all, not all clients of MemRegion care about offsets, and to keep our APIs clean in LLVM we tend to not like to embed algorithms that operate on datatypes into the datatypes themselves.  I'm also not a fan of using assertions in this way to catch unimplemented functionality, as it results in crashes (assertion failures) for users.  If we put this logic in one function, for example, we could have a switch statement off the MemRegion kind and the compiler would tell us when we missed something.

Actually this is my original implementation. I'll fix this in a later patch.

> What also bothers me is that the notion of an offset seems somewhat tailored to the expectations of a given StoreManager.  I'm concerned that the needs of FlatStoreManager and RegionStoreManager may diverge at some point, and may wish to compute offsets differently.  That wouldn't be ideal to diverge that logic, but it is something we should keep in mind when develop this API.

Yes. Currently region store and flat store do different offset computing.

More information about the cfe-commits mailing list