[lld] r226225 - Simplify.

[David Blaikie]

On Fri, Jan 16, 2015 at 3:02 AM, Jean-Daniel Dupas <dev at xenonium.com> wrote:

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

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.

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)


>
> > Le 16 janv. 2015 à 00:15, Rui Ueyama <ruiu at google.com> a écrit :
> >
> > Author: ruiu
> > Date: Thu Jan 15 17:15:09 2015
> > New Revision: 226225
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=226225&view=rev
> > Log:
> > Simplify.
> >
> > Modified:
> >    lld/trunk/lib/Driver/CoreDriver.cpp
> >    lld/trunk/lib/Driver/GnuLdDriver.cpp
> >    lld/trunk/lib/Driver/WinLinkDriver.cpp
> >
> > Modified: lld/trunk/lib/Driver/CoreDriver.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/CoreDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff
> >
> ==============================================================================
> > --- lld/trunk/lib/Driver/CoreDriver.cpp (original)
> > +++ lld/trunk/lib/Driver/CoreDriver.cpp Thu Jan 15 17:15:09 2015
> > @@ -150,10 +150,8 @@ bool CoreDriver::parse(int argc, const c
> >     case OPT_INPUT: {
> >       std::vector<std::unique_ptr<File>> files
> >         = loadFile(ctx, inputArg->getValue(), false);
> > -      for (std::unique_ptr<File> &file : files) {
> > -        ctx.getNodes().push_back(std::unique_ptr<Node>(
> > -            new FileNode(std::move(file))));
> > -      }
> > +      for (std::unique_ptr<File> &file : files)
> > +
> ctx.getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));
> >       break;
> >     }
> >
> >
> > Modified: lld/trunk/lib/Driver/GnuLdDriver.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/GnuLdDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff
> >
> ==============================================================================
> > --- lld/trunk/lib/Driver/GnuLdDriver.cpp (original)
> > +++ lld/trunk/lib/Driver/GnuLdDriver.cpp Thu Jan 15 17:15:09 2015
> > @@ -273,8 +273,7 @@ evaluateLinkerScript(ELFLinkingContext &
> >       for (std::unique_ptr<File> &file : files) {
> >         if (ctx.logInputFiles())
> >           diag << file->path() << "\n";
> > -        ctx.getNodes().push_back(
> > -            std::unique_ptr<Node>(new FileNode(std::move(file))));
> > +
> ctx.getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));
> >         ++numfiles;
> >       }
> >     }
> > @@ -590,8 +589,7 @@ bool GnuLdDriver::parse(int argc, const
> >       ErrorOr<StringRef> pathOrErr = findFile(*ctx, path, dashL);
> >       if (std::error_code ec = pathOrErr.getError()) {
> >         auto file = llvm::make_unique<ErrorFile>(path, ec);
> > -        ctx->getNodes().push_back(
> > -            std::unique_ptr<FileNode>(new FileNode(std::move(file))));
> > +
> ctx->getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));
> >         break;
> >       }
> >       std::string realpath = pathOrErr.get();
> > @@ -614,8 +612,7 @@ bool GnuLdDriver::parse(int argc, const
> >       for (std::unique_ptr<File> &file : files) {
> >         if (ctx->logInputFiles())
> >           diagnostics << file->path() << "\n";
> > -        ctx->getNodes().push_back(
> > -            std::unique_ptr<Node>(new FileNode(std::move(file))));
> > +
> ctx->getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));
> >       }
> >       numfiles += files.size();
> >       break;
> >
> > Modified: lld/trunk/lib/Driver/WinLinkDriver.cpp
> > URL:
> http://llvm.org/viewvc/llvm-project/lld/trunk/lib/Driver/WinLinkDriver.cpp?rev=226225&r1=226224&r2=226225&view=diff
> >
> ==============================================================================
> > --- lld/trunk/lib/Driver/WinLinkDriver.cpp (original)
> > +++ lld/trunk/lib/Driver/WinLinkDriver.cpp Thu Jan 15 17:15:09 2015
> > @@ -1414,8 +1414,7 @@ bool WinLinkDriver::parse(int argc, cons
> >       if (file->parse())
> >         return false;
> >     ctx.getResolvableSymsFile()->add(file.get());
> > -    ctx.getNodes().push_back(
> > -      std::unique_ptr<Node>(new FileNode(std::move(file))));
> > +
> ctx.getNodes().push_back(llvm::make_unique<FileNode>(std::move(file)));
> >   }
> >
> >   // Add the library group to the input graph.
> > @@ -1431,8 +1430,7 @@ bool WinLinkDriver::parse(int argc, cons
> >         if (file->parse())
> >           return false;
> >       ctx.getResolvableSymsFile()->add(file.get());
> > -      ctx.addLibraryFile(
> > -     std::unique_ptr<FileNode>(new FileNode(std::move(file))));
> > +      ctx.addLibraryFile(llvm::make_unique<FileNode>(std::move(file)));
> >     }
> >   }
> >
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20150116/ce2abd72/attachment.html>