Re: Hiding 'new' statements - Good or Evil?

I would say it comes down to the module. I'm very comfortable in my modules using auto, perhaps because our lifetime management is more clear. To me, the unsafe examples are pretty strawman-y, since it's very clear that there are at-risk lifetimes involved. (Returning a bare pointer and relying on

Re: Hiding 'new' statements - Good or Evil?

[SNIP] >> If foo->bar() can mutate the lifetime of foo, of course you must take a >> strong >> reference to foo. Nothing about auto or range-for changes this. > >What auto does is make it really hard to tell whether a strong reference is >being taken. > >> If you don't understand your lifetimes,

Re: Hiding 'new' statements - Good or Evil?

On 11/28/17 10:28 PM, Jeff Gilbert wrote: const auto& foo = getFoo(); foo->bar(); // foo is always safe here Sadly not true. Some counterexamples: Foo* getFoo() { return mFoo; // Hey, this is the common case! } const RefPtr& getFoo() { return mFoo; // Can be nulled out by the

Re: Hiding 'new' statements - Good or Evil?

const auto& foo = getFoo(); foo->bar(); // foo is always safe here Use of auto instead of auto& should be less common. I only use it for: Casts: `const auto foo = static_cast(bar);` Or long (but obvious) types I need a value of, usually RefPtrs of long types. Almost every other auto is `const

Re: Hiding 'new' statements - Good or Evil?

On 11/28/2017 06:33 AM, Boris Zbarsky wrote: On 11/27/17 7:45 PM, Eric Rescorla wrote: As for the lifetime question, can you elaborate on the scenario you are concerned about. Olli may have a different concern, but I'm thinking something like this: for (auto foo : myFoos) { foo->bar();

Re: Hiding 'new' statements - Good or Evil?

On Wed, Nov 29, 2017, at 12:56 AM, Eric Rescorla wrote: > On Mon, Nov 27, 2017 at 6:41 PM, Xidorn Quan wrote: >> On Tue, Nov 28, 2017, at 11:45 AM, Eric Rescorla wrote: >> > On Mon, Nov 27, 2017 at 4:07 PM, smaug wrote: >> > > And auto makes code reading

Re: Hiding 'new' statements - Good or Evil?

On Mon, Nov 27, 2017 at 6:41 PM, Xidorn Quan wrote: > On Tue, Nov 28, 2017, at 11:45 AM, Eric Rescorla wrote: > > On Mon, Nov 27, 2017 at 4:07 PM, smaug wrote: > > > And auto makes code reading harder. It hides important information like > > > lifetime

Re: Hiding 'new' statements - Good or Evil?

On 11/27/17 7:45 PM, Eric Rescorla wrote: As for the lifetime question, can you elaborate on the scenario you are concerned about. Olli may have a different concern, but I'm thinking something like this: for (auto foo : myFoos) { foo->bar(); } where bar() can run arbitrary script.

Re: Hiding 'new' statements - Good or Evil?

On Tue, Nov 28, 2017, at 11:45 AM, Eric Rescorla wrote: > On Mon, Nov 27, 2017 at 4:07 PM, smaug wrote: > > And auto makes code reading harder. It hides important information like > > lifetime management. > > It happens easily with auto that one doesn't even start to think

Re: Hiding 'new' statements - Good or Evil?

On Mon, Nov 27, 2017 at 4:07 PM, smaug wrote: > On 11/28/2017 12:53 AM, Jeff Gilbert wrote: > >> ranged-for issues are the same as those for doing manual iteration, >> > It is not, in case you iterate using > for (i = 0; i < foo.length(); ++i) > And that is the case which has

Re: Hiding 'new' statements - Good or Evil?

On Nov 27, 2017, at 6:07 PM, smaug wrote: > > And auto makes code reading harder. It hides important information like > lifetime management. > It happens easily with auto that one doesn't even start to think whether > nsCOMPtr/RefPtr should be used there. > > I'm saying this

Re: Hiding 'new' statements - Good or Evil?

On 11/28/2017 12:53 AM, Jeff Gilbert wrote: ranged-for issues are the same as those for doing manual iteration, It is not, in case you iterate using for (i = 0; i < foo.length(); ++i) And that is the case which has been often converted to ranged-for, with bad results. And auto makes code

Re: Hiding 'new' statements - Good or Evil?

ranged-for issues are the same as those for doing manual iteration, and your complaints about auto are caused by deficient code/design review. The further we deviate from standards, the harder it is for contributors (and not just new ones) to do the right thing. The default should be to move

Re: Hiding 'new' statements - Good or Evil?

On 11/27/2017 01:05 PM, Nicolas B. Pierron wrote: On 11/26/2017 12:45 AM, smaug wrote: On 11/24/2017 06:35 PM, Eric Rescorla wrote: On Thu, Nov 23, 2017 at 4:00 PM, smaug wrote: On 11/23/2017 11:54 PM, Botond Ballo wrote: I think it makes sense to hide a 'new' call in a

Re: Hiding 'new' statements - Good or Evil?

On 11/26/2017 12:45 AM, smaug wrote: On 11/24/2017 06:35 PM, Eric Rescorla wrote: On Thu, Nov 23, 2017 at 4:00 PM, smaug wrote: On 11/23/2017 11:54 PM, Botond Ballo wrote: I think it makes sense to hide a 'new' call in a Make* function when you're writing an abstraction

Re: Hiding 'new' statements - Good or Evil?

On Fri, Nov 24, 2017 at 11:35 AM, Eric Rescorla wrote: > On Thu, Nov 23, 2017 at 4:00 PM, smaug wrote: >> I guess I'd prefer UniquePtr::New() over MakeUnique to be more clear about >> the functionality. > > This seems like a reasonable argument in isolation, but I

Re: Hiding 'new' statements - Good or Evil?

On 11/24/2017 06:35 PM, Eric Rescorla wrote: On Thu, Nov 23, 2017 at 4:00 PM, smaug wrote: On 11/23/2017 11:54 PM, Botond Ballo wrote: I think it makes sense to hide a 'new' call in a Make* function when you're writing an abstraction that handles allocation *and*

Re: Hiding 'new' statements - Good or Evil?

On Thu, Nov 23, 2017 at 4:00 PM, smaug wrote: > On 11/23/2017 11:54 PM, Botond Ballo wrote: > >> I think it makes sense to hide a 'new' call in a Make* function when >> you're writing an abstraction that handles allocation *and* >> deallocation. >> >> So MakeUnique makes sense,

Re: Hiding 'new' statements - Good or Evil?

On 11/23/2017 11:54 PM, Botond Ballo wrote: I think it makes sense to hide a 'new' call in a Make* function when you're writing an abstraction that handles allocation *and* deallocation. So MakeUnique makes sense, because UniquePtr takes ownership of the allocated object, and will deallocate it

Re: Hiding 'new' statements - Good or Evil?

I think it makes sense to hide a 'new' call in a Make* function when you're writing an abstraction that handles allocation *and* deallocation. So MakeUnique makes sense, because UniquePtr takes ownership of the allocated object, and will deallocate it in the destructor. MakeRefPtr would make

Re: Hiding 'new' statements - Good or Evil?

On 23/11/2017 12:05, Jeff Gilbert wrote: I agree that MakeNotNull doesn't sound like it allocates. Reads to me as Make[Into]NotNull. Context will *usually* make this clear, but why not NewNotNull? (Honestly I don't like MakeUnique to begin with) As far as naming is concerned, ISTM that

Re: Hiding 'new' statements - Good or Evil?

I agree that MakeNotNull doesn't sound like it allocates. Reads to me as Make[Into]NotNull. Context will *usually* make this clear, but why not NewNotNull? (Honestly I don't like MakeUnique to begin with) NotNull::New(args...) is another option. "My first goal was to avoid the unnecessary

Re: Hiding 'new' statements - Good or Evil?

I'm not sure what the benefits are (the MakeUnique comments only really seem to give aesthetic ones), but they do make it a bit harder when searching through code to see where things are constructed. I suppose if we always stick to using Make* then (with regex) it wouldn't add too much burden

Hiding 'new' statements - Good or Evil?

Should we allow hiding 'new' statements, or keep them as visible as possible? Some context: Recently in bug 1410252 I added a MakeNotNull(args...) function that does `NotNull(new T(args...))`, in the style of MakeUnique and others. It also works with RefPtr. My first goal was to avoid