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 towards the standard, not cringe back from it. Sure, prove it out. But we really don't need more moz-specific constructs. We should choose our deviations carefully. On Mon, Nov 27, 2017 at 3:24 AM, smaug <sm...@welho.com> wrote: > 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 <sm...@welho.com> 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, because UniquePtr takes ownership of the >>>>>> allocated object, and will deallocate it in the destructor. MakeRefPtr >>>>>> would make sense for the same reason. >>>>>> >>>>> I almost agree with this, but, all these Make* variants hide the >>>>> information that they are allocating, >>>>> and since allocation is slow, it is usually better to know when >>>>> allocation >>>>> happens. >>>>> 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 think it's >>>> more >>>> important to mirror the standard C++ mechanisms and C++-14 already >>>> defines >>>> std::make_unique. >>> >>> >>> Is it? Isn't it more important to write less error prone code and code >>> where the reader >>> sees the possible performance issues. >>> Recent-ish C++ additions have brought quite a few bad concepts which we >>> shouldn't use >>> without considering whether they actually fit into our needs. >>> Something being new and hip and cool doesn't mean it is actually a good >>> thing. >> >> >> One example, is that the standard has a single std::function type which >> always allocate, and does not report failures because we disable exceptions. >> >> In this particular case I would recommend to have a stack-function wrapper >> and a heap-function wrapper. By diverging from the standard we could avoid >> bugs such as refusing stack-references for lambda being wrapped in >> heap-function (use-after-free/unwind), and adding extra checks that >> stack-function >> wrapper can only be fields of stack-only classes. C++ compilers have no >> way to understand the code enough to provide similar life-time errors. >> >>> (This broken record keeps reminding about the security issues and memory >>> leaks auto and ranged-for have caused.) >> >> >> Do you have pointers? Is this basically when we have a non-standard >> implementation of begin/end methods which are doing allocation? > > > ranged-for causes issues when you modify the data structure while iterating. > This used to be bad in nsTArray case at least, now we just crash safely. > > > And auto hides the type, so reading the code and understanding lifetime > management becomes harder. > This has caused security sensitive crashes and leaks. (And there was some > other type of error too recently, but it escapes my mind now) > > >> >>> >>>> >>>> As a post-script, given that we now can use C++-14, can we globally >>>> replace >>>> the MFBT clones of C++-14 mechanisms with the standard ones? >>>> >>>> -Ekr >>>> >>>> >>>>> >>>>> >>>>> -Olli >>>>> >>>>> >>>>> >>>>>> But in cases where a library facility is not taking ownership of the >>>>>> object, and thus the user will need to write an explicit 'delete', it >>>>>> makes sense to require that the user also write an explicit 'new', for >>>>>> symmetry. >>>>>> >>>>>> NotNull is a bit of a weird case because it can wrap either a raw >>>>>> pointer or a smart pointer, so it doesn't clearly fit into either >>>>>> category. Perhaps it would make sense for MakeNotNull to only be >>>>>> usable with smart pointers? >>>>>> >>>>>> Cheers, >>>>>> Botond >>>>>> >>>>>> >>>>> _______________________________________________ >>>>> 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 _______________________________________________ dev-platform mailing list dev-platform@lists.mozilla.org https://lists.mozilla.org/listinfo/dev-platform
