NFC: Remove simple_ilist comment mentioning ilist/iplist allocating

[lanza]

Summary

Allocation was removed from ilist in 2016

[dexonsmith]

LGTM. Thanks! (Please reference the commit that removed allocation in the commit message.)

[lanza]

On a similar note, I noticed elsewhere after fixing this that you mentioned in a commit message that you intended to remove iplist and ilist and rename as owning_ilist (or something of that sorts). Is that still on the table? Either way, the docs should be updated all around to account for either path name as there are still references to the pre-2016 implementation (e.g. https://llvm.org/docs/ProgrammersManual.html#dss-iplist).

[dexonsmith]

In D93953#2486016, @lanza wrote:

On a similar note, I noticed elsewhere after fixing this that you mentioned in a commit message that you intended to remove iplist and ilist and rename as owning_ilist (or something of that sorts). Is that still on the table?

TBH, a little while after I fixed the UB that was causing miscompiles for us, I was pulled into other things and never got back to this. I'm certainly not working on it right now, although I'm happy to review things in this area (and maybe I'll dive back in a some point if no one else does...).

Either way, the docs should be updated all around to account for either path name as there are still references to the pre-2016 implementation (e.g. https://llvm.org/docs/ProgrammersManual.html#dss-iplist).

Nice catch! If you're interested in writing a patch to fix the docs, that'd be awesome (especially since you seem to have a list of places that need updating), but let me know if not and I'll try to clean it up myself.

[lanza]

TBH, a little while after I fixed the UB that was causing miscompiles for us, I was pulled into other things and never got back to this. I'm certainly not working on it right now, although I'm happy to review things in this area (and maybe I'll dive back in a some point if no one else does...).

Am I recalling correctly that you more or less just wanted to do a wholesale sed 's/\(i\|p\)list/owning_ilist/g (with a more thorough script. of course) across the code base? Or was there more to it?

[dexonsmith]

In D93953#2514784, @lanza wrote:

TBH, a little while after I fixed the UB that was causing miscompiles for us, I was pulled into other things and never got back to this. I'm certainly not working on it right now, although I'm happy to review things in this area (and maybe I'll dive back in a some point if no one else does...).

Am I recalling correctly that you more or less just wanted to do a wholesale sed 's/\(i\|p\)list/owning_ilist/g (with a more thorough script. of course) across the code base? Or was there more to it?

Yes, that sounds right to me; I don't remember if I got around to doing an RFC, and if even I did, it probably deserves a new one (on llvm-dev). ilist is pretty broadly used by out-of-tree code as well as in-tree so it could be could be controversial if the churn isn't justified; IMO, a clear name is better, but an RFC would be good I think.

llvm/include/llvm/ADT/simple_ilist.h

 
 namespace llvm {
 
 /// A simple intrusive list implementation.
 ///
 /// This is a simple intrusive list for a \c T that inherits from \c
 /// ilist_node<T>.  The list never takes ownership of anything inserted in it.
 ///
-/// Unlike \a iplist<T> and \a ilist<T>, \a simple_ilist<T> never allocates or
-/// deletes values, and has no callback traits.
+/// Unlike \a iplist<T> and \a ilist<T>, \a simple_ilist<T> never deletes
+/// values, and has no callback traits.
 ///
 /// The API for adding nodes include \a push_front(), \a push_back(), and \a
 /// insert().  These all take values by reference (not by pointer), except for
 /// the range version of \a insert().
 ///
 /// There are three sets of API for discarding nodes from the list: \a
 /// remove(), which takes a reference to the node to remove, \a erase(), which
 /// takes an iterator or iterator range and returns the next one, and \a