On Mon, 6 Mar 2023 19:48:48 GMT, Pavel Rappo <[email protected]> wrote:

> Please review this explorative refactoring for VisibleMemberTable (VMT).
> 
> This is the first round of refactoring for VMT. This round is about *method 
> members*: declared (overriding and not) and inherited.
> 
> During this work I gained some insight into internal workings of VMT, fixed 
> what was feasible and left TODOs and FIXMEs for everything else. Leaving 
> those comments might look untidy, but leaving them out is wasteful: they 
> clearly mark issues that should be revisited in upcoming rounds of 
> refactoring.
> 
> As I see it today, the main issue with VMT is that implements complex and 
> error-prone computations from Java Language Specification (JLS) by hand. For 
> example, VMT interprets JLS rules for relations such as _inherits_, 
> _overrides_ and _hides_. As one would imagine, sometimes VMT does it 
> incorrectly. It would be better to eventually re-implement VMT using 
> `javax.lang.model` as much as possible. Unlike that of `jdk.javadoc`, the day 
> job of `javax.lang.model` is to provide JLS services.

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

> 647:             assert te.getKind().isClass();
> 648:             VisibleMemberTable vmt = 
> configuration.getVisibleMemberTable(te);
> 649:             for (Element e : 
> vmt.getMembers(VisibleMemberTable.Kind.METHODS)) {

In similar situations, I have found it useful to pass in a type token that 
determines the result type and which avoids casting the returned objects:

For example:

   <T> List<T extends Element> getMembers(VisibleMemberTable.Kind k, Class<T> 
clazz)

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

> 1037:             t = supertypes.get(0); // if non-empty, the first element 
> is always the superclass
> 1038:             var te = asTypeElement(t);
> 1039:             assert alreadySeen.add(te);

I guess it's OK, but it is a mild uugh to see an imperative method as an 
expression for `assert`

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

PR: https://git.openjdk.org/jdk/pull/12887

Reply via email to