>>! 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
