mgorny marked 3 inline comments as done. mgorny added inline comments.
================ Comment at: lldb/source/Host/posix/ConnectionFileDescriptorPosix.cpp:772-774 + llvm::Error error = llvm::Error::success(); + m_io_sp = std::make_shared<SerialPort>(fd, File::eOpenOptionReadWrite, + serial_options.get(), true, error); ---------------- labath wrote: > mgorny wrote: > > labath wrote: > > > mgorny wrote: > > > > labath wrote: > > > > > It would be better to hide this by making the constructor private and > > > > > have a static factory function returning > > > > > `Expected<unique_ptr<SerialPort>>`. Bonus points if you can move all > > > > > the fallible operations into the factory function so that the (now > > > > > private) constructor does not need the Error argument at all > > > > I know but I couldn't make this work — the compiler just won't find a > > > > way to convert the new `SerialPort` instance into > > > > `Expected<SerialPort>`. I suspect I would have to add move ctors and > > > > assignment operators all the way up the class hierarchy. > > > That's where the `unique_ptr` part comes in. :) > > I presume you mean `shared_ptr` here, or know some dirty trick. > a unique_ptr is convertible to a shared_ptr, but not the other way around, so > it's better to return the former Ah, indeed. I was missing std::move. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D111355/new/ https://reviews.llvm.org/D111355 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
