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

David Blaikie dblaikie at gmail.com
Mon Sep 15 15:22:58 PDT 2014


>>! 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)).



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






More information about the cfe-commits mailing list