[lld] r226225 - Simplify.

[David Blaikie]

On Sat, Jan 17, 2015 at 6:25 AM, Chandler Carruth <chandlerc at google.com>
wrote:

>
> On Sat, Jan 17, 2015 at 6:15 AM, Jean-Daniel Dupas <dev at xenonium.com>
> wrote:
>
>> > Le 16 janv. 2015 à 18:48, David Blaikie <dblaikie at gmail.com> a écrit :
>> >
>> >
>> >
>> > On Fri, Jan 16, 2015 at 3:02 AM, Jean-Daniel Dupas <dev at xenonium.com>
>> wrote:
>> > Just a question not directly related to that change, but similar. Is
>> there something that prevent the use of vector.emplace_back() in the lld
>> code base (compiler or supported c++ library issue) ?
>> >
>> > There is some places where we just create a new object and push it back
>> in a vector and could simplify them using emplace_back() instead.
>> >
>> > Personally I wouldn't use "emplace_back(new T())" on a container of
>> unique_ptr<T> because of exception safety, etc (though we don't use
>> exceptions, it's habits that make me more comfortable reading code, etc).
>> The risk is that emplace_back could fail (when trying to allocate more
>> space, for example) and then leak the T* before it ever created a
>> unique_ptr<T> around it.
>> >
>> > So I prefer "push_back(make_unique<T>())" (& in general I prefer
>> make_unique wherever possible, it means I don't have to think hard about
>> whether the 'new' is used safely, etc)
>> >
>>
>> Of course. I was thinking about simplifying construct like:
>>
>>   Section temp;
>>   file.sections.push_back(std::move(temp));
>>
>> or
>>
>>   pages.push_back(UnwindInfoPage());
>>
>
> Personally I find emplace_back significantly harder to read for little
> benefit. It hides the constructor call behind a fairly magical forwarding
> function. I would generally rather see a constructor call explicitly. The
> only time this bothers me is when it isn't really a constructor at all but
> just an aggregate thing. There, I would rather use list initialization, but
> we can't because of MSVC 2012.
>

FWIW I'm not strongly opposed to things like calling emplace_back without
args in the above cases, but neither do I find it to be particularly
cleaner.


> The times when emplace_back seems useful is when there is a specific
> reason that you need to construct things in place.
>

I generally agree & don't tend to use emplace_back unless I pretty much
need to - I usually assume movement is cheap enough that the lesser
strength of push_back (no explicit conversions can occur, etc) is a better
choice for readability.

(tangent: similar to the reasons I prefer "T t = ...;" over "T t(...);" -
making it explicit that no explicit conversions can occur here (really
helps with unique_ptr - the latter syntax (especially if ... is a function
call) might involve taking ownership of a raw pointer and I have to wonder
if the raw pointer is owning, if I'm allowed to take ownership, etc - the
former I know is type checked by the language, etc))
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150117/9011f8af/attachment.html>