[llvm] r214533 - Replace comment about ownership with std::unique_ptr.

[David Blaikie]

On Fri, Aug 1, 2014 at 11:09 AM, Rafael Espindola
<rafael.espindola at gmail.com> wrote:
> Author: rafael
> Date: Fri Aug  1 13:09:32 2014
> New Revision: 214533
>
> URL: http://llvm.org/viewvc/llvm-project?rev=214533&view=rev
> Log:
> Replace comment about ownership with std::unique_ptr.
>
> Modified:
>     llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h
>     llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
>     llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h
>     llvm/trunk/tools/lli/lli.cpp
>
> Modified: llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h?rev=214533&r1=214532&r2=214533&view=diff
> ==============================================================================
> --- llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h (original)
> +++ llvm/trunk/include/llvm/ExecutionEngine/ExecutionEngine.h Fri Aug  1 13:09:32 2014
> @@ -196,9 +196,7 @@ public:
>    /// resolve external symbols in objects it is loading.  If a symbol is found
>    /// in the Archive the contained object file will be extracted (in memory)
>    /// and loaded for possible execution.
> -  ///
> -  /// MCJIT will take ownership of the Archive.
> -  virtual void addArchive(object::Archive *A) {
> +  virtual void addArchive(std::unique_ptr<object::Archive> A) {
>      llvm_unreachable("ExecutionEngine subclass doesn't implement addArchive.");
>    }
>
>
> Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp?rev=214533&r1=214532&r2=214533&view=diff
> ==============================================================================
> --- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp (original)
> +++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.cpp Fri Aug  1 13:09:32 2014
> @@ -87,10 +87,6 @@ MCJIT::~MCJIT() {
>    }
>    LoadedObjects.clear();
>
> -
> -  for (object::Archive *A : Archives)
> -    delete A;
> -
>    Archives.clear();
>
>    delete TM;
> @@ -118,8 +114,8 @@ void MCJIT::addObjectFile(std::unique_pt
>    NotifyObjectEmitted(*LoadedObject);
>  }
>
> -void MCJIT::addArchive(object::Archive *A) {
> -  Archives.push_back(A);
> +void MCJIT::addArchive(std::unique_ptr<object::Archive> A) {
> +  Archives.push_back(std::move(A));
>  }
>
>
> @@ -294,7 +290,7 @@ uint64_t MCJIT::getSymbolAddress(const s
>    if (Addr)
>      return Addr;
>
> -  for (object::Archive *A : Archives) {
> +  for (std::unique_ptr<object::Archive> &A : Archives) {
>      // Look for our symbols in each Archive
>      object::Archive::child_iterator ChildIt = A->findSym(Name);
>      if (ChildIt != A->child_end()) {
>
> Modified: llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h?rev=214533&r1=214532&r2=214533&view=diff
> ==============================================================================
> --- llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h (original)
> +++ llvm/trunk/lib/ExecutionEngine/MCJIT/MCJIT.h Fri Aug  1 13:09:32 2014
> @@ -215,7 +215,7 @@ class MCJIT : public ExecutionEngine {
>
>    OwningModuleContainer OwnedModules;
>
> -  SmallVector<object::Archive*, 2> Archives;
> +  SmallVector<std::unique_ptr<object::Archive>, 2> Archives;
>
>    typedef SmallVector<ObjectImage *, 2> LoadedObjectList;
>    LoadedObjectList  LoadedObjects;
> @@ -239,7 +239,7 @@ public:
>    /// @{
>    void addModule(Module *M) override;
>    void addObjectFile(std::unique_ptr<object::ObjectFile> O) override;
> -  void addArchive(object::Archive *O) override;
> +  void addArchive(std::unique_ptr<object::Archive> O) override;
>    bool removeModule(Module *M) override;
>
>    /// FindFunctionNamed - Search all of the active modules to find the one that
>
> Modified: llvm/trunk/tools/lli/lli.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/lli/lli.cpp?rev=214533&r1=214532&r2=214533&view=diff
> ==============================================================================
> --- llvm/trunk/tools/lli/lli.cpp (original)
> +++ llvm/trunk/tools/lli/lli.cpp Fri Aug  1 13:09:32 2014
> @@ -545,12 +545,14 @@ int main(int argc, char **argv, char * c
>        return 1;
>      }
>      std::error_code EC;
> -    object::Archive *Ar = new object::Archive(std::move(ArBuf.get()), EC);
> -    if (EC || !Ar) {
> +    std::unique_ptr<object::Archive> Ar =
> +        llvm::make_unique<object::Archive>(std::move(ArBuf.get()), EC);

auto, rather than repeating the type on both sides of the '='?

Or maybe this code could use Archive::create instead? (I'm
considering/trying to refactor Archive::create so it doesn't rely on
the constructor providing an error code - so not having multiple
places relying on this would be for the best)

> +    assert(Ar);
> +    if (EC) {
>        Err.print(argv[0], errs());

Is this the right error handling? "Err" is for errors from
ParseIRFile, right? "EC" is where the errors for this failure path
are?

>        return 1;
>      }
> -    EE->addArchive(Ar);
> +    EE->addArchive(std::move(Ar));
>    }
>
>    // If the target is Cygwin/MingW and we are generating remote code, we
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits