[PATCH] D156618: [IR] Fix a memory leak if Function::dropAllReferences() is followed by setHungoffOperand

Duncan P. N. Exon Smith via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Sep 6 11:57:41 PDT 2023


dexonsmith requested changes to this revision.
dexonsmith added a comment.
This revision now requires changes to proceed.

I looked at the documentation for `User::dropAllReferences()`, which this overrides. That has strengthened my opinion so I'm requesting changes.

Here are the docs:

  /// Drop all references to operands.
  ///
  /// This function is in charge of "letting go" of all objects that this User
  /// refers to.  This allows one to 'delete' a whole class at a time, even
  /// though there may be circular references...  First all references are
  /// dropped, and all use counts go to zero.  Then everything is deleted for
  /// real.  Note that no operations are valid on an object that has "dropped
  /// all references", except operator delete.

I don't think it's appropriate for the `Function` override to behave differently.

Instead, it looks like `Function::deleteBody()` should stop calling `dropAllReferences()`. Probably a private helper could be extracted from `dropAllReferences()` since you'd want to do almost all the same things (except, don't call `User::dropAllReferences()`, and do add references to `ptr null`).

In D156618#4637792 <https://reviews.llvm.org/D156618#4637792>, @taolq wrote:

>> What is "dector"?
>
> I made a typo. I mean dtor, destructor.

Thanks, I probably should have guessed that!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156618



More information about the llvm-commits mailing list