hg: jdk7/tl/langtools: 6907575: [classfile] add support for classfile dependency analysis

[Jonathan Gibbons]

Rémi Forax wrote:
> Le 13/12/2009 00:35, Rémi Forax a écrit :
>> Le 12/12/2009 18:33, jonathan.gibbons at sun.com a écrit :
>>> Changeset: fbeb560f39e7
>>> Author:    jjg
>>> Date:      2009-12-12 09:28 -0800
>>> URL:       
>>> http://hg.openjdk.java.net/jdk7/tl/langtools/rev/fbeb560f39e7
>>>
>>> 6907575: [classfile] add support for classfile dependency analysis
>>> Reviewed-by: ksrini
>>>
>>> + src/share/classes/com/sun/tools/classfile/Dependencies.java
>>> + src/share/classes/com/sun/tools/classfile/Dependency.java
>>> + test/tools/javap/classfile/deps/GetDeps.java
>>> + test/tools/javap/classfile/deps/T6907575.java
>>> + test/tools/javap/classfile/deps/T6907575.out
>>> + test/tools/javap/classfile/deps/p/C1.java
>>>
>>
>> Hi Jon,
>> I have some comments about this patch set.
>
> [...]
>
> I've forget to say that in GetDeps, there is a typo in originComparator,
> getTarget is used instead of getOrigin:
>
> private Comparator<Dependency> originComparator = new 
> Comparator<Dependency>() {
>             public int compare(Dependency o1, Dependency o2) {
>                 return 
> o1.getTarget().toString().compareTo(o2.getOrigin().toString());
>                                 ^^^^^^^
>
>             }
>         };
>
> Cheers,
> Rémi

Rémi,

I've been through your comments. The comment above is the most serious 
and needs to be fixed. I'll also fix some of the minor issues regarding 
visibility of fields.

As for wildcards in return types, I think there is a time when it is 
appropriate to use them and this is one of those times.  In particular, 
they allow the use of covariant return types in overriding methods, 
which I anticipate happening here if someone were to create a finder 
that returns a iterable of a subtype of Dependency.  Also, since in this 
case we're talking about an Iterable, most clients just use Iterable 
objects in for loops, and don't need to worry about any wildcard in the 
element type. Ultimately, like most of your other comments, I guess it 
comes down to a matter of programming and API design style.

-- Jon