[Lldb-commits] [PATCH] D56688: Make CompilerType::getBitSize() / getByteSize() return an optional result. (NFC)

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

Overall, I am pretty sure we will end up with some kind of a special `Optional` class sooner or later. However, it's not clear to me whether this is the place to start. What kind of space this is saving? I would expect that size matters when you store something as a data member in a class, because you can have zillion of instances of that class on the heap. However, all the uses of this I see are local (stack) variables, where the size does not matter that much, and so you might as well have used an `Optional`. Have I missed something?

Overall, I think a good way to keep things sane would be to keep these specialized Optional classes as implementation details of classes, and convert to a real `Optional` as soon as it goes out to the rest of the world. This would be more consistent with how the rest of llvm does this (see `llvm::VersionTuple` for instance, which also does some storage optimizations, though without a special class). This would allow us to use the richer `Optional` interface in most of the places (e.g. `Optional` has special overloads of operator==, which allow you to compare an `Optional<T>` with plain `T`), while still keeping the size footprint in check.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D56688/new/

https://reviews.llvm.org/D56688