RFR: 8271513: support JavaThreadIteratorWithHandle replacement by new ThreadsList::Iterator [v2]

David Holmes dholmes at openjdk.java.net
Wed Aug 4 02:49:36 UTC 2021

On Tue, 3 Aug 2021 11:03:49 GMT, Kim Barrett <kbarrett at openjdk.org> wrote:

>> See above. I think I understand this now. Don't like it; but understand it.
> I'm not sure what declaration in the hpp file you are referring to.  The function declaration specifies a return type of (unqualified) `Iterator` which is declared (via the type alias) in that class.  Outside the scope of that class it needs to be qualified, i.e. `ThreadsListHandle::Iterator`.
> There's nothing bizarre about it.  Maybe this looks unfamiliar because nested types are relatively uncommon in HotSpot, and even more so using such in a return type for a definition outside the body of the class.
> The definition can't use unqualified `Iterator` because that wouldn't compile.  The leading return type isn't in the scope of the class of the member, so must itself be qualified too.  It's qualified by the class of the member, so that it refers to the name in that class.  The scoping rules for the return type of a member function are an annoying bit of C++ syntax that can be dealt with differently using C++11 trailing return types , but that's a not yet discussed feature in the style guide.  That is, this could have been written as
> `auto ThreadsListHandle::begin() -> Iterator { return list()->begin(); }`
> (note the arrow pointing to the return type, which doesn't need qualification because the trailing return type *is* in the scope of ThreadsListHandle).  Or it can be even shorter using type deduction
> `auto ThreadsListHandle::begin() { return list()->begin(); }`
> though I'm not sure type deduction helps readability here.

@kimbarrett not knowing how the aliasing works I was looking at it more as a textual substitution so ThreadsListHandle::Iterator looked to me like it would "expand" to ThreadsListHandler::ThreadsList::Iterator (which makes no sense) and I was not reading plain "Iterator" as ThreadsListHandle::Iterator.
I don't see why the ThreadsListHandle methods couldn't just return ThreadsList::Iterator as we know a ThreadsListHandle is just a wrapper for a ThreadsList. I don't think that is something we have to worry about "wiring" into clients. As I said the right style here would have an abstract Iterator type and we wouldn't care about the concrete implementation type.  YMMV.


PR: https://git.openjdk.java.net/jdk/pull/4948

More information about the hotspot-runtime-dev mailing list