<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jan 16, 2015 at 3:02 AM, Jean-Daniel Dupas <span dir="ltr"><<a href="mailto:dev@xenonium.com" target="_blank">dev@xenonium.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Just a question not directly related to that change, but similar. Is there something that prevent the use of vector.emplace_back() in the lld code base (compiler or supported c++ library issue) ?<br>
<br>
There is some places where we just create a new object and push it back in a vector and could simplify them using emplace_back() instead.<br></blockquote><div><br>Personally I wouldn't use "emplace_back(new T())" on a container of unique_ptr<T> because of exception safety, etc (though we don't use exceptions, it's habits that make me more comfortable reading code, etc). The risk is that emplace_back could fail (when trying to allocate more space, for example) and then leak the T* before it ever created a unique_ptr<T> around it.<br><br>So I prefer "push_back(make_unique<T>())" (& in general I prefer make_unique wherever possible, it means I don't have to think hard about whether the 'new' is used safely, etc)<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
> Le 16 janv. 2015 à 00:15, Rui Ueyama <<a href="mailto:ruiu@google.com">ruiu@google.com</a>> a écrit :<br>
<div class="HOEnZb"><div class="h5">><br>
> Author: ruiu<br>
> Date: Thu Jan 15 17:15:09 2015<br>
> New Revision: 226225<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=226225&view=rev" target="_blank">http://llvm.org/viewvc/llvm-project?rev=226225&view=rev</a><br>
> Log:<br>
> Simplify.<br>
><br>
> Modified:<br>
> lld/trunk/lib/Driver/CoreDriver.cpp<br>
> lld/trunk/lib/Driver/GnuLdDriver.cpp<br>
> lld/trunk/lib/Driver/WinLinkDriver.cpp<br>
><br>
> Modified: lld/trunk/lib/Driver/CoreDriver.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/CoreDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/CoreDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff</a><br>
> ==============================================================================<br>
> --- lld/trunk/lib/Driver/CoreDriver.cpp (original)<br>
> +++ lld/trunk/lib/Driver/CoreDriver.cpp Thu Jan 15 17:15:09 2015<br>
> @@ -150,10 +150,8 @@ bool CoreDriver::parse(int argc, const c<br>
> case OPT_INPUT: {<br>
> std::vector<std::unique_ptr<File>> files<br>
> = loadFile(ctx, inputArg->getValue(), false);<br>
> - for (std::unique_ptr<File> &file : files) {<br>
> - ctx.getNodes().push_back(std::unique_ptr<Node>(<br>
> - new FileNode(std::move(file))));<br>
> - }<br>
> + for (std::unique_ptr<File> &file : files)<br>
> + ctx.getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));<br>
> break;<br>
> }<br>
><br>
><br>
> Modified: lld/trunk/lib/Driver/GnuLdDriver.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/GnuLdDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/GnuLdDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff</a><br>
> ==============================================================================<br>
> --- lld/trunk/lib/Driver/GnuLdDriver.cpp (original)<br>
> +++ lld/trunk/lib/Driver/GnuLdDriver.cpp Thu Jan 15 17:15:09 2015<br>
> @@ -273,8 +273,7 @@ evaluateLinkerScript(ELFLinkingContext &<br>
> for (std::unique_ptr<File> &file : files) {<br>
> if (ctx.logInputFiles())<br>
> diag << file->path() << "\n";<br>
> - ctx.getNodes().push_back(<br>
> - std::unique_ptr<Node>(new FileNode(std::move(file))));<br>
> + ctx.getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));<br>
> ++numfiles;<br>
> }<br>
> }<br>
> @@ -590,8 +589,7 @@ bool GnuLdDriver::parse(int argc, const<br>
> ErrorOr<StringRef> pathOrErr = findFile(*ctx, path, dashL);<br>
> if (std::error_code ec = pathOrErr.getError()) {<br>
> auto file = llvm::make_unique<ErrorFile>(path, ec);<br>
> - ctx->getNodes().push_back(<br>
> - std::unique_ptr<FileNode>(new FileNode(std::move(file))));<br>
> + ctx->getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));<br>
> break;<br>
> }<br>
> std::string realpath = pathOrErr.get();<br>
> @@ -614,8 +612,7 @@ bool GnuLdDriver::parse(int argc, const<br>
> for (std::unique_ptr<File> &file : files) {<br>
> if (ctx->logInputFiles())<br>
> diagnostics << file->path() << "\n";<br>
> - ctx->getNodes().push_back(<br>
> - std::unique_ptr<Node>(new FileNode(std::move(file))));<br>
> + ctx->getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));<br>
> }<br>
> numfiles += files.size();<br>
> break;<br>
><br>
> Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff" target="_blank">http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff</a><br>
> ==============================================================================<br>
> --- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)<br>
> +++ lld/trunk/lib/Driver/WinLinkDriver.cpp Thu Jan 15 17:15:09 2015<br>
> @@ -1414,8 +1414,7 @@ bool WinLinkDriver::parse(int argc, cons<br>
> if (file->parse())<br>
> return false;<br>
> ctx.getResolvableSymsFile()->add(file.get());<br>
> - ctx.getNodes().push_back(<br>
> - std::unique_ptr<Node>(new FileNode(std::move(file))));<br>
> + ctx.getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));<br>
> }<br>
><br>
> // Add the library group to the input graph.<br>
> @@ -1431,8 +1430,7 @@ bool WinLinkDriver::parse(int argc, cons<br>
> if (file->parse())<br>
> return false;<br>
> ctx.getResolvableSymsFile()->add(file.get());<br>
> - ctx.addLibraryFile(<br>
> - std::unique_ptr<FileNode>(new FileNode(std::move(file))));<br>
> + ctx.addLibraryFile(llvm::make_unique<FileNode>(std::move(file)));<br>
> }<br>
> }<br>
><br>
><br>
><br>
> _______________________________________________<br>
> llvm-commits mailing list<br>
> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
</div></div></blockquote></div><br></div></div>