[PATCH] Make ownership of CompilerInvocation objects explicit by using std::unique_ptr.

David Blaikie dblaikie at gmail.com
Sat Aug 9 15:33:04 PDT 2014


Fixing up a few cases of std::unique_ptr<T>::release to push explicitly typed memory ownership through several APIs.

One of the weirder things was the crash cleanup handling in libclang/Indexing.cpp - in several existing cases, as well as one I had to modify, objects are dynamically allocated, owned via std::unique_ptr, then registered with a crash handler to delete the object in case of a crash. I think these could be reasonably simplified by having a crash handler that just calls the dtor rather than delete, thus leaving these objects on the stack instead.

The general ownership code in this whole area (see other members of CompilerInvocation and other related types nearby) seems rather complex and I'm wondering if anyone has a bigger picture idea of what direction this code should be heading in. Is there some unwritten rewrite that's desired here that would simplify this ownership in another way? I'd be willing to scrap this patch and work on that, otherwise I'll probably keep muddling through with unique_ptr/shared_ptr doing mechanical cleanup and perhaps some better design will become clearer over time.

I've used a mix of unique_ptr and shared_ptr here - starting out with unique_ptr as far as it would go, then having to switch to shared_ptr in areas where sharing seems to be occurring based on my best effort to understand the ownership here. This means we don't get the single-allocation benefits of std::make_shared, but simplifies parts of the code where only single ownership is required. Open to competing attitudes/ideas in how to approach this.

http://reviews.llvm.org/D4838

Files:
  examples/clang-interpreter/main.cpp
  include/clang/Frontend/ASTUnit.h
  include/clang/Frontend/CompilerInstance.h
  include/clang/Frontend/CompilerInvocation.h
  include/clang/Frontend/Utils.h
  include/clang/Tooling/Tooling.h
  lib/ARCMigrate/ARCMT.cpp
  lib/Frontend/ASTUnit.cpp
  lib/Frontend/ChainedIncludesSource.cpp
  lib/Frontend/CompilerInstance.cpp
  lib/Frontend/CompilerInvocation.cpp
  lib/Frontend/CreateInvocationFromCommandLine.cpp
  lib/Tooling/Tooling.cpp
  tools/libclang/Indexing.cpp
  unittests/AST/ExternalASTSourceTest.cpp
  unittests/Frontend/FrontendActionTest.cpp
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D4838.12327.patch
Type: text/x-patch
Size: 34309 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140809/bc2d64e4/attachment.bin>


More information about the cfe-commits mailing list