Re: New [must_use] property in XPIDL

2016-08-26 Thread Ehsan Akhgari
On 2016-08-23 6:30 PM, Gerald Squelart wrote: > On the build-time vs static analysis point: > > I'd much prefer to have errors pointed out right from './mach build', which I > can fix on the spot; rather than wait hours until I notice static analysis > errors on a treeherder build. > (e.g., I

Re: New [must_use] property in XPIDL

2016-08-25 Thread Nicholas Nethercote
On Thu, Aug 25, 2016 at 10:33 PM, Gijs Kruitbosch wrote: > > Sorry, I'm not being very clear, I'm afraid. I'm not asking to ditch > nsresult. > > I'm wondering if we can make a "smarter" annotation than [must_use] that > requires *either* a nullcheck on the outparam or

Re: New [must_use] property in XPIDL

2016-08-25 Thread Nicholas Nethercote
On Fri, Aug 26, 2016 at 3:31 AM, ISHIKAWA,chiaki wrote: > > There ought to be a long-term plan for C-C and M-C to introduce such missing > checks gradually. That's exactly what I'm trying to do with MOZ_MUST_USE/[must_use]. Bug 1268766 is the tracking bug. There's a lot of

Re: New [must_use] property in XPIDL

2016-08-25 Thread ISHIKAWA,chiaki
On 2016/08/25 6:38, Mike Hommey wrote: On Mon, Aug 22, 2016 at 04:39:15PM -0700, R Kent James wrote: On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: I strongly encourage people to do likewise on any IDL files with which they are familiar. Adding appropriate checks isn't always easy Exactly,

Re: New [must_use] property in XPIDL

2016-08-25 Thread Gijs Kruitbosch
On 25/08/2016 06:14, Nicholas Nethercote wrote: On Wed, Aug 24, 2016 at 9:38 PM, Gijs Kruitbosch wrote: I'm mostly in frontend Firefox land so excuse the potentially dumb question and probably incorrect terminology... My impression was that we already have things like

Re: New [must_use] property in XPIDL

2016-08-24 Thread Nicholas Nethercote
On Wed, Aug 24, 2016 at 9:38 PM, Gijs Kruitbosch wrote: > > Can this problem be solved with more of these types of > annotations/"compiler-aids", that is, could we teach the compiler to accept > either nsresult or out checks at least for simple xpidl property fetches? >

Re: New [must_use] property in XPIDL

2016-08-24 Thread Mike Hommey
On Mon, Aug 22, 2016 at 04:39:15PM -0700, R Kent James wrote: > On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: > > I strongly encourage people to do likewise on > > any IDL files with which they are familiar. Adding appropriate checks isn't > > always easy > > Exactly, and I hope that you and

Re: New [must_use] property in XPIDL

2016-08-24 Thread Jim Blandy
On Wed, Aug 24, 2016 at 12:17 PM, R Kent James wrote: > But for the record, there are LOTS of issues in Mozilla code that are > missing a "part of writing production code" My own pet peeve is JS code > that gives no hint about the types of inputs, when there are complex >

Re: New [must_use] property in XPIDL

2016-08-24 Thread R Kent James
On 8/24/2016 11:31 AM, Jim Blandy wrote: > On Mon, Aug 22, 2016 at 4:39 PM, R Kent James > wrote: > > Exactly, and I hope that you and others restrain your exuberance a > little bit for this reason. A warning would be one thing, but a hard >

Re: New [must_use] property in XPIDL

2016-08-24 Thread Jim Blandy
On Mon, Aug 22, 2016 at 4:39 PM, R Kent James wrote: > Exactly, and I hope that you and others restrain your exuberance a > little bit for this reason. A warning would be one thing, but a hard > failure that forces developers to drop what they are doing and think > hard about an

Re: New [must_use] property in XPIDL

2016-08-24 Thread Boris Zbarsky
On 8/24/16 7:38 AM, Gijs Kruitbosch wrote: Separately, the "return failure but also set pointer to non-null" concept sounds very strange to me. There are two possible ways of thinking about outparams in XPCOM: 1) The callee must not touch the outparam values when returning an error result.

Re: New [must_use] property in XPIDL

2016-08-24 Thread Gijs Kruitbosch
On 24/08/2016 01:29, Nicholas Nethercote wrote: On Tue, Aug 23, 2016 at 3:22 PM, R Kent James wrote: A common programming pattern that I would use, when I don't really care about why something failed, is: nsCOMPtr inputStream;

Re: New [must_use] property in XPIDL

2016-08-24 Thread Jörg Knobloch
On 23/08/2016 06:28, Nicholas Nethercote wrote: I see that I've caused several Thunderbird breakages with the changes I've been making. I apologize for not giving you advance warning about this. Since I'm the guy who was, as Kent said, "forced [...] to drop everything", I think some warning

Re: New [must_use] property in XPIDL

2016-08-23 Thread Nicholas Nethercote
On Tue, Aug 23, 2016 at 11:40 PM, Benjamin Smedberg wrote: > cast-to-void is commonly suggested as an alternative to an explicit unused > marking, and it is something that I wanted to use originally. > Unfortunately, we have not been able to make that work: this is

Re: New [must_use] property in XPIDL

2016-08-23 Thread Gerald Squelart
On the build-time vs static analysis point: I'd much prefer to have errors pointed out right from './mach build', which I can fix on the spot; rather than wait hours until I notice static analysis errors on a treeherder build. (e.g., I always forget explicit/MOZ_IMPLICIT!) Unless we can get

Re: New [must_use] property in XPIDL

2016-08-23 Thread Michael Layzell
We already have mozilla::Unused in mfbt/Unused.h. You can use it as `mozilla::unused << SomethingReturningAValue();`. I believe that the existing static analyses which look at unused variables respect this. On Tue, Aug 23, 2016 at 9:47 AM, Eric Rescorla wrote: > Fair enough. I

Re: New [must_use] property in XPIDL

2016-08-23 Thread Eric Rescorla
Fair enough. I wouldn't be against introducing a separate unused marker for this purpose. -Ekr On Tue, Aug 23, 2016 at 6:40 AM, Benjamin Smedberg wrote: > cast-to-void is commonly suggested as an alternative to an explicit unused > marking, and it is something that I

Re: New [must_use] property in XPIDL

2016-08-23 Thread Benjamin Smedberg
cast-to-void is commonly suggested as an alternative to an explicit unused marking, and it is something that I wanted to use originally. Unfortunately, we have not been able to make that work: this is primarily because compilers often remove the cast-to-void as part of the parsing phase, so it's

Re: New [must_use] property in XPIDL

2016-08-23 Thread Eric Rescorla
I'm indifferent to this particular case, but I think that rkent's point about static checking is a good one. Given that C++ has existing annotations that say: - This does not produce a useful return value (return void) - I am explicitly ignoring the return value (cast to void) And that we have

Re: New [must_use] property in XPIDL

2016-08-22 Thread R Kent James
On 8/22/2016 9:28 PM, Nicholas Nethercote wrote: > On Tue, Aug 23, 2016 at 9:39 AM, R Kent James wrote: >> On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: >>> I strongly encourage people to do likewise on >>> any IDL files with which they are familiar. Adding appropriate checks

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nicholas Nethercote
On Mon, Aug 22, 2016 at 2:14 PM, Nicholas Nethercote wrote: > > If you file a bug to add [must_use] or MOZ_MUST_USE somewhere, please make the > bug block the tracking bug Some recent work on this front: - Jan Varga used [must_use] in

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nicholas Nethercote
On Tue, Aug 23, 2016 at 9:39 AM, R Kent James wrote: > On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: >> I strongly encourage people to do likewise on >> any IDL files with which they are familiar. Adding appropriate checks isn't >> always easy > > Exactly, and I hope that you

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nathan Froyd
On Mon, Aug 22, 2016 at 7:39 PM, R Kent James wrote: > On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: >> I strongly encourage people to do likewise on >> any IDL files with which they are familiar. Adding appropriate checks isn't >> always easy > > Exactly, and I hope that you

Re: New [must_use] property in XPIDL

2016-08-22 Thread Nick Fitzgerald
On Mon, Aug 22, 2016 at 4:39 PM, R Kent James wrote: > On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: > > I strongly encourage people to do likewise on > > any IDL files with which they are familiar. Adding appropriate checks > isn't > > always easy > > Exactly, and I hope

Re: New [must_use] property in XPIDL

2016-08-22 Thread Bobby Holley
On Mon, Aug 22, 2016 at 4:39 PM, R Kent James wrote: > On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: > > I strongly encourage people to do likewise on > > any IDL files with which they are familiar. Adding appropriate checks > isn't > > always easy > > Exactly, and I hope

Re: New [must_use] property in XPIDL

2016-08-22 Thread R Kent James
On 8/21/2016 9:14 PM, Nicholas Nethercote wrote: > I strongly encourage people to do likewise on > any IDL files with which they are familiar. Adding appropriate checks isn't > always easy Exactly, and I hope that you and others restrain your exuberance a little bit for this reason. A warning

New [must_use] property in XPIDL

2016-08-21 Thread Nicholas Nethercote
Greetings, I just added [1] a new property to XPIDL called [must_use]. When used with an IDL method, it will add MOZ_MUST_USE to the generated C++ declarations and macros relating to that method. Here is an example: /* [must_use] void init (in nsIFile file); */ MOZ_MUST_USE NS_IMETHOD Init