ilya-biryukov added inline comments.

================
Comment at: lib/Tooling/Tooling.cpp:287
+  if (Files->getVirtualFileSystem() != VirtualFileSystem) {
+    Files = new FileManager(Invocation->getFileSystemOpts(), 
VirtualFileSystem);
+  }
----------------
vladimir.plyashkun wrote:
> ilya-biryukov wrote:
> > `Files` is a raw pointer, so we're leaking memory here.
> > 
> Agree.
> I can try to replace type of this field to `llvm::IntrusiveRefCntPtr<T>`  
> instead of raw pointer.
> But if i understand correctly, this class is available during whole 
> execution, so memory will be freed on exit. 
> I saw some other usages, for example, `createFileManager` method in the 
> `CompilerInstance` also just reassign value to raw pointer.
> https://clang.llvm.org/doxygen/classclang_1_1CompilerInstance.html#abeb2bbf46a8de987c227125a84935802
Using `IntrusiveRefCntPtr<FileManager>` locally should do the trick, the 
clients can take ownership if they want and `FileManager` will be properly 
freed if they don't do that.

`CompilerInstance::createFileManager` stores `IntrusiveRefCntPtr` as a field 
before returning a raw pointer, so it seems to properly manage memory there.


Repository:
  rC Clang

https://reviews.llvm.org/D41594



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to