On Wed, 2 Mar 2022 16:01:47 GMT, Pavel Rappo <pra...@openjdk.org> wrote:

> Instances of TypeMirror that are equal (TypeMirror::equals), aren't 
> necessarily the same (Types::isSameType). If care is not taken when putting 
> instances of TypeMirror into a set, that set might end up containing the same 
> instances.
> 
> If Utils.getAllInterfaces is called on a type that extends or implements a 
> particular interface multiple times (on different levels of that type's 
> hierarchy), the returned set might contain multiple representations of that 
> interface. For example, I've seen a case where getAllInterfaces that was 
> passed a TypeElement corresponding to java.util.ArrayList returned a set 
> containing 3 instances of TypeMirror corresponding to 
> `java.util.Collection<E>`.
> 
> A bit of archaeology. The old standard doclet (removed in JDK-8177511, commit 
> 33ab1995) collected instances of com.sun.javadoc.Type in a TreeMap, keying 
> them by instances of com.sun.tools.javadoc.main.ClassDocImpl (removed in 
> JDK-8215584, commit 151e628) which implemented Comparable. 
> ClassDocImpl.compareTo worked by comparing instances of CollationKey derived 
> from FQNs of types represented by respective instances of ClassDocImpl.
> 
> I'm not sure why `TreeMap<ClassDocImpl, Type>` was changed to 
> `LinkedHashSet<TypeMirror>`.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/util/Utils.java
 line 919:

> 917:             TypeElement e = asTypeElement(t);
> 918:             if (isInterface(e)) {
> 919:                 if ((isPublic(e) || isLinkable(e)) && 
> visited.add(typeUtils.asElement(t))) {

Don't you always want to record you've visited the item, whether or not it is 
public or linkable?  Would it be better to put the new `&& expr` in the 
previous `if` expression?

or even `if (isInterface(e) && visited.add(e)) ...`

-------------

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

Reply via email to