On Aug 1, 2010, at 9:56 PM, Zhongxing Xu <[email protected]> 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.
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.
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
