[PATCH] D92480: [llvm] Add asserts in (ThreadSafe)?RefCountedBase destructors

Nathan James via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Dec 4 14:28:19 PST 2020


njames93 marked 6 inline comments as done and 2 inline comments as done.
njames93 added inline comments.


================
Comment at: llvm/include/llvm/Support/ManagedStatic.h:25
+// that are const with no params.
+template <typename T> struct HasRetainRelease {
+private:
----------------
dblaikie wrote:
> Are there many uses that rely on this? I don't think it's really worth all this infrastructure - compared to having it supported on an as-needed basis, such as directly in TrueMatcherImpl's ctor and dtor.
It doesn't work in TrueMatcherImpl ctor/dtor due to the whole double free issue(See [[ https://reviews.llvm.org/D92480#inline-864517 | previous comment ]]). I can go back to just supporting it for TrueMatcherImpl by writing a single custom creator/deleter for that class.

This approach taken was a little overkill but likely a little more foolproof for someone using the library. I'll go ahead with whichever approach you would prefer. 


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D92480/new/

https://reviews.llvm.org/D92480



More information about the llvm-commits mailing list