[PATCH] D69230: RFC: specialized Optional<T> for T that can represent its own invalid state

Lawrence D'Anna via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Oct 20 18:32:47 PDT 2019


lawrence_danna created this revision.
lawrence_danna added a reviewer: labath.
Herald added a subscriber: dexonsmith.
Herald added a project: LLVM.

Some types have a representation that would allow them to serve as their own 
optional.   For example `PythonObject` in LLDB is just a `PyObject *` under
the hood.  An `Optional<PythonObject>` could be represented by that one pointer, 
with the value of NULL in the None case.

Right now, instead of using `Optional<PythonObject>`, we just allow them to be in 
an invalid state, with lots of checks to `PythonObject::IsValid` everywhere.   
I think it would be better to actually mark which PythonObjects are allowed
to be invalid using optionals.   But it seems like a shame to add an extra runtime
bool when we can just use a NULL pointer instead.

I'm posting this patch as a straw-man, as I strongly suspect there's a better way
to do it.

It creates a type trait `llvm::optional_detail::has_is_valid`.   If a class
is marked with this trait, it should implement `MakeInvalid()` and `IsValid()`.

What I like: it's easy to opt a class in without a bunch of boilerplate.

What I don't like: MakeInvalid() isn't private, even though you're not supposed to, 
you can still create invalid objects instead of using optionals.

Another way to do it would be to require the invalid state is all zeros represent 
it as

  union {
      T value;
      uint8_t bytes[sizeof(T)];
  }

What do you think?   Is this a good idea in general?  If so, what's the right way
to implement it?

Also who else should be added to reviewers?


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D69230

Files:
  llvm/include/llvm/ADT/Optional.h
  llvm/unittests/ADT/OptionalTest.cpp

-------------- next part --------------
A non-text attachment was scrubbed...
Name: D69230.225812.patch
Type: text/x-patch
Size: 3280 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20191021/51c4a74b/attachment.bin>


More information about the llvm-commits mailing list