Re: Introducing mozilla::Result for better error handling

On Thu, Jan 5, 2017 at 7:31 AM, Xidorn Quan wrote: > Probably it can add a constructor which accepts T&& and Move it down the > chain. > Agreed. We didn't do this initially to keep things simple and because we didn't need it, but if there's a good use case we should add move

Re: Introducing mozilla::Result for better error handling

On Thu, Jan 5, 2017, at 03:28 PM, Cameron McCormack wrote: > On Tue, Dec 20, 2016, at 09:46 PM, Jan de Mooij wrote: > > A few weeks ago we added mozilla::Result to MFBT [0][1]. I was > > asked > > to inform dev-platform about this, so here's a quick overview. > > mozilla::Result is

Re: Introducing mozilla::Result for better error handling

On Tue, Dec 20, 2016, at 09:46 PM, Jan de Mooij wrote: > A few weeks ago we added mozilla::Result to MFBT [0][1]. I was > asked > to inform dev-platform about this, so here's a quick overview. > mozilla::Result is based on Rust's Result type [2]. It contains > either a success value of

Re: Introducing mozilla::Result for better error handling

On 12/22/2016 02:22 AM, 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-call-out-issues > form. I understand this is going to have differing opinions -- I think > there's merit to

Re: Introducing mozilla::Result for better error handling

On Sat, Dec 24, 2016 at 2:10 AM, Ehsan Akhgari wrote: > > > However, new files should follow standard Mozilla style and use > > an upper-case letter at the start of function names. > > Isn't mozilla::Result new code? Yes, and I requested the use of upper-case first

Re: Introducing mozilla::Result for better error handling

On Fri, Dec 23, 2016 at 2:03 AM, smaug wrote: > On 12/22/2016 08:14 PM, Bobby Holley wrote: > >> We've had this debate several times already, culminating in the attempt to >> ban NS_ENSURE_* macros. It didn't work. >> >> This is a bit different. One of the most common NS_ENSURE_

Re: Introducing mozilla::Result for better error handling

On 2016-12-23 6:40 AM, Nicholas Nethercote wrote: > It was once a mess, but is not any more. MFBT has for some time used > standard Mozilla style, with the following minor exception described in > mfbt/STYLE: > > - Some of the files use a lower-case letter at the start of function names. > This

Re: Introducing mozilla::Result for better error handling

It was once a mess, but is not any more. MFBT has for some time used standard Mozilla style, with the following minor exception described in mfbt/STYLE: - Some of the files use a lower-case letter at the start of function names. This is because MFBT used to use a different style, and was later

Re: Introducing mozilla::Result for better error handling

On Fri, Dec 23, 2016 at 12:07:28PM +0200, smaug wrote: > On 12/20/2016 03:46 PM, Jan de Mooij wrote: > > Hi all, > > > > A few weeks ago we added mozilla::Result to MFBT [0][1]. I was asked > > to inform dev-platform about this, so here's a quick overview. > > mozilla::Result is based

Re: Introducing mozilla::Result for better error handling

On 12/20/2016 03:46 PM, Jan de Mooij wrote: Hi all, A few weeks ago we added mozilla::Result to MFBT [0][1]. I was asked to inform dev-platform about this, so here's a quick overview. mozilla::Result is based on Rust's Result type [2]. It contains either a success value of type V or

Re: Introducing mozilla::Result for better error handling

On 12/22/2016 08:14 PM, Bobby Holley wrote: We've had this debate several times already, culminating in the attempt to ban NS_ENSURE_* macros. It didn't work. This is a bit different. One of the most common NS_ENSURE_ macros is SUCCESS variant which explicitly has the return value. MOZ_TRY

Re: Introducing mozilla::Result for better error handling

On 22/12/2016 19:14, Bobby Holley wrote: > We've had this debate several times already, culminating in the attempt to > ban NS_ENSURE_* macros. It didn't work. If we don't want to get rid of it then why not make the intent immediately obvious from the name? Something like MOZ_RETURN_IF_FAILED()

Re: Introducing mozilla::Result for better error handling

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

Re: Introducing mozilla::Result for better error handling

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-call-out-issues form. I've never been convinced by that argument. Reviewers accept code

Re: Introducing mozilla::Result for better error handling

We've had this debate several times already, culminating in the attempt to ban NS_ENSURE_* macros. It didn't work. Subjectively, I think the reasons for the failure were: (a) We want logging/warnings on exceptional return paths, especially the ones we don't expect. (b) We want to check every

Re: Introducing mozilla::Result for better error handling

On 12/22/2016 06:53 AM, 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

Re: Introducing mozilla::Result for better error handling

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-call-out-issues form. I understand this is going to have differing opinions -- I think there's merit to more concise code -- but for the things I look at I

Re: Introducing mozilla::Result for better error handling

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,

Re: Introducing mozilla::Result for better error handling

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

Re: Introducing mozilla::Result for better error handling

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 > >

Re: Introducing mozilla::Result for better error handling

On Tue, Dec 20, 2016 at 10:31 AM, Ehsan Akhgari wrote: > > Result Baz() { MOZ_TRY(Bar()); ... } > > I have one question: what is Bar() expected to return here? Result where F is implicitly convertible to E. An E type, > or a type that's implicitly

Re: Introducing mozilla::Result for better error handling

On Tue, Dec 20, 2016 at 5:31 PM, Ehsan Akhgari wrote: > Is there a > potential for mistakes like for example caused by the error types being > implicitly convertible to each other but the values changing their > meanings upon the conversion (for example a bool error type

Re: Introducing mozilla::Result for better error handling

On 2016-12-20 8:46 AM, Jan de Mooij wrote: > Hi all, > > A few weeks ago we added mozilla::Result to MFBT [0][1]. Amazing! > I was asked > to inform dev-platform about this, so here's a quick overview. > mozilla::Result is based on Rust's Result type [2]. It contains > either a

Re: Introducing mozilla::Result for better error handling

Thanks for getting this landed! Hooray for better error handling! \o/ On Tue, Dec 20, 2016 at 5:46 AM, Jan de Mooij wrote: > Hi all, > > A few weeks ago we added mozilla::Result to MFBT [0][1]. I was asked > to inform dev-platform about this, so here's a quick

Introducing mozilla::Result for better error handling

Hi all, A few weeks ago we added mozilla::Result to MFBT [0][1]. I was asked to inform dev-platform about this, so here's a quick overview. mozilla::Result is based on Rust's Result type [2]. It contains either a success value of type V or an error value of type E. For example, a