[PATCH] Own/produce FrontendActionFactories by value, rather than new'd/unique_ptr ownership.

[Manuel Klimek]

 On Tue, Sep 16, 2014, 00:23 David Blaikie <dblaikie at gmail.com> wrote:

>>! In D4313#12, @klimek wrote:
> This has now been resolved differently, right? Can we abandon the
revision?

Not exactly - r207396 made the newFrontendActionFactory change to return
unique_ptr, but the original memory leak could still easily be written
because it takes a raw pointer (this both complicates callers (having them
either use "newFrontendActionFactory(...).get()" or take the address of a
local to pass) and risks someone passing a raw 'new' to the function)).

 Isn't that just DI? Are you suggesting to not use DI any more just because
somebody might hand in an unowned pointer?


The follow-on refactor to have newFrontendActionFactory return by value (&
then, potentially, remove its virtual dtor) is just to simplify the call
sites even further. Rather than replacing
"run(newFrontendActionFactory(...).get())" with
"run(*newFrontendActionFactory(...))" (which might make people twitch a
bit, wondering whether that leaks or not) it could just be
"run(newFrontendActionFactory(...))" which seems less surprising.

I figure if I'm going to have to touch every caller anyway, it wasn't
really extra churn to change newFrontendActionFactory to be value-based.

And there's still the matter of runToolOnCode taking ownership via raw
pointer (which then feeds into that confusion we had over ToolInvocation
needing to take ownership and SingleFrontendActionFactory taking/returning
ownership of that FrontendActionFactory). But that's fairly separable.

http://reviews.llvm.org/D4313
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140916/7c2bbd2a/attachment.html>