[Lldb-commits] [PATCH] D65122: [Symbol] Use llvm::Expected when getting TypeSystems

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

In D65122#1602067 <https://reviews.llvm.org/D65122#1602067>, @JDevlieghere wrote:

> In D65122#1602025 <https://reviews.llvm.org/D65122#1602025>, @xiaobai wrote:
>
> > After going through this and modifying this patch, I can't help but wonder if `llvm::Optional<TypeSystem &>` would be more appropriate. There are plenty of instances where it's not a hard error if you can't get a TypeSystem and the appropriate action is probably just to log and move on. I am conflicted because I like how Expected forces you to be more rigorous with error handling but I can't help but feel it is the wrong abstraction. Thoughts?
>
>
> I think an `Optional` would be fine. We can always create an `Error` when necessary, with the trade-off of being a little less precision about the root cause, which honestly doesn't seem that informative anyway.


I'm not that familiar with type systems, so I don't know if an explicit error value is useful. Adopting Error/Expected initially has a slightly higher barrier because the surrounding code does not know about Errors, and so you have to do something explicit and verbose to the error, and then create a default value of the return type anyway. But, as the surrounding code adopts Errors, the verboseness should disappear. This, of course, assumes that we want to adopt Errors here and in the surrounding code.

What I am familiar with though is the `Optional` template, I can note that there is no such thing as an `Optional<T&>` (the Optional template does not play well with references). You could make that a thing, but an `Optional<T&>` is essentially just a `T *`, which is what we have now :D. And I wouldn't want `Optional<T*>` as that just creates more ambiguity.

Thinking ahead I am wondering if we could just arrange things such that the TypeSystem creation always succeeds. One day, I am hoping to arrange is so that a Module will always have an associated ObjectFile and a SymbolFile (by avoiding creating a Module if the ObjectFile construction fails, and creating an "empty" SymbolFile if needed). At that point being able to always create a type system would not seem unreasonable. I don't think any of this is directly relevant here and now, but it may speak against introducing Expected here (if you agree with my vision, that is).


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

https://reviews.llvm.org/D65122