<div dir="ltr">Splitting some of this off from the "Own/produce FrontendActionFactories by value, rather than new'd/unique_ptr ownership" <br><br>This is just to change the FrontendActionFactory::create to return by unique_ptr.<br><br>But I did find a bunch of verbose and repetitious code (creating FrontendActionFactories and FrontendActions) that could be greatly simplified with some utilities and lambdas. This would be simpler if the other FrontendActionFactor utility functions were returning by value too. <br><br>Anyway, bunch of things between this patch and the other:<br><br>1) Just the FrontendActionFactory::create -> unique_ptr change (propagates to runToolOnCode* functions needing to take by unique_ptr too, as previously discussed)<br>2) More FrontendActionFactory helpers with lambdas (these could use real names - they just have placeholders at the moment)<br>3) Tool::run pass FrontendActionFactory by reference<br>4) Move existing FrontendActionFactory helpers to return by value (new* -> make* rename, probably)<br><br>1 and 2 are in these patches (but can be separated and committed in either order - or one/the other taken while dropping the other)<br><br>3 and 4 are in the other review, again - can be separated and accepted/declined<br><br>But generally all 4 would work best together.<br><br>(1) without (2) is more churn on every caller - when I could just tidy up most callers (most of the google-internal callers too - <a href="https://github.com/google/souper/blob/master/lib/ClangTool/Actions.cpp#L76">https://github.com/google/souper/blob/master/lib/ClangTool/Actions.cpp#L76</a> is the only google-internal one that needs the "less trivial" flavor - all the other internal FrontendActionFactory subclasses can be removed and replaced with makeTrivialFrontendActionFactory). <br><br>(3) makes (1) and (2) easier - moving to more return-by-value means some callers can't be easily migrated (see clang-modernize - which has a factory producing unique_ptr<FrontendActionFactory> and callers that "Tool.run(factory().get())" - until run takes a reference, that can't be migrated to return by value without having to introduce extra vsariables at the caller.<br><br>(4) would make those builder/factory/helper functions consistent with (2) and avoid the need for dynamic allocation. Only one caller would be inconvenienced by this, it's simpler for everyone else (and less subtle code: "Tool.run(*newFrontendActionFactory(...))" - wait, does newFrontendActionFactory return a raw owning pointer? Did this just leak?)<br><br>Sorry about going around and around with all this - hopefully this is a reasonable summary of some of the interdependencies/benefits... I can try to write something more structured/explicit if it'd be helpful. Happy to split/apply these in whatever parts you see fit.</div>