FWIW, you can also avoid the check-for-error-and-early-return-or-else-unwrap dance that is being "hidden" within MOZ_TRY by using `mozilla::Result::andThen` (and `mozilla::Result::map`) which are on inbound and will be on m-c soon.
The patch in https://bugzilla.mozilla.org/show_bug.cgi?id=1324829 has example usage. On Thu, Dec 22, 2016 at 10:20 AM, Kris Maglione <kmagli...@mozilla.com> wrote: > On Wed, Dec 21, 2016 at 11:22:24PM -0800, Eric Rahm wrote: > >> The key point for me is that we're hiding the return. I'm fine with the >> more verbose explicitly-return-and-make-it-easier-for-the-reviewer-to-cal >> l-out-issues >> form. >> > > I've never been convinced by that argument. Reviewers accept code with > early return macros should be familiar enough with them that they don't > make it any harder to spot issues caused by early return. On the flip side, > explicit early return is a pervasive enough pattern that I think most of us > tend to just filter it out as noise. But it's noise that makes the > structure of the code much harder to follow, which makes other errors much > easier to miss. > > And, by the same token, this is part of MFBT, and comes along with a lot > of other helpers, and a general coding style, that makes it much easier to > RAII all the things, and to generally do the right thing. In isolation, I > might worry more about it leading to new early return errors, but in > context, I think it's clearly a step in the right direction. > > > On Wed, Dec 21, 2016 at 10:59 PM, Kris Maglione <kmagli...@mozilla.com> >> wrote: >> >> On Wed, Dec 21, 2016 at 10:53:45PM -0800, Eric Rahm wrote: >>> >>> I like the idea of pulling in some Rusty concepts, but I'm concerned >>>> about >>>> adding yet another early return macro -- absolutely no arguments on the >>>> new >>>> type, just the |MOZ_TRY| macro. In practice these have lead to security >>>> issues (UAF, etc) and memory leaks in the C++ world (I'm looking at you >>>> NS_ENSURE). These aren't hypothetical issues, I've run into them both >>>> working on memshrink and sec issues in Firefox. >>>> >>>> >>> We run into the same issues without those macros. The only real >>> difference >>> is that when you have to follow every call with: >>> >>> if (NS_WARN_IF(NS_FAILED(rv))) { >>> return rv; >>> } >>> >>> It's much easier to lose track of the allocation you did four calls ago >>> that's now 20 lines away. >>> >>> >>> On Wed, Dec 21, 2016 at 9:53 AM, Ted Mielczarek <t...@mielczarek.org> >>> >>>> wrote: >>>> >>>> On Wed, Dec 21, 2016, at 12:30 PM, Jason Orendorff wrote: >>>> >>>>> > The implicit conversion solves a real problem. Imagine these two >>>>> > operations >>>>> > have two different error types: >>>>> > >>>>> > MOZ_TRY(JS_DoFirstThing()); // JS::Error& >>>>> > MOZ_TRY(mozilla::pkix::DoSecondThing()); // pkix::Error >>>>> > >>>>> > We don't want our error-handling scheme getting in the way of using >>>>> them >>>>> > together. So we need a way of unifying the two error types: a shared >>>>> base >>>>> > class, perhaps, or a variant. >>>>> > >>>>> > Experience with Rust says that MOZ_TRY definitely needs to address >>>>> this >>>>> > problem somehow. C++ implicit conversion is just one way to go; we >>>>> can >>>>> > discuss alternatives in the bug. >>>>> >>>>> The `try` macro in Rust will auto-convert error types that implement >>>>> `Into<E>`, AIUI, but that's not automatic for all error types. I >>>>> haven't >>>>> tried it, but I have seen multiple recommendations for the >>>>> `error_chain` >>>>> crate to make this smoother: >>>>> https://docs.rs/error-chain/0.7.1/error_chain/ >>>>> >>>>> It's basically just boilerplate to implement conversions from other >>>>> error types. I wouldn't be surprised if something like that percolates >>>>> into the Rust standard library at some point. >>>>> >>>>> -Ted >>>>> _______________________________________________ >>>>> dev-platform mailing list >>>>> dev-platform@lists.mozilla.org >>>>> https://lists.mozilla.org/listinfo/dev-platform >>>>> >>>>> _______________________________________________ >>>>> >>>> dev-platform mailing list >>>> dev-platform@lists.mozilla.org >>>> https://lists.mozilla.org/listinfo/dev-platform >>>> >>>> >>> -- >>> Kris Maglione >>> Firefox Add-ons Engineer >>> Mozilla Corporation >>> >>> It's always good to take an orthogonal view of something. It develops >>> ideas. >>> --Ken Thompson >>> >>> >>> > -- > Kris Maglione > Firefox Add-ons Engineer > Mozilla Corporation > > Life is too important to be taken seriously. > --Oscar Wilde > > > _______________________________________________ > dev-platform mailing list > dev-platform@lists.mozilla.org > https://lists.mozilla.org/listinfo/dev-platform > _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
