> Are deeper cycles of concern? I was thinking of this: There are a couple of ways existing java.util code handles self-cycles. The deepToString method handles them at all levels, so it is robust. But it is tricky and expensive. (Look at the variable named “dejaVu”.)
If you grep for /"(this / in the java.util sources, you will see several examples of a one-level exclusion of self-references. This is what the present PR emulates, and I think it is just fine to follow those precedents. It’s a 99% solution. Omitting the self-check would be a 90% solution, but the self-check is so simple that, why not do it? Adding a “dejaVu” table is not simple; don’t. One thing I noticed, when doing that grep, is that the type name is usually “detuned”. We should do that as well in this patch. For example, in Hashtable.java, the string says “(this Map)” not “(this Hashtable)”. The toString method tilts away from TMI. I think we have a slight TMI problem in this patch, maybe, and given the precedents I would expect “(this Function)” not “(this StableEnumFunction)”, etc. (TMI = Too Much Information. See also Gafter on the TMI temptation for language designers, which applies to API design as well: https://gafter.blogspot.com/2017/06/making-new-language-features-stand-out.html ) I see the PR’s unit tests look at this string. My humble take on it is, let’s dial back the TMI before it’s too late. (My sensitivity to TMI also informs other comments I made on this PR.)