[Lldb-commits] [PATCH] D51557: Replace uses of LazyBool with LazyBool template

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

I've been wanting to implement something like this for a long time, so thank you for working on this. :)

In https://reviews.llvm.org/D51557#1221273, @zturner wrote:

> Also I think it doesn't need to be specific to member variables.  We could just have
>
>   template <typename T> class Lazy {
>     std::function<T()> Update;
>     Optional<T> Value;
>   public:
>     LazyValue(std::function<T()> Update) : Update(std::move(Update)) {}
>   };
>


I think the issue with this approach is size. std::function is huge and heap-allocated, whereas this implementation is the same size as `Optional<T>` (which might still be too much for some of our use cases -- in some classes we store a bunch of `needs_update` flags as bitfields to conserve space, which would not be possible here).



================
Comment at: include/lldb/DataFormatters/ValueObjectPrinter.h:138-144
+  bool UpdateShouldPrint();
+  bool UpdateIsNil();
+  bool UpdateIsUnit();
+  bool UpdateIsPtr();
+  bool UpdateIsRef();
+  bool UpdateIsAggregate();
+  bool UpdateIsInstancePtr();
----------------
I wouldn't call these `update` as they don't actually update anything and just return the value. In other parts of the codebase we use `compute` for functions like this.


================
Comment at: include/lldb/DataFormatters/ValueObjectPrinter.h:146
+
+#define LLDB_VOP ValueObjectPrinter
+  LazyBoolMember<LLDB_VOP, &LLDB_VOP::UpdateShouldPrint> m_should_print;
----------------
shafik wrote:
> davide wrote:
> > can you typedef?
> I feel like using ... = is cleaner
I am actually tempted to have a macro which would define all of this boilerplate for you. :D

Something like
```
#define LAZY_MEMBER(Class, Type, Name) \
public: \
  Type get ## Name() { return m_ ## Name.get(*this); } \
private: \
  LazyMember<Type, Class, &Class::compute ## Name> m_ ## Name; \
  Type compute ## Name();
```

Then all that would be left for the user is to define the compute function in the cpp file.


================
Comment at: include/lldb/Utility/Lazy.h:25-28
+  T m_value;
+  bool m_needs_update;
+
+  static_assert(std::is_trivial<T>::value, "Only trivial types are supported.");
----------------
if you use `Optional<T>` instead of hand-rolling the flag here, you could probably get rid of the "trivial type" limitation.


Repository:
  rLLDB LLDB

https://reviews.llvm.org/D51557