[llvm] r212403 - Declare variable on first use.

[David Blaikie]

On Mon, Jul 7, 2014 at 4:45 PM, Alp Toker <alp at nuanti.com> wrote:
>
> On 08/07/2014 02:05, David Blaikie wrote:
>>
>> I see you've been playing around with LTO/createFromBuffer - any
>> chance of createFromBuffer returning by value (possibly Optional<T> or
>> ErrorOr<T> so it can return the invalid result) or std::unique_ptr?
>> Reason it can't? (just seeing those raw pointers & wondering)
>
>
> Surely create* functions other allocation functions should return raw
> pointers, leaving it up to the caller how to adopt them?

I disagree and it's certainly become pretty common for factory
functions to return unique_ptrs to enshrine the transfer of ownership
in the type system. If a caller wants to ref count it, they can easily
release it from the unique_ptr, but that domain-jump is explicit and
clear without leaving raw pointers with potentially unclear ownership
lying around.

> This is built right into the C++ programming language (new/delete),

new/delete may be the language primitives, but there's good reasons to
have make_shared and make_unique and other things that ensure
ownership from an early stage. That they can be implemented as library
features rather than language features doesn't make them less valuable
(ie: just because the language primitives don't have ownership
semantics doesn't mean we should prefer to mirror that)

> verifiable using all kinds of checkers and instantly recognisable to C and
> C++ developers.

I'd rather not learn on asynchronous checkers (either because static
analysis is slow, or runtime analysis requires executing all/obscure
code paths). We saw even a few weeks ago with the MSan leak cleanup
that we weren't exactly leak free. Building that into the type system
makes leaks far less likely/easy.

> I'm really not keen on wrapping these return types in multiple layers of
> templates, and I don't think anyone seriously thinks the current situation
> ErrorOr<std::unique_ptr<MemoryBuffer>> is turning out well.

I can't say I've seen any major discussion/dissent about this pattern.
But if you've interpreted lack of discussion as "no one thinks this is
turning out well", I'll add my own voice here: "I think
ErrorOr<unique_ptr<T>> is a good pattern".

(the only thing I dislike about these patterns is that unique_ptr is
possibly null - I'd love it if null-ness and ownership were separate
(but composable with std::experimental::optional) so that
ErrorOr<unique_ptr<T>> was either an error or an owning pointer to a T
object (not null))

> Ultimately every raw pointer in the codebase could be wrapped with some kind
> of smart pointer,

Not every raw pointer. Only those that actually describe ownership -
there are many non-owning pointers (optional, rebindable, references).

>  but it'd achieve little other than wearing down < and >
> keys, so let's avoid going that way.

I disagree. I'm strongly in favor of going that way and believe it
achieves a lot more than wearing down < and > keys. It makes ownership
and ownership transfer explicit (making it harder to write memory
leaks) and much harder to get wrong in the face of early returns,
error paths, multiple function parameters/subexpressions, etc.

- David