ilya-biryukov added inline comments.
================
Comment at: clangd/Function.h:141
namespace detail {
+/// Optional-like type that sets its value to llvm::None when it was moved
+/// constructed from.
----------------
sammccall wrote:
> As noted in the thread I don't think this pulls its weight for us.
> Swapping unique_ptr for optional doesn't seem like it'll ever be a bottleneck
> for us. If we really must avoid the allocation for some reason then adding
> move logic to ScopeExitGuard seems simpler.
> If you feel strongly about keeping this as a separate concept, it should
> really go in llvm/ADT/Optional.h, and probably needs a new name.
>
It's not a bottleneck, but I don't see why something like `ScopeExitGuard`
should incur an overhead of an extra alloc that needs to be done by
`unique_ptr`.
That also seems like a nice reusable concept for move-only things.
I don't subscribe to the view that this class carries much weight. It's a
really simple wrapper. It will probably get much more complicated if we want to
support the general case (i.e. make it a substitute of `llvm::Optional` with
extra semantics). And I don't see how we can put it into ADT/Optional.h without
supporting the general case.
Repository:
rCTE Clang Tools Extra
https://reviews.llvm.org/D42525
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
