ilya-biryukov added inline comments.
================ Comment at: clangd/JSONRPCDispatcher.h:78 llvm::Optional<json::Expr> ID; + std::unique_ptr<trace::Span> Tracer; }; ---------------- ilya-biryukov wrote: > sammccall wrote: > > ilya-biryukov wrote: > > > Why do we need `unique_ptr`? Are `Span`s non-movable? > > Spans aren't movable, they have an explicitly declared destructor. (The > > copy constructor is only deprecated by this condition, but Span has a > > unique_ptr member). > > > > We could make them movable, though it's not absolutely trivial (we need an > > extra bool to indicate that this is moved-from and the destructor should be > > a no-op). > > > > I think wrapping them in a `unique_ptr` here is slightly simpler than > > implementing the right move semantics by hand, but LMK what you think. > > We could make them movable, though it's not absolutely trivial (we need an > > extra bool to indicate that this is moved-from and the destructor should be > > a no-op). > I kinda hate this part when dealing with movable objects, too. I still do > that, mostly to avoid heap allocs, but we're not on a hot path, so it's fine > both ways. > We could use `llvm::Optional` instead of `unique_ptr` to get rid of the heap > alloc too. Oh, sorry, `llvm::Optional` surely doesn't help here, it would still be non-movable. Forget what I said. https://reviews.llvm.org/D40132 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
